| From: | Greg Burd <greg(at)burd(dot)me> |
|---|---|
| To: | pgsql-hackers(at)postgresql(dot)org |
| Cc: | geidav(dot)pg(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: Hash index inserts serialise on the metapage tuple counter |
| Date: | 2026-10-26 14:45:03 |
| Message-ID: | at9mA5pMsJH3ykAL@floki |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 2026-10-26 David Geier wrote:
> +1 on fixing this.
Hello David,
Thanks for looking at this so quickly, I really appreciate it.
> The recomputed count is only used if no split or insert has occurred
> since VACUUM started its scan. If there's constant churn on the table,
> it can happen that hashm_ntuples is not recomputed for many VACUUM runs.
Right, stated plainly: hashm_ntuples is already allowed to drift from the true
tuple count between VACUUMs, batching or not. What VACUUM guarantees is a
recompute at the end of a scan that saw no concurrent split/insert, not a bound
on staleness under churn. So the counter is not a reliable tuple count today;
its only consumer is the fill-factor test that decides when to split. My
change makes it drift a bit more, in a direction that only delays splits. It
doesn't introduce a new correctness dependency, because there isn't one to
break.
> Beyond that, the batching benefit degrades as concurrency increases: to
> maintain the same bound on the aggregate counting error, the batch size
> must decrease inversely with the number of inserting backends [...]
This is a fair hit, but I think it rests on wanting a bounded counting error,
and I'm not sure we need one. The count exists to time a split. A split is
not a deadline; it's "the buckets are getting full, add more". If the index
runs batch_size * nbackends tuples past the fill factor before someone splits,
the cost is some extra overflow pages until the next inserter crosses the
threshold and splits so self-correcting, and the fill factor is a soft target
anyway. So I'd rather keep the batch fixed and accept a staleness that grows
with concurrency than shrink the batch and reintroduce the contention at high
client counts, which is the whole thing we're trying to remove.
Where I do agree with you is the determinism point:
> 2. [...] the splitting behavior is workload dependent and by that
> non-deterministic. Which makes it hard to reason about [...]
Yes. Split timing becoming a function of how many backends happened to be
inserting is the part of batched counting I like least, more than the storage
mess. It makes a reproduction depend on client count, which is exactly the
kind of thing that turns a bug report into a week of "works here".
> Maybe using the number of overflow pages vs the number of bucket pages
> would work okay enough?
I tend to agree with this because it replaces a global counter with information
the inserter already has in hand. An inserter that has to allocate or walk
onto an overflow page for its bucket already knows that bucket is over-full;
that's a local, lock-free signal that this bucket (hence, on average, the
index) wants more buckets. No metapage write on the common path at all.
Your two objections to it are the real ones, though:
> deletes will leave partially filled pages
So overflow-page count lags downward after deletes: the index can look
over-full and keep splitting when it isn't. But a hash index already doesn't
reclaim buckets on delete (maxbucket never decreases) so "splits a bit eagerly
after a delete-heavy phase" is closer to today's behaviour than the tuple
counter is. I'd want to measure this first.
> splitting overflow pages full of unique values won't cut down on the
> number of overflow pages when splitting
This is the one that actually worries me: if a bucket's overflow is caused by
many duplicate keys (all hashing to the same bucket), splitting won't relieve
it (the dups stay together) so an overflow-triggered split loops, splitting
again and again without reducing overflow. The current code has the same
pathology in slower motion (ntuples keeps rising, it keeps splitting), but a
local overflow trigger could make it tighter. A guard would be needed: don't
re-trigger on a bucket that didn't actually shed overflow across its last
split, or cap splits-per-insert.
So I think the plan is:
1. Settle that the counter's only job is approximate split timing (above).
2. Prototype the overflow-vs-bucket signal and measure it on the same
workload, specifically the delete-heavy and high-duplicate cases, which
are where it can misbehave.
3. Keep the batched counter in my back pocket as the fallback if the
overflow signal can't be made to behave, it's not elegant but it does
remove the contention.
I'll build (2) and report numbers, including the two adversarial cases, so
we're comparing measured behaviour rather than intuitions. If the overflow
signal holds up it's the better answer for the reasons you give.
best,
-greg
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-10-26 14:56:54 | Re: ANSI SQL proposal: SELECT DISTINCT ON (... ORDER BY ...) and UNION DISTINCT ON (... ORDER BY ...) |
| Previous Message | Greg Burd | 2026-10-26 14:33:16 | Re: WAL record CRC calculated incorrectly because of underlying buffer modification |