| From: | Greg Burd <greg(at)burd(dot)me> |
|---|---|
| To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Cc: | geidav(dot)pg(at)gmail(dot)com |
| Subject: | Re: Hash index inserts serialise on the metapage tuple counter |
| Date: | 2026-10-26 16:09:53 |
| Message-ID: | qbWuKS4ENTccbKF7XkP1ejOq5aIkXe4RrvCqLm6aP_3zJDFKYKYlI0BTthLKhNluo6DcCGBghbZids9W-03szF-N3kJH1hdz1T-YM2JFr1I=@burd.me |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Monday, October 26th, 2026 at 11:49 AM, Greg Burd <greg(at)burd(dot)me> wrote:
> Hey David,
>
> I built the overflow-signal prototype and measured it, including the two
> adversarial cases you flagged. Short version: it recovers the scaling,
> and the delete-heavy and high-duplicate cases behave better than either
> of us feared, not worse.
>
> Design: an insert that fits without allocating a new overflow page
> doesn't touch the metapage at all, it just bumps a per-backend pending
> count. Only an insert that adds an overflow page flushes that pending
> count into hashm_ntuples and asks for a split. The flush path already
> holds the metapage exclusive lock (that's where _hash_addovflpage
> allocated the page), so it costs nothing extra there.
> _hash_expandtable()'s own threshold re-check stays as the split
> decision, which turns out to matter (see the duplicate case below). The
> batched delta rides in the insert WAL record so a standby advances
> hashm_ntuples in the same steps.
>
> Box: c7i.8xlarge, 32 vCPU, one NUMA node, data on tmpfs, master built -O2.
>
> Scaling (pgbench, single-row INSERT, 2M preload, synchronous_commit off, 20s,
> best of 3):
>
> clients 1 16 32 64
> master 38810 361819 288961 262614
> overflow 40743 487632 558926 588115
>
> Same shape as the batched POC: master peaks at 16 clients and declines,
> overflow keeps scaling, 2.24x at 64 and no decline.
>
> > deletes will leave partially filled pages
>
> Delete-heavy (insert 2M, delete 90%, insert 2M more):
>
> master: 18692 pages, 13030 buckets, ntuples 4000000
> overflow: 16695 pages, 11980 buckets, ntuples 3898197
>
> It doesn't over-split after deletes, it splits *less*, because a split
> only fires when a bucket actually overflows, and deletes that free space
> mean fewer overflow events. So the direction of the concern is
> backwards: the local signal reacts to real fullness, where the global
> counter keeps climbing across deletes until VACUUM corrects it.
> (ntuples runs ~2.5% low from the unflushed pending counts; harmless, it
> only times splits and VACUUM recomputes it.)
>
> > splitting overflow pages full of unique values won't cut down on the number
> > of overflow pages when splitting
>
> This was the one I thought could loop, and it doesn't. High-duplicate
> load, 2M rows in 50 distinct keys:
>
> master: 12070 pages, 6515 buckets
> overflow: 10044 pages, 4931 buckets
>
> and as a direct split-loop check, 500k inserts of a *single* key:
>
> overflow: maxbucket = 1229
>
> 1229 is the normal bucket count for 500k tuples at the fill factor, not
> an explosion. If an overflow-triggered split were looping on the
> duplicates, maxbucket would run away. It doesn't, because
> _hash_expandtable() re-checks hashm_ntuples against the threshold and
> declines a split the count doesn't warrant. A bucket that overflows
> purely on duplicate keys asks for a split and gets told no. Master
> actually over-splits more here, testing the threshold on every insert.
>
> Correctness: mixed unique/duplicate/delete load gives identical row
> counts to master (596572), and index probes match seqscan on both unique
> and heavy- duplicate values. A streaming standby replays clean and ends
> at the same row count as the primary, with index probes matching.
>
> One thing worth separating out: with wal_consistency_checking='hash' the
> standby fails with "incorrect resource manager data checksum". That is
> *not* this change, unpatched master fails the same way under the same
> setting. It's the REGBUF_NO_CHANGE full-page-image bug from the other
> thread ("WAL record CRC calculated incorrectly because of underlying
> buffer modification") [0]. The two are orthogonal; with consistency
> checking off, this change replays cleanly.
>
> So I think the overflow signal is the better answer, for the reason you
> gave: it replaces a global counter on the hot path with a local signal
> the inserter already has, and the threshold re-check in
> _hash_expandtable() is a natural guard against the duplicate-key
> pathology.
>
> I'll clean the prototype into a real patch, the pending-count storage is
> still a fixed backend-local array that loses its count on backend exit,
> which is fine for correctness but I want to look at whether a relcache
> home is worth it. Prototype diff attached for reference (not submission
> quality yet).
>
> best,
>
> -greg
>
> [0] https://postgr.es/m/ats4pi-RM0Uom0T9%40floki
I forgot to attach it, lol. :)
-greg
| Attachment | Content-Type | Size |
|---|---|---|
| overflow-signal-prototype.diff | text/x-patch | 6.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-10-26 16:20:08 | Re: Let's drop support for named POSIX semaphores |
| Previous Message | Greg Burd | 2026-10-26 15:57:23 | Re: Let's drop support for named POSIX semaphores |