Re: Buildfarm failure and dubious coding in predicate.c

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@postgresql.org
Date: 2017-07-22T20:32:39Z
Lists: pgsql-hackers
I wrote:
> And, while I'm looking at this ... isn't this "scratch target" logic
> just an ineffective attempt at waving a dead chicken?  It's assuming
> that freeing an entry in a shared hash table guarantees that it can
> insert another entry.  But that hash table is partitioned, meaning it has
> a separate freelist per partition.  So the extra entry only provides a
> guarantee that you can insert something into the same partition it's in,
> making it useless for this purpose AFAICS.

After further study I found the bit in get_hash_entry() about "borrowing"
entries from other freelists.  That makes all of this safe after all,
which is a good thing because I'd also found other assumptions that would
have been broken by the behavior I posited above.  But I think that that
code is desperately undercommented -- the reader could be forgiven for
thinking that it was an optional optimization, and not something that
is critical for correctness of several different callers.  I'm going to
go improve the comments.

Meanwhile, it's still pretty unclear what happened yesterday on
culicidae.

			regards, tom lane


Commits

  1. Fix race condition in predicate-lock init code in EXEC_BACKEND builds.

  2. Improve comments about partitioned hash table freelists.