Re: Hash index inserts serialise on the metapage tuple counter

Hi David, all,

v1 attached. It is not the overflow-signal prototype I posted
earlier today, because that prototype had a hole.

The prototype only added to hashm_ntuples when an insert allocated an
overflow page, and kept every other insert's count in a per-backend
slot. Those slots die with the backend. A VACUUM that removes no
tuples never calls hashbulkdelete, so on an insert-only table nothing
puts the lost counts back. pgbench with long-lived connections never
exits, which is why the earlier numbers looked fine. With a new
connection per insert (pgbench -C, 8 clients, 200k rows):

master: ntuples 200000, maxbucket 651, 77 overflow pages
prototype: ntuples 487, maxbucket 7, 487 overflow pages

Once the count is that far off, _hash_expandtable() turns down every
split, and the chains just keep growing. CREATE INDEX had the same
problem, since the build inserts through _hash_doinsert().

v1 keeps today's rule for when to split and only stops waiting for the
lock:

- Try the metapage lock with ConditionalLockBuffer(). If we get it,
add this tuple plus anything pending to hashm_ntuples and test the
split threshold, exactly as now. Without concurrent inserters the
lock is always free, so the count is exact and split timing is the
same as master's.

- If someone else holds it, don't wait. Add 1 to a backend-local
pending count for that index (a dynahash keyed by index OID), and
leave the metapage out of the WAL record.

- Wait for the lock anyway if this insert added an overflow page, or
if 32 tuples are already pending.

- When an insert counts more than its own tuple, the increment goes
in the metapage's block data in XLOG_HASH_INSERT, and redo adds it.
pg_waldump prints "ntuples +N".

At any moment the count lags by at most 32 tuples per inserting
backend, and only by tuples that found the lock taken. That bound
grows with the number of backends, as you said it would for batching.
The difference is that a pending count only builds up while the lock
is actually contended, and the backend's next insert that finds the
lock free flushes it.

The worst case I could build was 64 concurrent short sessions
(pgbench -C, 64 clients, 40s), where the count ended 1.04% low and the
index had 3819 buckets against master's 3872. Since splits are
decided by the same count test as before, nothing new happens with
duplicates, so the duplicates-only overflow counter shouldn't be
needed. The duplicate and delete-heavy runs below come out the
same as master.

Same c7i.8xlarge (32 vCPU, one NUMA node, tmpfs, -O2, no asserts),
single-row INSERT, 2M preload, 20s per point, best of 3:

clients 1 16 32 64 128
master 39963 373439 295513 275827 251213
v1 39967 510042 569896 562110 561281

Measured against master on identical data:

- Mixed load (8 concurrent writers, unique keys plus a heavy
duplicate, delete a third, VACUUM, insert more): same row count,
same ntuples and maxbucket, index probes match seqscan for both
the unique and the duplicate key.
- 500k inserts of one key: maxbucket 1628 on both.
- Delete-heavy (2M, delete 90%, VACUUM, 2M more): 9369 pages and
7167 buckets on both.
- High-duplicate (2M rows, 50 keys, 8 writers): 6515 buckets on
both, 4902 overflow pages against 4900.
- Single-client INSERT ... SELECT of 2M rows: median 2729 ms against
2723 ms over 7 interleaved runs, which is within noise.
- WAL per insert at 32 clients: 204.5 bytes against 215.9.
- Index probes after a 64-client insert load: 935k against 942k tps,
on an index with twice the rows.

A streaming standby replaying 8 concurrent insert-ers (long and short
sessions, plus a delete and VACUUM) ends with the same row count and
the same metapage ntuples and maxbucket as the primary, and probes
match. The wal_consistency_checking=hash checksum failure I mentioned
still happens on unpatched upstream master, with or without this
patch.

Tests in src/test/modules/hash gets a regression test that forces the
deferred path through a new injection point, hash-insert-defer-count,
and checks the count, the cap, the per-index entries and that the
index still splits. It also gets a TAP test with concurrent inserts
on a primary and standby. meson test passes.

This changes the XLOG_HASH_INSERT record: the metapage block is now
optional and can carry data. So it's master-only, and whoever commits
it should bump XLOG_PAGE_MAGIC.

best.

-greg