Hash index inserts serialise on the metapage tuple counter

Hi,

While looking at hash index behaviour under concurrency I found that
single-row insert throughput stops scaling at a fairly low client count
and then goes backwards, where an equivalent btree keeps scaling. The
cause is the metapage, and I'd like to float a fix before writing a
finished patch.

The problem is that _hash_doinsert() takes an exclusive buffer lock on
the metapage for every insert, solely to run:

metap->hashm_ntuples += 1;
do_expand = metap->hashm_ntuples >
(double) metap->hashm_ffactor * (metap->hashm_maxbucket + 1);

and to register the metapage in the insert's WAL record. Every inserter
into the index therefore serialises on that one buffer. The v10
metapage cache (293e24e5078) removed the metapage read from the lookup
path, but the write on insert was left in place; Mithun Cy noted at the
time that inserts still touch the metapage every time [1], and it
doesn't seem to have been revisited.

I wanted to be sure, so I did some measurements on a c7i.8xlarge (32
vCPU, one NUMA node), data on tmpfs, synchronous_commit off,
shared_buffers 16GB. pgbench, one single-row INSERT of a random int8
per transaction, index pre-loaded with 2M rows, 20s per point, three
runs averaged:

clients 1 16 32 64
btree 44k 514k 547k 597k
hash 44k 364k 299k 273k

Hash peaks around 16 clients and declines from there. Sampling
pg_stat_activity at 32 clients, ~68% of hash inserter samples are in the
buffer-content-lock wait; btree shows none.

Timing LockBuffer(EXCLUSIVE) per buffer with a uprobe, one buffer
accounts for 99.9% of the exclusive- lock wait time, and pg_buffercache
maps it to block 0 of the index, the metapage.

Bucket splits themselves are not the problem: measured against btree
page splits they cost the same (16-32us typical, ~8ms worst case for
both, dominated by relation extension).

So here's what I propose to do, don't touch the metapage on most
inserts. Each backend accumulates its own insert count for a given
index and only takes the metapage lock once every N inserts, adding the
batch and testing the split threshold then. hashm_ntuples only drives
split timing, and VACUUM already recomputes it from scratch
(hashbulkdelete -> hashm_ntuples = num_index_tuples), so a slightly
stale counter costs at most a somewhat delayed split, never correctness.

With a proof-of-concept doing exactly this (batch size 64, attached),
the hash numbers become:

clients 1 16 32 64
hash 44k 533k 557k 604k

with that change hash now scales like btree (btree was 597k at 64).
Regression and isolation tests pass; a streaming standby with 16
concurrent inserters and splits replayed to the same hashm_ntuples and
maxbucket as the primary, and index probes on the standby found every
row.

I was chasing something else when I found this, so the attached patch is
a proof of concept, not a finished submission. It makes the point and
the numbers are real, but it cuts corners I'd want the list's opinion on
before building the real thing:

- Where should the pending per-backend count live? The POC uses a
fixed-size backend-local array keyed by relid, and simply loses the
pending count when the backend exits (VACUUM fixes hashm_ntuples
later). A cleaner home might be the relcache entry, flushed on
relcache invalidation and at transaction end, but that has its own
edge cases (invalidation dropping the count, subxact abort).

- How stale may the split trigger be? With per-backend batching the
index can run up to batch_size * nbackends tuples past the fill
factor before anyone splits. That is self-correcting (every
inserter that crosses the threshold tries to split), but it does
mean the index can briefly carry more overflow pages than today.

- Is there any appetite for removing the global counter from the hot
path entirely instead, and triggering a split from local information
the inserter already has (e.g. having had to add or walk an overflow
page)? That would also make inserts that don't split read-only on
the metapage. It's a bigger change and I haven't prototyped it yet.

If the general direction is acceptable I'll turn this into a proper
patch with the pending-count storage settled.

best.

-greg

[1] https://postgr.es/m/CAD__OuiAjLG_3mDchgy193s5-hhTjO6B8e4jvuPGGSv_tAzorA@mail.gmail.com