Re: "Allow usage of huge maintenance_work_mem for GIN build" patch

From: Noah Misch <noah(at)leadboat(dot)com>
To: Teodor Sigaev <teodor(at)sigaev(dot)ru>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: "Allow usage of huge maintenance_work_mem for GIN build" patch
Date: 2016-05-29 05:20:55
Message-ID: 20160529052055.GA496776@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, May 12, 2016 at 11:33:24AM -0400, Robert Haas wrote:
> On Fri, May 6, 2016 at 7:58 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > I noticed that commit 30bb26b5 ("Allow usage of huge
> > maintenance_work_mem for GIN build") made the following modification:
> >
> > --- a/src/include/access/gin_private.h
> > +++ b/src/include/access/gin_private.h
> > @@ -903,7 +903,7 @@ typedef struct GinEntryAccumulator
> > typedef struct
> > {
> > GinState *ginstate;
> > - long allocatedMemory;
> > + Size allocatedMemory;
> > GinEntryAccumulator *entryallocator;
> > uint32 eas_used;
> > RBTree *tree;
> >
> > Are you sure this is safe, Teodor? I don't have time to study the
> > patch in detail, but offhand I think that it might have been better to
> > make allocatedMemory of type int64, just like the tuplesort.c memory
> > accounting variables are post-MaxAllocHuge. It's not obvious to me
> > that this variable isn't allowed to occasionally become negative, just
> > like in tuplesort.c. It looks like that *might* be true -- ginbulk.c
> > may let allocatedMemory go negative for a period, which would now be
> > broken.
> >
> > If you did make this exact error, you would not be the first. If it
> > isn't actually broken, perhaps you should still make this change,
> > simply on general principle. I'd like to hear other opinions on that,
> > though.
>
> I've added this to the open items list.

[This is a generic notification.]

The above-described topic is currently a PostgreSQL 9.6 open item. Teodor,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
9.6 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within 72 hours of this
message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping 9.6rc1. Consequently, I will appreciate your
efforts toward speedy resolution. Thanks.

[1] http://www.postgresql.org/message-id/20160527025039.GA447393@tornado.leadboat.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2016-05-29 05:26:03 Re: Perf Benchmarking and regression.
Previous Message Tatsuo Ishii 2016-05-28 23:23:45 Re: Statement timeout