From: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
---|---|
To: | Костя Кузнецов <chapaev28(at)ya(dot)ru> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: New gist vacuum. |
Date: | 2015-10-31 06:55:32 |
Message-ID: | CAMkU=1zh6y+DS=PdJyLCoBpj5g71L_Xfpb9Y_arg08AgAo=r4g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 10, 2015 at 3:52 PM, Костя Кузнецов <chapaev28(at)ya(dot)ru> wrote:
> Hello. I am student from gsoc programm.
> My project is sequantial access in vacuum of gist.
>
> New vacuum has 2 big step:
> physical order scan pages and cleaning after 1 step.
>
>
> 1 scan - scan all pages and create information map(hashmap) and add
> information to rescan stack( stack of pages that needed to rescanning
This is interesting work. I think the patch needs a rebase to the git
HEAD. There is a minor conflict in gistRedoPageUpdateRecord. It is a
little confusing because your patch introduces new code and then
immediately comments it out (using //, which is not a comment style
allowed in our style guide) and that phantom code confuses the
conflict resolution process.
There are several other places throughout the patch that use //
comment style to comment out code which the patch itself added. Those
should be removed, and the real comments should be converted to /*
this */ style.
I also got a compiler warning, it looks like a missing #include:
gistutil.c: In function 'gistNewBuffer':
gistutil.c:788:4: warning: implicit declaration of function
'TransactionIdPrecedes' [-Wimplicit-function-declaration]
if (GistPageIsDeleted(page) &&
TransactionIdPrecedes(p->pd_prune_xid, RecentGlobalDataXmin)) {
^
Also, I didn't see a check on the size of the stack. Is there an
argument that this stack will not be able to grow to be large enough
to cause trouble?
Thanks,
Jeff
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2015-10-31 07:03:59 | Re: September 2015 Commitfest |
Previous Message | Robert Haas | 2015-10-31 06:05:40 | Re: ParallelContexts can get confused about which worker is which |