From 921cb976199f806186d69a980e837b1a76c38b71 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Tue, 27 Oct 2026 18:29:54 -0400 Subject: [PATCH v2] Don't make hash index inserts serialize on the metapage Every hash index insert took the metapage's exclusive content lock to increment hashm_ntuples and test whether a bucket split is due. With concurrent inserters that one buffer lock serializes them all: on a 48-core machine, single-row INSERT throughput into a hash-indexed table peaks at 16 clients and falls to 50k tps at 256, while btree reaches 720k. hashm_ntuples only decides when to split. So when an insert finds the metapage lock taken, don't wait. Leave the tuple uncounted, as a pending count in a backend-local hash table keyed by index OID, and let a later insert by the same backend that gets the lock add it to the metapage. An insert that defers its count does not register the metapage in its WAL record. When an insert counts more than its own tuple, the increment goes in the metapage's block data, and redo applies it. Even a failed ConditionalLockBuffer() writes the buffer header, as does a pin, so with many inserters the metapage's cache line still moves between CPUs on every insert. So pin the metapage only when needed, and once a backend has done 1024 inserts into the index and then found the lock taken, have it try the lock only on every 32nd insert, or when it added an overflow page. It waits for the lock once 256 tuples would be pending. Shorter sessions try on every insert, because tuples pending at backend exit are never counted. Without concurrent inserters the lock is always free and the count is exact, as before. Under concurrency the count lags by less than 256 tuples per backend, which only delays a split slightly; a VACUUM that removes tuples recounts them. Since the count can now be low, don't let the subtraction in _hash_vacuum_one_page() and its redo take it below zero. This changes the XLOG_HASH_INSERT record: the metapage block reference is now optional and may carry data. Older servers cannot replay the new records, so this is for master only, and XLOG_PAGE_MAGIC needs a bump. Discussion: https://postgr.es/m/ats7u1ISmgZaWcge@floki --- src/backend/access/hash/README | 51 ++++- src/backend/access/hash/hash_xlog.c | 17 +- src/backend/access/hash/hashinsert.c | 181 ++++++++++++++--- src/backend/access/rmgrdesc/hashdesc.c | 12 ++ src/include/access/hash_xlog.h | 3 +- src/test/modules/hash/Makefile | 6 +- .../hash/expected/hash_deferred_count.out | 183 ++++++++++++++++++ src/test/modules/hash/meson.build | 6 + .../modules/hash/sql/hash_deferred_count.sql | 84 ++++++++ .../modules/hash/t/001_concurrent_inserts.pl | 96 +++++++++ src/tools/pgindent/typedefs.list | 1 + 11 files changed, 597 insertions(+), 43 deletions(-) create mode 100644 src/test/modules/hash/expected/hash_deferred_count.out create mode 100644 src/test/modules/hash/sql/hash_deferred_count.sql create mode 100644 src/test/modules/hash/t/001_concurrent_inserts.pl diff --git a/src/backend/access/hash/README b/src/backend/access/hash/README index fc9031117c9..8575a883530 100644 --- a/src/backend/access/hash/README +++ b/src/backend/access/hash/README @@ -304,17 +304,25 @@ The insertion algorithm is rather similar: release lock but not pin, read/exclusive-lock next page; repeat as needed >> see below if no space in any page of bucket - take buffer content lock in exclusive mode on metapage + if this backend has done 1024 inserts into the index, has since seen the + metapage lock taken, this is not its 32nd insert since, and we did not + add an overflow page, don't touch the metapage + else pin the metapage and try to take the lock without waiting; if that + fails and 255 tuples are pending, wait for it insert tuple at appropriate place in page mark current page dirty - increment tuple count, decide if split needed - mark meta page dirty + if we got the metapage lock + add this tuple and any pending ones to the tuple count, decide if + split needed + mark meta page dirty + else + remember the tuple as pending write WAL for insertion of tuple - release the buffer content lock on metapage + release the buffer content lock on metapage, if we hold it release buffer content lock on current page if current page is not a bucket page, release the pin on bucket page if split is needed, enter Split algorithm below - release the pin on metapage + release the pin on metapage, if we hold one To speed searches, the index entries within any individual index page are kept sorted by hash code; the insertion code must take care to insert new @@ -326,10 +334,31 @@ that readers do not see a partially-updated page. To avoid deadlock between readers and inserters, whenever there is a need to lock multiple buckets, we always take in the order suggested in Lock Definitions above. This algorithm allows them a very high degree of -concurrency. (The exclusive metapage lock taken to update the tuple count -is stronger than necessary, since readers do not care about the tuple count, -but the lock is held for such a short time that this is probably not an -issue.) +concurrency. + +The tuple count in the metapage (hashm_ntuples) only decides when to split +a bucket. Taking the metapage lock on every insert just to bump it +serializes concurrent inserters on that one buffer, so an inserter that +finds the lock taken does not wait for it. It leaves its tuple uncounted, +as a per-backend pending count for that index. Even a failed attempt to +take the lock, or a pin, writes to the buffer header, and with many +inserters that cache line moves between CPUs on every insert. So once a +backend has done 1024 inserts into the index and then found the lock +taken, it only looks at the metapage on every 32nd insert: it tries the +lock, and if it gets it adds the pending tuples to the count. Shorter +sessions try the lock on every insert, because tuples still pending when a +backend exits are never counted, and batching would leave a large share of +a short session's tuples uncounted. An insert that had to add an overflow +page tries the lock too, because a full bucket is the sign that a split +may be due. The backend waits for the lock if 256 tuples would otherwise +be pending. Without concurrent inserters the lock is always free, and the +count is exact. + +Tuples still pending when a backend exits are never counted. That costs +at most a slightly late split, since a backend leaves fewer than 256 +tuples per index uncounted. A VACUUM that removes tuples recounts them +(see below), and the code that subtracts from the count does not let it go +negative. When an inserter cannot find space in any existing page of a bucket, it must obtain an overflow page and add that page to the bucket's chain. @@ -423,7 +452,9 @@ buckets did *not* change at any time during the scan. This provides a way of correcting the stored tuple count if it gets out of sync for some reason. But if a split or insertion does occur concurrently, the scan count is untrustworthy; instead, subtract the number of tuples deleted from the stored -tuple count and use that. +tuple count and use that. (An insertion that left its tuple pending goes +unnoticed here, but the error that causes is bounded by the cap on pending +tuples.) Interlocking Between Scans and VACUUM ------------------------------------- diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c index e9a2b9aa9a7..63ae2cc0108 100644 --- a/src/backend/access/hash/hash_xlog.c +++ b/src/backend/access/hash/hash_xlog.c @@ -123,8 +123,18 @@ hash_xlog_insert(XLogReaderState *record) if (BufferIsValid(buffer)) UnlockReleaseBuffer(buffer); + /* The metapage is included only if the insert updated the tuple count */ + if (!XLogRecHasBlockRef(record, 1)) + return; + if (XLogReadBufferForRedo(record, 1, &buffer) == BLK_NEEDS_REDO) { + uint32 ntup_delta = 1; + + if (XLogRecHasBlockData(record, 1)) + memcpy(&ntup_delta, XLogRecGetBlockData(record, 1, NULL), + sizeof(uint32)); + /* * Note: in normal operation, we'd update the metapage while still * holding lock on the page we inserted into. But during replay it's @@ -133,7 +143,7 @@ hash_xlog_insert(XLogReaderState *record) */ page = BufferGetPage(buffer); metap = HashPageGetMeta(page); - metap->hashm_ntuples += 1; + metap->hashm_ntuples += ntup_delta; PageSetLSN(page, lsn); MarkBufferDirty(buffer); @@ -1027,7 +1037,10 @@ hash_xlog_vacuum_one_page(XLogReaderState *record) metapage = BufferGetPage(metabuf); metap = HashPageGetMeta(metapage); - metap->hashm_ntuples -= xldata->ntuples; + if (metap->hashm_ntuples > xldata->ntuples) + metap->hashm_ntuples -= xldata->ntuples; + else + metap->hashm_ntuples = 0; PageSetLSN(metapage, lsn); MarkBufferDirty(metabuf); diff --git a/src/backend/access/hash/hashinsert.c b/src/backend/access/hash/hashinsert.c index 3395bbc13f8..e42213ed939 100644 --- a/src/backend/access/hash/hashinsert.c +++ b/src/backend/access/hash/hashinsert.c @@ -20,8 +20,40 @@ #include "access/xloginsert.h" #include "miscadmin.h" #include "storage/predicate.h" +#include "utils/injection_point.h" +#include "utils/memutils.h" #include "utils/rel.h" +/* + * Per-index insert state of this backend, mainly the tuples it inserted + * without adding them to the metapage's hashm_ntuples. See the description + * of the insertion algorithm in the README. Entries are keyed by index OID + * and never removed; a stale count left by a dropped or truncated index is + * less than HASH_MAX_PENDING_TUPLES, which is harmless. + */ +typedef struct HashPendingTuples +{ + Oid indexrelid; /* hash key */ + uint32 ntuples; /* tuples not yet in hashm_ntuples */ + uint32 ninserted; /* inserts, counting up to HASH_BATCH_AFTER */ + bool batching; /* seen the metapage lock taken? */ +} HashPendingTuples; + +static HTAB *hash_pending_tuples = NULL; + +/* + * Once this backend has inserted HASH_BATCH_AFTER tuples into the index and + * then found the metapage lock taken, its inserts stop looking at the + * metapage. Every HASH_PENDING_BATCH inserts it tries the lock again, and + * counts what is pending if it gets it. If HASH_MAX_PENDING_TUPLES are + * pending it waits for the lock. The tuples left pending when a backend + * exits are lost, so short sessions, whose share of those would be large, + * never batch. + */ +#define HASH_BATCH_AFTER 1024 +#define HASH_PENDING_BATCH 32 +#define HASH_MAX_PENDING_TUPLES 256 + static void _hash_vacuum_one_page(Relation rel, Relation hrel, Buffer metabuf, Buffer buf); @@ -39,14 +71,17 @@ _hash_doinsert(Relation rel, IndexTuple itup, Relation heapRel, bool sorted) { Buffer buf = InvalidBuffer; Buffer bucket_buf; - Buffer metabuf; + Buffer metabuf = InvalidBuffer; HashMetaPage metap; HashMetaPage usedmetap = NULL; - Page metapage; Page page; HashPageOpaque pageopaque; Size itemsz; bool do_expand; + bool added_ovfl = false; + bool metalocked; + HashPendingTuples *pending = NULL; + uint32 ntup_delta = 0; uint32 hashkey; Bucket bucket; OffsetNumber itup_off; @@ -65,12 +100,14 @@ _hash_doinsert(Relation rel, IndexTuple itup, Relation heapRel, bool sorted) restart_insert: /* - * Read the metapage. We don't lock it yet; HashMaxItemSize() will - * examine pd_pagesize_version, but that can't change so we can examine it - * without a lock. + * Lock the primary bucket page for the target bucket. Most inserts never + * touch the metapage, not even to pin it, because under concurrency every + * pin and lock attempt on that one buffer moves its header between CPUs. + * We pin it only when we need it. */ - metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_NOLOCK, LH_META_PAGE); - metapage = BufferGetPage(metabuf); + buf = _hash_getbucketbuf_from_hashkey(rel, hashkey, HASH_WRITE, + &usedmetap); + Assert(usedmetap != NULL); /* * Check whether the item can fit on a hash page at all. (Eventually, we @@ -79,18 +116,13 @@ restart_insert: * * XXX this is useless code if we are only storing hash keys. */ - if (itemsz > HashMaxItemSize(metapage)) + if (itemsz > HashMaxItemSize(BufferGetPage(buf))) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("index row size %zu exceeds hash maximum %zu", - itemsz, HashMaxItemSize(metapage)), + itemsz, HashMaxItemSize(BufferGetPage(buf))), errhint("Values larger than a buffer page cannot be indexed."))); - /* Lock the primary bucket page for the target bucket. */ - buf = _hash_getbucketbuf_from_hashkey(rel, hashkey, HASH_WRITE, - &usedmetap); - Assert(usedmetap != NULL); - CheckForSerializableConflictIn(rel, NULL, BufferGetBlockNumber(buf)); /* remember the primary bucket buffer to release the pin on it at end. */ @@ -114,6 +146,7 @@ restart_insert: /* release the lock on bucket buffer, before completing the split. */ LockBuffer(buf, BUFFER_LOCK_UNLOCK); + metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_NOLOCK, LH_META_PAGE); _hash_finish_split(rel, metabuf, buf, bucket, usedmetap->hashm_maxbucket, usedmetap->hashm_highmask, @@ -122,6 +155,7 @@ restart_insert: /* release the pin on old and meta buffer. retry for insert. */ _hash_dropbuf(rel, buf); _hash_dropbuf(rel, metabuf); + metabuf = InvalidBuffer; goto restart_insert; } @@ -140,6 +174,9 @@ restart_insert: if (IsBufferCleanupOK(buf)) { + if (!BufferIsValid(metabuf)) + metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_NOLOCK, + LH_META_PAGE); _hash_vacuum_one_page(rel, heapRel, metabuf, buf); if (PageGetFreeSpace(page) >= itemsz) @@ -179,8 +216,12 @@ restart_insert: LockBuffer(buf, BUFFER_LOCK_UNLOCK); /* chain to a new overflow page */ + if (!BufferIsValid(metabuf)) + metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_NOLOCK, + LH_META_PAGE); buf = _hash_addovflpage(rel, metabuf, buf, (buf == bucket_buf)); page = BufferGetPage(buf); + added_ovfl = true; /* should fit now, given test above */ Assert(PageGetFreeSpace(page) >= itemsz); @@ -193,8 +234,71 @@ restart_insert: /* * Write-lock the metapage so we can increment the tuple count. After * incrementing it, check to see if it's time for a split. + * + * Every insert wants this lock, so under concurrency it serializes the + * inserters. If someone else holds it, don't wait: leave the tuple + * pending. Once this backend has done HASH_BATCH_AFTER inserts into the + * index, a failed try also makes it start batching: it only tries the + * lock on every HASH_PENDING_BATCH'th insert, since even a failed try + * writes to the buffer header and moves that cache line between CPUs. An + * insert that added an overflow page tries anyway, since the bucket may + * be due for a split. Wait for the lock if HASH_MAX_PENDING_TUPLES are + * pending. */ - LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); + { + Oid indexrelid = RelationGetRelid(rel); + bool found; + + if (hash_pending_tuples == NULL) + { + HASHCTL ctl; + + ctl.keysize = sizeof(Oid); + ctl.entrysize = sizeof(HashPendingTuples); + ctl.hcxt = TopMemoryContext; + hash_pending_tuples = hash_create("hash index pending tuples", + 16, &ctl, + HASH_ELEM | HASH_BLOBS | + HASH_CONTEXT); + } + pending = hash_search(hash_pending_tuples, &indexrelid, HASH_ENTER, + &found); + if (!found) + { + pending->ntuples = 0; + pending->ninserted = 0; + pending->batching = false; + } + if (pending->ninserted < HASH_BATCH_AFTER) + pending->ninserted++; + } + + if (!added_ovfl && pending->batching && + pending->ntuples % HASH_PENDING_BATCH != HASH_PENDING_BATCH - 1) + metalocked = false; + else + { + if (!BufferIsValid(metabuf)) + metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_NOLOCK, + LH_META_PAGE); + +#ifdef USE_INJECTION_POINTS + /* let tests act as if the lock was taken */ + if (IS_INJECTION_POINT_ATTACHED("hash-insert-defer-count")) + metalocked = false; + else +#endif + metalocked = ConditionalLockBuffer(metabuf); + + if (!metalocked && pending->ntuples >= HASH_MAX_PENDING_TUPLES - 1) + { + LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); + metalocked = true; + } + } + + if (!metalocked && pending->ninserted >= HASH_BATCH_AFTER) + pending->batching = true; /* Do the update. No ereport(ERROR) until changes are logged */ START_CRIT_SECTION(); @@ -203,15 +307,24 @@ restart_insert: itup_off = _hash_pgaddtup(rel, buf, itemsz, itup, sorted); MarkBufferDirty(buf); - /* metapage operations */ - metap = HashPageGetMeta(metapage); - metap->hashm_ntuples += 1; + do_expand = false; + if (metalocked) + { + /* metapage operations */ + ntup_delta = 1 + pending->ntuples; + pending->ntuples = 0; - /* Make sure this stays in sync with _hash_expandtable() */ - do_expand = metap->hashm_ntuples > - (double) metap->hashm_ffactor * (metap->hashm_maxbucket + 1); + metap = HashPageGetMeta(BufferGetPage(metabuf)); + metap->hashm_ntuples += ntup_delta; - MarkBufferDirty(metabuf); + /* Make sure this stays in sync with _hash_expandtable() */ + do_expand = metap->hashm_ntuples > + (double) metap->hashm_ffactor * (metap->hashm_maxbucket + 1); + + MarkBufferDirty(metabuf); + } + else + pending->ntuples++; /* XLOG stuff */ if (RelationNeedsWAL(rel)) @@ -223,7 +336,12 @@ restart_insert: XLogBeginInsert(); XLogRegisterData(&xlrec, SizeOfHashInsert); - XLogRegisterBuffer(1, metabuf, REGBUF_STANDARD); + if (metalocked) + { + XLogRegisterBuffer(1, metabuf, REGBUF_STANDARD); + if (ntup_delta > 1) + XLogRegisterBufData(1, &ntup_delta, sizeof(uint32)); + } XLogRegisterBuffer(0, buf, REGBUF_STANDARD); XLogRegisterBufData(0, itup, IndexTupleSize(itup)); @@ -234,12 +352,14 @@ restart_insert: recptr = XLogGetFakeLSN(rel); PageSetLSN(BufferGetPage(buf), recptr); - PageSetLSN(BufferGetPage(metabuf), recptr); + if (metalocked) + PageSetLSN(BufferGetPage(metabuf), recptr); END_CRIT_SECTION(); /* drop lock on metapage, but keep pin */ - LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); + if (metalocked) + LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); /* * Release the modified page and ensure to release the pin on primary @@ -253,8 +373,9 @@ restart_insert: if (do_expand) _hash_expandtable(rel, metabuf); - /* Finally drop our pin on the metapage */ - _hash_dropbuf(rel, metabuf); + /* Finally drop our pin on the metapage, if we took one */ + if (BufferIsValid(metabuf)) + _hash_dropbuf(rel, metabuf); } /* @@ -417,8 +538,12 @@ _hash_vacuum_one_page(Relation rel, Relation hrel, Buffer metabuf, Buffer buf) pageopaque = HashPageGetOpaque(page); pageopaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES; + /* the count can be low by pending tuples; don't let it go negative */ metap = HashPageGetMeta(BufferGetPage(metabuf)); - metap->hashm_ntuples -= ndeletable; + if (metap->hashm_ntuples > ndeletable) + metap->hashm_ntuples -= ndeletable; + else + metap->hashm_ntuples = 0; MarkBufferDirty(buf); MarkBufferDirty(metabuf); diff --git a/src/backend/access/rmgrdesc/hashdesc.c b/src/backend/access/rmgrdesc/hashdesc.c index 593013afe9c..6b410b80638 100644 --- a/src/backend/access/rmgrdesc/hashdesc.c +++ b/src/backend/access/rmgrdesc/hashdesc.c @@ -46,6 +46,18 @@ hash_desc(StringInfo buf, XLogReaderState *record) xl_hash_insert *xlrec = (xl_hash_insert *) rec; appendStringInfo(buf, "off %u", xlrec->offnum); + /* a full-page image of the metapage drops the increment */ + if (XLogRecHasBlockRef(record, 1) && + (XLogRecHasBlockData(record, 1) || + !XLogRecHasBlockImage(record, 1))) + { + uint32 ntup_delta = 1; + + if (XLogRecHasBlockData(record, 1)) + memcpy(&ntup_delta, XLogRecGetBlockData(record, 1, NULL), + sizeof(uint32)); + appendStringInfo(buf, ", ntuples +%u", ntup_delta); + } break; } case XLOG_HASH_ADD_OVFL_PAGE: diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h index 149aed69dc5..af65e5b5ce3 100644 --- a/src/include/access/hash_xlog.h +++ b/src/include/access/hash_xlog.h @@ -57,7 +57,8 @@ * This data record is used for XLOG_HASH_INSERT * * Backup Blk 0: original page (data contains the inserted tuple) - * Backup Blk 1: metapage (HashMetaPageData) + * Backup Blk 1: metapage, present only if the tuple count was updated (data + * is the uint32 count increment if it is not 1) */ typedef struct xl_hash_insert { diff --git a/src/test/modules/hash/Makefile b/src/test/modules/hash/Makefile index 977c05ef4d2..8d8b798ebac 100644 --- a/src/test/modules/hash/Makefile +++ b/src/test/modules/hash/Makefile @@ -1,8 +1,10 @@ # src/test/modules/hash/Makefile -EXTRA_INSTALL = src/test/modules/injection_points +EXTRA_INSTALL = src/test/modules/injection_points contrib/pageinspect -REGRESS = hash_incomplete_splits +REGRESS = hash_deferred_count hash_incomplete_splits + +TAP_TESTS = 1 ifdef USE_PGXS PG_CONFIG = pg_config diff --git a/src/test/modules/hash/expected/hash_deferred_count.out b/src/test/modules/hash/expected/hash_deferred_count.out new file mode 100644 index 00000000000..26bcc326136 --- /dev/null +++ b/src/test/modules/hash/expected/hash_deferred_count.out @@ -0,0 +1,183 @@ +-- Test the deferred tuple count of hash index inserts. +-- +-- An insert that finds the metapage locked leaves its tuple pending instead +-- of waiting for the lock. Once a backend has done 1024 inserts into the +-- index, a failed try also makes it start batching: it only tries the lock +-- on every 32nd insert. It waits for the lock once 256 tuples would be +-- pending. The hash-insert-defer-count injection point makes every try act +-- as if the lock was taken. +CREATE EXTENSION injection_points; +CREATE EXTENSION pageinspect; +SELECT injection_points_set_local(); + injection_points_set_local +---------------------------- + +(1 row) + +CREATE TABLE hash_count_test (v int4) WITH (autovacuum_enabled = false); +CREATE INDEX hash_count_index ON hash_count_test USING hash (v); +-- Without contention, the count is exact +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 10) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 10 +(1 row) + +-- Deferred inserts leave the count alone +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 20) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 10 +(1 row) + +-- Before 1024 inserts, every insert tries the lock; the next one that gets +-- it counts what is pending +SELECT injection_points_detach('hash-insert-defer-count'); + injection_points_detach +------------------------- + +(1 row) + +INSERT INTO hash_count_test VALUES (1); +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 31 +(1 row) + +-- After 1024 inserts, a failed try starts batching +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 1000) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1031 +(1 row) + +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 20) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1031 +(1 row) + +-- The insert that would leave 256 pending waits for the lock and counts +-- them all, leaving 64 of these 300 pending +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 300) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1287 +(1 row) + +-- With the lock free again, the backend still only tries it on every 32nd +-- insert: 31 more leave 95 pending, and the next one counts all 96 +SELECT injection_points_detach('hash-insert-defer-count'); + injection_points_detach +------------------------- + +(1 row) + +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 31) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1287 +(1 row) + +INSERT INTO hash_count_test VALUES (1); +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1383 +(1 row) + +SELECT count(*) FROM hash_count_test; + count +------- + 1383 +(1 row) + +-- Each index has its own pending count: a new index counts every insert +CREATE INDEX hash_count_index2 ON hash_count_test USING hash (v); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 5) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + ntuples +--------- + 1383 +(1 row) + +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index2', 0)); + ntuples +--------- + 1388 +(1 row) + +-- A deferred count cannot stop the index from splitting, because at most +-- 255 tuples stay pending. Build a 2-bucket index, then insert with every +-- try of the lock failing. +DROP INDEX hash_count_index, hash_count_index2; +TRUNCATE hash_count_test; +INSERT INTO hash_count_test SELECT 1 FROM generate_series(1, 10); +ANALYZE hash_count_test; +CREATE INDEX hash_count_index ON hash_count_test USING hash (v); +SELECT maxbucket FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + maxbucket +----------- + 1 +(1 row) + +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); + injection_points_attach +------------------------- + +(1 row) + +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 10000) g; +SELECT injection_points_detach('hash-insert-defer-count'); + injection_points_detach +------------------------- + +(1 row) + +SELECT maxbucket > 16 AS split, + ntuples >= 10010 - 255 AND ntuples <= 10010 AS count_ok + FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + split | count_ok +-------+---------- + t | t +(1 row) + +-- Index scans see the deferred tuples +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM hash_count_test WHERE v = 1; + count +------- + 11 +(1 row) + +SELECT count(*) FROM hash_count_test WHERE v = 9999; + count +------- + 1 +(1 row) + +RESET enable_seqscan; +RESET enable_bitmapscan; +DROP TABLE hash_count_test; +DROP EXTENSION pageinspect; +DROP EXTENSION injection_points; diff --git a/src/test/modules/hash/meson.build b/src/test/modules/hash/meson.build index 284ebab70ff..9b009607396 100644 --- a/src/test/modules/hash/meson.build +++ b/src/test/modules/hash/meson.build @@ -10,7 +10,13 @@ tests += { 'bd': meson.current_build_dir(), 'regress': { 'sql': [ + 'hash_deferred_count', 'hash_incomplete_splits', ], }, + 'tap': { + 'tests': [ + 't/001_concurrent_inserts.pl', + ], + }, } diff --git a/src/test/modules/hash/sql/hash_deferred_count.sql b/src/test/modules/hash/sql/hash_deferred_count.sql new file mode 100644 index 00000000000..d89e98b080b --- /dev/null +++ b/src/test/modules/hash/sql/hash_deferred_count.sql @@ -0,0 +1,84 @@ +-- Test the deferred tuple count of hash index inserts. +-- +-- An insert that finds the metapage locked leaves its tuple pending instead +-- of waiting for the lock. Once a backend has done 1024 inserts into the +-- index, a failed try also makes it start batching: it only tries the lock +-- on every 32nd insert. It waits for the lock once 256 tuples would be +-- pending. The hash-insert-defer-count injection point makes every try act +-- as if the lock was taken. +CREATE EXTENSION injection_points; +CREATE EXTENSION pageinspect; +SELECT injection_points_set_local(); + +CREATE TABLE hash_count_test (v int4) WITH (autovacuum_enabled = false); +CREATE INDEX hash_count_index ON hash_count_test USING hash (v); + +-- Without contention, the count is exact +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 10) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- Deferred inserts leave the count alone +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 20) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- Before 1024 inserts, every insert tries the lock; the next one that gets +-- it counts what is pending +SELECT injection_points_detach('hash-insert-defer-count'); +INSERT INTO hash_count_test VALUES (1); +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- After 1024 inserts, a failed try starts batching +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 1000) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 20) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- The insert that would leave 256 pending waits for the lock and counts +-- them all, leaving 64 of these 300 pending +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 300) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- With the lock free again, the backend still only tries it on every 32nd +-- insert: 31 more leave 95 pending, and the next one counts all 96 +SELECT injection_points_detach('hash-insert-defer-count'); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 31) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); +INSERT INTO hash_count_test VALUES (1); +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); +SELECT count(*) FROM hash_count_test; + +-- Each index has its own pending count: a new index counts every insert +CREATE INDEX hash_count_index2 ON hash_count_test USING hash (v); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 5) g; +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); +SELECT ntuples FROM hash_metapage_info(get_raw_page('hash_count_index2', 0)); + +-- A deferred count cannot stop the index from splitting, because at most +-- 255 tuples stay pending. Build a 2-bucket index, then insert with every +-- try of the lock failing. +DROP INDEX hash_count_index, hash_count_index2; +TRUNCATE hash_count_test; +INSERT INTO hash_count_test SELECT 1 FROM generate_series(1, 10); +ANALYZE hash_count_test; +CREATE INDEX hash_count_index ON hash_count_test USING hash (v); +SELECT maxbucket FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); +SELECT injection_points_attach('hash-insert-defer-count', 'notice'); +INSERT INTO hash_count_test SELECT g FROM generate_series(1, 10000) g; +SELECT injection_points_detach('hash-insert-defer-count'); +SELECT maxbucket > 16 AS split, + ntuples >= 10010 - 255 AND ntuples <= 10010 AS count_ok + FROM hash_metapage_info(get_raw_page('hash_count_index', 0)); + +-- Index scans see the deferred tuples +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM hash_count_test WHERE v = 1; +SELECT count(*) FROM hash_count_test WHERE v = 9999; +RESET enable_seqscan; +RESET enable_bitmapscan; + +DROP TABLE hash_count_test; +DROP EXTENSION pageinspect; +DROP EXTENSION injection_points; diff --git a/src/test/modules/hash/t/001_concurrent_inserts.pl b/src/test/modules/hash/t/001_concurrent_inserts.pl new file mode 100644 index 00000000000..ba865cc2031 --- /dev/null +++ b/src/test/modules/hash/t/001_concurrent_inserts.pl @@ -0,0 +1,96 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Concurrent hash index inserts defer their tuple count when they find the +# metapage locked. Check that such inserts replay correctly on a standby, +# that the index still splits as it grows, and that the deferred count stays +# close to the real one even when sessions are short. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Utils; +use PostgreSQL::Test::Cluster; +use Test::More; + +my $primary = PostgreSQL::Test::Cluster->new('primary'); +$primary->init(allows_streaming => 1); +$primary->append_conf('postgresql.conf', 'autovacuum = off'); +$primary->start; +$primary->safe_psql('postgres', 'CREATE EXTENSION pageinspect'); + +$primary->backup('bkp'); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($primary, 'bkp', has_streaming => 1); +$standby->start; + +$primary->safe_psql( + 'postgres', q{ +CREATE TABLE hash_conc (v int4); +CREATE INDEX hash_conc_idx ON hash_conc USING hash (v); +}); + +my $meta = q{SELECT ntuples, maxbucket + FROM hash_metapage_info(get_raw_page('hash_conc_idx', 0))}; +my (undef, $maxbucket0) = split /\|/, $primary->safe_psql('postgres', $meta); + +# Long sessions, and short ones that exit with tuples pending +$primary->pgbench( + '--no-vacuum --client=8 --jobs=8 --transactions=2000', + 0, + [qr{actually processed}], + [qr{^$}], + 'concurrent inserts', + { + 'hash_conc_long' => q{ +\set v random(1, 1000000000) +INSERT INTO hash_conc VALUES (:v); +INSERT INTO hash_conc VALUES (:v + 1); +} + }); +$primary->pgbench( + '--no-vacuum --client=8 --jobs=8 --transactions=200 --connect', + 0, + [qr{actually processed}], + [qr{^$}], + 'concurrent short sessions', + { + 'hash_conc_short' => q{ +\set v random(1, 1000000000) +INSERT INTO hash_conc VALUES (:v); +} + }); + +my $nrows = $primary->safe_psql('postgres', 'SELECT count(*) FROM hash_conc'); +is($nrows, 8 * 2000 * 2 + 8 * 200, 'all rows inserted'); + +my ($ntuples, $maxbucket) = + split /\|/, $primary->safe_psql('postgres', $meta); +cmp_ok($maxbucket, '>', $maxbucket0, 'index split'); +cmp_ok($ntuples, '<=', $nrows, 'tuple count not too high'); +# A session leaves fewer than 256 tuples uncounted, and a single-insert +# session at most one +cmp_ok( + $ntuples, '>=', + $nrows - (8 * 256 + 8 * 200), + 'tuple count not too low'); + +$primary->wait_for_replay_catchup($standby); + +is($standby->safe_psql('postgres', $meta), + "$ntuples|$maxbucket", 'standby metapage matches primary'); + +my $probe = q{ +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM hash_conc h + WHERE (SELECT count(*) FROM hash_conc i WHERE i.v = h.v) = + (SELECT count(*) FROM hash_conc s WHERE s.v + 0 = h.v) + AND h.v % 97 = 0}; +my $expected = $primary->safe_psql('postgres', + 'SELECT count(*) FROM hash_conc WHERE v % 97 = 0'); +is($primary->safe_psql('postgres', $probe), + $expected, 'index probes match heap on primary'); +is($standby->safe_psql('postgres', $probe), + $expected, 'index probes match heap on standby'); + +done_testing(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index cfef572df80..97804a81712 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1229,6 +1229,7 @@ HashPageOpaque HashPageOpaqueData HashPageStat HashPath +HashPendingTuples HashScanOpaque HashScanOpaqueData HashScanPosData -- 2.54.0