Re: New gist vacuum.

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
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

In response to

Responses

Browse pgsql-hackers by date

  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