From c8513fc2233a05b69ef839b51ea8dba894d7b16e Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Mon, 2 Nov 2026 17:50:53 -0500 Subject: [PATCH v3 2/2] Allocate and free hash overflow pages without the metapage lock Allocating an overflow page took the metapage's exclusive lock twice, and searched the bitmap page while holding its exclusive lock; freeing one took the metapage lock to lower hashm_firstfree. A growing hash index allocates and frees overflow pages about as often as it splits buckets: linear hashing splits buckets in order, so the buckets not yet split in a round fill past a page and each get an overflow page, which they give back when they are split. With the tuple counter off the insert path, those locks were where concurrent inserters waited. Search the bitmap pages using our cached copy of the metapage, under a share lock, and lock a bitmap page exclusively only to claim a bit. A free bit always stands for an existing page, since bits for pages not yet added are set, but our copy may not know the splitpoint a recently added page is in, so refresh the copy before claiming a bit past the range it knows. Lock the metapage only when no free page is found: search all bitmap pages again, from the first, and if there is still none, extend the index by 64 overflow pages at once, all but the first left free in the bitmap. The XLOG_HASH_ADD_OVFL_PAGE record says how many pages were added and registers the metapage only when it changes. Freeing a page no longer lowers hashm_firstfree, so it does not lock the metapage either. hashm_firstfree now only tells searches which bitmap page to start at; a page freed in an earlier bitmap page is found by the search under the metapage lock, before the index is extended. The metapage is now only locked before a bitmap page, never while one is held. This changes XLOG_HASH_ADD_OVFL_PAGE, so it is for master only, and XLOG_PAGE_MAGIC needs a bump. Discussion: https://postgr.es/m/ats7u1ISmgZaWcge@floki --- src/backend/access/hash/README | 93 +++-- src/backend/access/hash/hash_xlog.c | 15 +- src/backend/access/hash/hashovfl.c | 393 ++++++++++++------ src/backend/access/hash/hashpage.c | 6 +- src/backend/access/rmgrdesc/hashdesc.c | 5 +- src/include/access/hash.h | 2 + src/include/access/hash_xlog.h | 11 +- src/test/modules/hash/Makefile | 2 +- .../hash/expected/hash_overflow_pages.out | 108 +++++ src/test/modules/hash/meson.build | 1 + .../modules/hash/sql/hash_overflow_pages.sql | 65 +++ 11 files changed, 516 insertions(+), 185 deletions(-) create mode 100644 src/test/modules/hash/expected/hash_overflow_pages.out create mode 100644 src/test/modules/hash/sql/hash_overflow_pages.sql diff --git a/src/backend/access/hash/README b/src/backend/access/hash/README index 8575a883530..6a084378853 100644 --- a/src/backend/access/hash/README +++ b/src/backend/access/hash/README @@ -495,36 +495,56 @@ overflow page to the free pool. Obtaining an overflow page: + using our cached copy of the metapage, start at the bitmap page that + first-free-bit is in + for each bitmap page: + pin bitmap page and take content lock in shared mode + search for a free page (zero bit in bitmap) + if found: + retake the content lock in exclusive mode, and search again + if the bit is past the overflow pages our copy knows of, + release the bitmap page, refresh our copy, retry the page + set bit in bitmap + mark bitmap page dirty + return page number + release bitmap page buffer content lock +-- here when none found take metapage content lock in exclusive mode - determine next bitmap page number; if none, exit loop - release meta page content lock - pin bitmap page and take content lock in exclusive mode - search for a free page (zero bit in bitmap) - if found: - set bit in bitmap - mark bitmap page dirty - take metapage buffer content lock in exclusive mode - if first-free-bit value did not change, - update it and mark meta page dirty - else (not found): - release bitmap page buffer content lock - loop back to try next bitmap page, if any --- here when we have checked all bitmap pages; we hold meta excl. lock - extend index to add another overflow page; update meta information + search all bitmap pages again as above, from the first one + if found, lower first-free-bit to it if it's higher, and return it + extend index to add a batch of overflow pages; update meta information + mark all but the first of the new pages free in the bitmap + set first-free-bit to the second new page mark meta page dirty return page number -It is slightly annoying to release and reacquire the metapage lock -multiple times, but it seems best to do it that way to minimize loss of -concurrency against processes just entering the index. We don't want -to hold the metapage exclusive lock while reading in a bitmap page. -(We can at least avoid repeated buffer pin/unpin here.) +Most overflow pages are allocated and freed while the index grows. Linear +hashing splits buckets in order, so the buckets not yet split in the +current round fill past a page and get an overflow page each, which they +give back when they are split. Taking the metapage lock for each of those, +as was done before, serialized concurrent inserters behind it, and holding +the bitmap page lock in exclusive mode while searching it serialized them +again. So the metapage is locked only to add pages, and then pages are +added 64 at a time. + +A bit is always set for a page that does not exist yet, and set for a page +in use, so a free bit always stands for an existing page. But a cached copy +of the metapage can be older than the splitpoint the page was added in, and +then would compute the wrong block number for it; hence the refresh. The +search under the metapage lock, from the first bitmap page, also finds +pages freed in bitmap pages before the one first-free-bit is in; see below. + +Lock order: the metapage is only ever locked before a bitmap page, never +while one is held. The normal path for extending the index does not require doing I/O while holding the metapage lock. We do have to do I/O when the extension requires adding a new bitmap page as well as the required overflow page ... but that is an infrequent case, so the loss of concurrency seems -acceptable. +acceptable. A batch of pages is added by writing the last of them, as for +a splitpoint's bucket pages; the pages in between read as zeroes until +they are used. A batch never crosses into a new bitmap page; when one is +needed, a single page is added along with it, as before. The portion of tuple insertion that calls the above subroutine looks like this: @@ -564,27 +584,24 @@ accessors of pages in the bucket. The algorithm is: delink overflow page from bucket chain (this requires read/update/write/release of fore and aft siblings) - pin meta page and take buffer content lock in shared mode - determine which bitmap page contains the free space bit for page - release meta page buffer content lock + using our cached copy of the metapage (refreshed if it doesn't know + the page), determine which bitmap page contains the free space bit pin bitmap page and take buffer content lock in exclusive mode - retake meta page buffer content lock in exclusive mode move (insert) tuples that belong to the overflow page being freed update bitmap bit mark bitmap page dirty - if page number is still less than first-free-bit, - update first-free-bit field and mark meta page dirty write WAL for delinking overflow page operation release buffer content lock and pin - release meta page buffer content lock and pin -We have to do it this way because we must clear the bitmap bit before -changing the first-free-bit field (hashm_firstfree). It is possible that -we set first-free-bit too small (because someone has already reused the -page we just freed), but that is okay; the only cost is the next overflow -page acquirer will scan more bitmap bits than he needs to. What must be -avoided is having first-free-bit greater than the actual first free bit, -because then that free page would never be found by searchers. +Freeing a page does not lower first-free-bit (hashm_firstfree), so it does +not lock the metapage. First-free-bit only says which bitmap page searches +start at; within a bitmap page, a search looks at all its bits. A page +freed in an earlier bitmap page is found by the search under the metapage +lock, which starts at the first bitmap page, before the index is extended. +So a free page is never lost, though one in an earlier bitmap page may wait +until the next time the index would otherwise be extended. Only indexes +with more than one bitmap page (more than 32768 overflow pages, with 8kB +pages) are affected. The reason of moving tuples from overflow page while delinking the later is to make that as an atomic operation. Not doing so could lead to spurious reads @@ -609,9 +626,9 @@ write-ahead log record anyway, because we can log only a fixed number of pages, as given by XLR_MAX_BLOCK_ID (32), with current XLog machinery. Ordinary item insertions (that don't force a page split or need a new overflow -page) are single WAL entries. They touch a single bucket page and the -metapage. The metapage is updated during replay as it is updated during -original operation. +page) are single WAL entries. They touch a single bucket page, and the +metapage if they update the tuple count. The metapage is updated during +replay as it is updated during original operation. If an insertion causes the addition of an overflow page, there will be one WAL entry for the new overflow page and second entry for insert itself. diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c index 63ae2cc0108..308c3a0290d 100644 --- a/src/backend/access/hash/hash_xlog.c +++ b/src/backend/access/hash/hash_xlog.c @@ -230,7 +230,14 @@ hash_xlog_add_ovfl_page(XLogReaderState *record) data = XLogRecGetBlockData(record, 2, &datalen); bitmap_page_bit = (uint32 *) data; - SETBIT(freep, *bitmap_page_bit); + if (xlrec->bmpage_found) + SETBIT(freep, *bitmap_page_bit); + else + { + /* added pages left free */ + for (uint32 k = 0; k < xlrec->nadded - 1; k++) + CLRBIT(freep, *bitmap_page_bit + k); + } PageSetLSN(mappage, lsn); MarkBufferDirty(mapbuffer); @@ -256,6 +263,10 @@ hash_xlog_add_ovfl_page(XLogReaderState *record) UnlockReleaseBuffer(newmapbuf); } + /* The metapage is present only if it was modified */ + if (!XLogRecHasBlockRef(record, 4)) + return; + if (XLogReadBufferForRedo(record, 4, &metabuf) == BLK_NEEDS_REDO) { HashMetaPage metap; @@ -271,7 +282,7 @@ hash_xlog_add_ovfl_page(XLogReaderState *record) if (!xlrec->bmpage_found) { - metap->hashm_spares[metap->hashm_ovflpoint]++; + metap->hashm_spares[metap->hashm_ovflpoint] += xlrec->nadded; if (new_bmpage) { diff --git a/src/backend/access/hash/hashovfl.c b/src/backend/access/hash/hashovfl.c index dbc57ef958c..d864dc02a0c 100644 --- a/src/backend/access/hash/hashovfl.c +++ b/src/backend/access/hash/hashovfl.c @@ -25,6 +25,19 @@ static uint32 _hash_firstfreebit(uint32 map); +static Buffer _hash_findfreeovflpage(Relation rel, Buffer *metabuf, + HashMetaPage *metapp, Buffer *mapbufp, + uint32 *bitmap_page_bit, uint32 *bitp, + bool metalocked); +static bool _hash_ovflblkno_to_bitno_internal(HashMetaPage metap, + BlockNumber ovflblkno, + uint32 *bitno); + +/* + * When an overflow page has to be added at the end of the index, add this + * many, and mark the ones we don't use yet as free. + */ +#define HASH_OVFL_EXTEND_BATCH 64 /* @@ -60,6 +73,23 @@ bitno_to_blkno(HashMetaPage metap, uint32 ovflbitnum) */ uint32 _hash_ovflblkno_to_bitno(HashMetaPage metap, BlockNumber ovflblkno) +{ + uint32 bitnum; + + if (!_hash_ovflblkno_to_bitno_internal(metap, ovflblkno, &bitnum)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("invalid overflow block number %u", ovflblkno))); + return bitnum; +} + +/* + * Workhorse for _hash_ovflblkno_to_bitno; returns false instead of raising an + * error if ovflblkno isn't an overflow page according to metap. + */ +static bool +_hash_ovflblkno_to_bitno_internal(HashMetaPage metap, BlockNumber ovflblkno, + uint32 *bitno) { uint32 splitnum = metap->hashm_ovflpoint; uint32 i; @@ -80,13 +110,13 @@ _hash_ovflblkno_to_bitno(HashMetaPage metap, BlockNumber ovflblkno) */ if (bitnum > metap->hashm_spares[i - 1] && bitnum <= metap->hashm_spares[i]) - return bitnum - 1; /* -1 to convert 1-based to 0-based */ + { + *bitno = bitnum - 1; /* -1 to convert 1-based to 0-based */ + return true; + } } - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("invalid overflow block number %u", ovflblkno))); - return 0; /* keep compiler quiet */ + return false; } /* @@ -117,21 +147,20 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf, bool retain_pin) HashPageOpaque pageopaque; HashPageOpaque ovflopaque; HashMetaPage metap; + HashMetaPage cachedmetap; Buffer mapbuf = InvalidBuffer; Buffer newmapbuf = InvalidBuffer; BlockNumber blkno; - uint32 orig_firstfree; - uint32 splitnum; + uint32 splitnum = 0; uint32 *freep = NULL; uint32 max_ovflpg; uint32 bit; - uint32 bitmap_page_bit; - uint32 first_page; + uint32 bitmap_page_bit = 0; uint32 last_bit; - uint32 last_page; - uint32 i, - j; + uint16 nadded = 1; bool page_found = false; + bool metalocked = false; + bool metamodified = false; XLogRecPtr recptr; /* @@ -181,91 +210,53 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf, bool retain_pin) buf = _hash_getbuf(rel, nextblkno, HASH_WRITE, LH_OVERFLOW_PAGE); } - /* Get exclusive lock on the meta page */ + /* + * Look for a free overflow page in the bitmap pages, without the metapage + * lock. The bitmap pages' locations and the range of existing overflow + * pages come from our cached copy of the metapage. A bitmap page is + * searched under a share lock, and exclusive-locked only to claim a bit. + * Bits for pages beyond the last existing overflow page are always set, + * so a free bit always stands for a real page, but a page added since our + * copy was taken might be in a splitpoint the copy doesn't know of; so + * refresh the copy before claiming a bit beyond its range. + */ + cachedmetap = _hash_getcachedmetap(rel, &metabuf, false); + ovflbuf = _hash_findfreeovflpage(rel, &metabuf, &cachedmetap, &mapbuf, + &bitmap_page_bit, &bit, false); + if (BufferIsValid(ovflbuf)) + { + page_found = true; + metap = cachedmetap; + goto found; + } + + /* + * None found. Lock the metapage, and search again with its current + * contents, from the first bitmap page: this also finds pages freed in + * bitmap pages before the one hashm_firstfree points into. + */ LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); + metalocked = true; _hash_checkpage(rel, metabuf, LH_META_PAGE); metap = HashPageGetMeta(BufferGetPage(metabuf)); - /* start search at hashm_firstfree */ - orig_firstfree = metap->hashm_firstfree; - first_page = orig_firstfree >> BMPG_SHIFT(metap); - bit = orig_firstfree & BMPG_MASK(metap); - i = first_page; - j = bit / BITS_PER_MAP; - bit &= ~(BITS_PER_MAP - 1); - - /* outer loop iterates once per bitmap page */ - for (;;) + ovflbuf = _hash_findfreeovflpage(rel, &metabuf, &metap, &mapbuf, + &bitmap_page_bit, &bit, true); + if (BufferIsValid(ovflbuf)) { - BlockNumber mapblkno; - Page mappage; - uint32 last_inpage; - - /* want to end search with the last existing overflow page */ - splitnum = metap->hashm_ovflpoint; - max_ovflpg = metap->hashm_spares[splitnum] - 1; - last_page = max_ovflpg >> BMPG_SHIFT(metap); - last_bit = max_ovflpg & BMPG_MASK(metap); - - if (i > last_page) - break; - - Assert(i < metap->hashm_nmaps); - mapblkno = metap->hashm_mapp[i]; - - if (i == last_page) - last_inpage = last_bit; - else - last_inpage = BMPGSZ_BIT(metap) - 1; - - /* Release exclusive lock on metapage while reading bitmap page */ - LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); - - mapbuf = _hash_getbuf(rel, mapblkno, HASH_WRITE, LH_BITMAP_PAGE); - mappage = BufferGetPage(mapbuf); - freep = HashPageGetBitmap(mappage); - - for (; bit <= last_inpage; j++, bit += BITS_PER_MAP) - { - if (freep[j] != ALL_SET) - { - page_found = true; - - /* Reacquire exclusive lock on the meta page */ - LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); - - /* convert bit to bit number within page */ - bit += _hash_firstfreebit(freep[j]); - bitmap_page_bit = bit; - - /* convert bit to absolute bit number */ - bit += (i << BMPG_SHIFT(metap)); - /* Calculate address of the recycled overflow page */ - blkno = bitno_to_blkno(metap, bit); - - /* Fetch and init the recycled page */ - ovflbuf = _hash_getinitbuf(rel, blkno); - - goto found; - } - } - - /* No free space here, try to advance to next map page */ - _hash_relbuf(rel, mapbuf); - mapbuf = InvalidBuffer; - i++; - j = 0; /* scan from start of next map page */ - bit = 0; - - /* Reacquire exclusive lock on the meta page */ - LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); + page_found = true; + goto found; } /* * No free pages --- have to extend the relation to add an overflow page. * First, check to see if we have to add a new bitmap page too. */ + splitnum = metap->hashm_ovflpoint; + max_ovflpg = metap->hashm_spares[splitnum] - 1; + last_bit = max_ovflpg & BMPG_MASK(metap); + if (last_bit == (uint32) (BMPGSZ_BIT(metap) - 1)) { /* @@ -290,9 +281,38 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf, bool retain_pin) else { /* - * Nothing to do here; since the page will be past the last used page, - * we know its bitmap bit was preinitialized to "in use". + * Add a batch of pages, so that the next overflow pages can be found + * in the bitmap without the metapage lock. All the pages' bits must + * be on the last bitmap page. Their bits are set, since they are + * past the last used page; clear all but the first, which we return. */ + nadded = Min(HASH_OVFL_EXTEND_BATCH, + BMPGSZ_BIT(metap) - 1 - last_bit); + + if (nadded > 1) + { + BlockNumber firstblk = bitno_to_blkno(metap, max_ovflpg + 1); + + /* + * Extend the relation to cover the whole batch. The pages after + * the first are left as zero pages, to be initialized when used. + * If a previous attempt crashed after extending, they may exist + * already. + */ + if (RelationGetNumberOfBlocks(rel) < firstblk + nadded && + !_hash_alloc_buckets(rel, firstblk + 1, nadded - 1)) + nadded = 1; + } + + if (nadded > 1) + { + mapbuf = _hash_getbuf(rel, metap->hashm_mapp[metap->hashm_nmaps - 1], + HASH_WRITE, LH_BITMAP_PAGE); + freep = HashPageGetBitmap(BufferGetPage(mapbuf)); + bitmap_page_bit = last_bit + 2; + } + else + nadded = 1; } /* Calculate address of the new overflow page */ @@ -314,6 +334,9 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf, bool retain_pin) found: + if (page_found) + freep = HashPageGetBitmap(BufferGetPage(mapbuf)); + /* * Do the update. No ereport(ERROR) until changes are logged. We want to * log the changes for bitmap page and overflow page together to avoid @@ -328,11 +351,28 @@ found: /* mark page "in use" in the bitmap */ SETBIT(freep, bitmap_page_bit); MarkBufferDirty(mapbuf); + + /* + * If we searched with the metapage locked, start the next searches at + * this page's bitmap page. + */ + if (metalocked && bit < metap->hashm_firstfree) + { + metap->hashm_firstfree = bit; + MarkBufferDirty(metabuf); + metamodified = true; + } } else { - /* update the count to indicate new overflow page is added */ - metap->hashm_spares[splitnum]++; + /* update the count to indicate new overflow pages are added */ + metap->hashm_spares[splitnum] += nadded; + + /* the pages after the first are free */ + for (uint32 k = 0; k < nadded - 1; k++) + CLRBIT(freep, bitmap_page_bit + k); + if (nadded > 1) + MarkBufferDirty(mapbuf); if (BufferIsValid(newmapbuf)) { @@ -345,7 +385,11 @@ found: metap->hashm_spares[splitnum]++; } + /* start the next searches after this page */ + metap->hashm_firstfree = bit + 1; + MarkBufferDirty(metabuf); + metamodified = true; /* * for new overflow page, we don't need to explicitly set the bit in @@ -353,16 +397,6 @@ found: */ } - /* - * Adjust hashm_firstfree to avoid redundant searches. But don't risk - * changing it if someone moved it while we were searching bitmap pages. - */ - if (metap->hashm_firstfree == orig_firstfree) - { - metap->hashm_firstfree = bit + 1; - MarkBufferDirty(metabuf); - } - /* initialize new overflow page */ ovflpage = BufferGetPage(ovflbuf); ovflopaque = HashPageGetOpaque(ovflpage); @@ -386,6 +420,7 @@ found: xlrec.bmpage_found = page_found; xlrec.bmsize = metap->hashm_bmsize; + xlrec.nadded = page_found ? 0 : nadded; XLogBeginInsert(); XLogRegisterData(&xlrec, SizeOfHashAddOvflPage); @@ -404,8 +439,11 @@ found: if (BufferIsValid(newmapbuf)) XLogRegisterBuffer(3, newmapbuf, REGBUF_WILL_INIT); - XLogRegisterBuffer(4, metabuf, REGBUF_STANDARD); - XLogRegisterBufData(4, &metap->hashm_firstfree, sizeof(uint32)); + if (metamodified) + { + XLogRegisterBuffer(4, metabuf, REGBUF_STANDARD); + XLogRegisterBufData(4, &metap->hashm_firstfree, sizeof(uint32)); + } recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_ADD_OVFL_PAGE); } @@ -421,7 +459,8 @@ found: if (BufferIsValid(newmapbuf)) PageSetLSN(BufferGetPage(newmapbuf), recptr); - PageSetLSN(BufferGetPage(metabuf), recptr); + if (metamodified) + PageSetLSN(BufferGetPage(metabuf), recptr); END_CRIT_SECTION(); @@ -433,7 +472,8 @@ found: if (BufferIsValid(mapbuf)) _hash_relbuf(rel, mapbuf); - LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); + if (metalocked) + LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); if (BufferIsValid(newmapbuf)) _hash_relbuf(rel, newmapbuf); @@ -441,6 +481,104 @@ found: return ovflbuf; } +/* + * _hash_findfreeovflpage() + * + * Find a free overflow page in the bitmap pages, and return it pinned, + * write-locked and initialized, with the bitmap page that has its bit + * write-locked in *mapbufp. The bit is not set yet: the caller does that + * in its critical section. *bitmap_page_bit is the bit's number within its + * bitmap page, and *bitp the overflow page's bit number. Returns + * InvalidBuffer if there is no free page. + * + * If metalocked, *metapp points to the metapage, which the caller holds + * write-locked, and we search all the bitmap pages. Otherwise *metapp is + * our cached copy of the metapage, and we search from the bitmap page that + * hashm_firstfree is in. A bit beyond the range of overflow pages the copy + * knows of can stand for a page in a splitpoint added since, so we refresh + * the copy before using such a bit. Bits for pages that don't exist yet + * are set, so a free bit always stands for an existing page. + * + * Lock order: we lock the metapage only if no bitmap page lock is held. + */ +static Buffer +_hash_findfreeovflpage(Relation rel, Buffer *metabuf, HashMetaPage *metapp, + Buffer *mapbufp, uint32 *bitmap_page_bit, uint32 *bitp, + bool metalocked) +{ + HashMetaPage metap = *metapp; + uint32 i; + + i = metalocked ? 0 : metap->hashm_firstfree >> BMPG_SHIFT(metap); + + while (i < metap->hashm_nmaps) + { + Buffer mapbuf; + uint32 *freep; + uint32 nwords = metap->hashm_bmsize / sizeof(uint32); + uint32 j; + bool exclusive = false; + + mapbuf = _hash_getbuf(rel, metap->hashm_mapp[i], HASH_READ, + LH_BITMAP_PAGE); + freep = HashPageGetBitmap(BufferGetPage(mapbuf)); + + for (j = 0; j < nwords; j++) + { + uint32 bit; + + if (freep[j] == ALL_SET) + continue; + + if (!exclusive) + { + /* lock it exclusively and look at this word again */ + LockBuffer(mapbuf, BUFFER_LOCK_UNLOCK); + LockBuffer(mapbuf, BUFFER_LOCK_EXCLUSIVE); + exclusive = true; + if (freep[j] == ALL_SET) + continue; + } + + bit = (i << BMPG_SHIFT(metap)) + j * BITS_PER_MAP + + _hash_firstfreebit(freep[j]); + + if (metalocked && + bit >= metap->hashm_spares[metap->hashm_ovflpoint]) + elog(ERROR, "free bit %u past the last overflow page in hash index \"%s\"", + bit, RelationGetRelationName(rel)); + + if (!metalocked && + bit >= metap->hashm_spares[metap->hashm_ovflpoint]) + { + /* refresh our copy of the metapage, and retry this page */ + _hash_relbuf(rel, mapbuf); + metap = _hash_getcachedmetap(rel, metabuf, true); + *metapp = metap; + if (bit >= metap->hashm_spares[metap->hashm_ovflpoint]) + elog(ERROR, "free bit %u past the last overflow page in hash index \"%s\"", + bit, RelationGetRelationName(rel)); + break; + } + + *mapbufp = mapbuf; + *bitmap_page_bit = bit & BMPG_MASK(metap); + *bitp = bit; + + /* Fetch and init the recycled page */ + return _hash_getinitbuf(rel, bitno_to_blkno(metap, bit)); + } + + if (j < nwords) + continue; /* refreshed the metapage copy; retry page */ + + _hash_relbuf(rel, mapbuf); + i++; + } + + return InvalidBuffer; +} + /* * _hash_firstfreebit() * @@ -495,7 +633,7 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, BufferAccessStrategy bstrategy) { HashMetaPage metap; - Buffer metabuf; + Buffer metabuf = InvalidBuffer; Buffer mapbuf; BlockNumber ovflblkno; BlockNumber prevblkno; @@ -512,8 +650,7 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, Bucket bucket PG_USED_FOR_ASSERTS_ONLY; Buffer prevbuf = InvalidBuffer; Buffer nextbuf = InvalidBuffer; - bool update_metap = false, - mod_wbuf, + bool mod_wbuf, is_prim_bucket_same_wrt, is_prev_bucket_same_wrt; XLogRecPtr recptr; @@ -554,12 +691,18 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, /* Note: bstrategy is intentionally not used for metapage and bitmap */ - /* Read the metapage so we can determine which bitmap page to use */ - metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE); - metap = HashPageGetMeta(BufferGetPage(metabuf)); - - /* Identify which bit to set */ - ovflbitno = _hash_ovflblkno_to_bitno(metap, ovflblkno); + /* + * Determine which bitmap page to use from our cached copy of the + * metapage. If the page was added after the copy was taken, refresh it. + * We don't update hashm_firstfree, so we don't lock the metapage; see + * "Free Space Management" in the README. + */ + metap = _hash_getcachedmetap(rel, &metabuf, false); + if (!_hash_ovflblkno_to_bitno_internal(metap, ovflblkno, &ovflbitno)) + { + metap = _hash_getcachedmetap(rel, &metabuf, true); + ovflbitno = _hash_ovflblkno_to_bitno(metap, ovflblkno); + } bitmappage = ovflbitno >> BMPG_SHIFT(metap); bitmapbit = ovflbitno & BMPG_MASK(metap); @@ -568,18 +711,12 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, elog(ERROR, "invalid overflow bit number %u", ovflbitno); blkno = metap->hashm_mapp[bitmappage]; - /* Release metapage lock while we access the bitmap page */ - LockBuffer(metabuf, BUFFER_LOCK_UNLOCK); - /* read the bitmap page to clear the bitmap bit */ mapbuf = _hash_getbuf(rel, blkno, HASH_WRITE, LH_BITMAP_PAGE); mappage = BufferGetPage(mapbuf); freep = HashPageGetBitmap(mappage); Assert(ISSET(freep, bitmapbit)); - /* Get write-lock on metapage to update firstfree */ - LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE); - /* This operation needs to log multiple tuples, prepare WAL for that */ if (RelationNeedsWAL(rel)) XLogEnsureRecordSpace(HASH_XLOG_FREE_OVFL_BUFS, 4 + nitups); @@ -639,14 +776,6 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, CLRBIT(freep, bitmapbit); MarkBufferDirty(mapbuf); - /* if this is now the first free page, update hashm_firstfree */ - if (ovflbitno < metap->hashm_firstfree) - { - metap->hashm_firstfree = ovflbitno; - update_metap = true; - MarkBufferDirty(metabuf); - } - /* Determine which pages are modified */ is_prim_bucket_same_wrt = (wbuf == bucketbuf); is_prev_bucket_same_wrt = (wbuf == prevbuf); @@ -722,12 +851,6 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, XLogRegisterBuffer(5, mapbuf, REGBUF_STANDARD); XLogRegisterBufData(5, &bitmapbit, sizeof(uint32)); - if (update_metap) - { - XLogRegisterBuffer(6, metabuf, REGBUF_STANDARD); - XLogRegisterBufData(6, &metap->hashm_firstfree, sizeof(uint32)); - } - recptr = XLogInsert(RM_HASH_ID, XLOG_HASH_SQUEEZE_PAGE); } else /* !RelationNeedsWAL(rel) */ @@ -746,9 +869,6 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, PageSetLSN(BufferGetPage(mapbuf), recptr); - if (update_metap) - PageSetLSN(BufferGetPage(metabuf), recptr); - END_CRIT_SECTION(); /* release previous bucket if it is not same as write bucket */ @@ -762,7 +882,8 @@ _hash_freeovflpage(Relation rel, Buffer bucketbuf, Buffer ovflbuf, _hash_relbuf(rel, nextbuf); _hash_relbuf(rel, mapbuf); - _hash_relbuf(rel, metabuf); + if (BufferIsValid(metabuf)) + _hash_dropbuf(rel, metabuf); return nextblkno; } diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c index a0ab2528167..429d668cb58 100644 --- a/src/backend/access/hash/hashpage.c +++ b/src/backend/access/hash/hashpage.c @@ -38,8 +38,6 @@ #include "utils/injection_point.h" #include "utils/rel.h" -static bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock, - uint32 nblocks); static void _hash_splitbucket(Relation rel, Buffer metabuf, Bucket obucket, Bucket nbucket, Buffer obuf, @@ -975,6 +973,8 @@ fail: * EOF to the end of the splitpoint; this keeps smgr's idea of the EOF in * sync with ours, so that we don't get complaints from smgr. * + * _hash_addovflpage() uses this too, to add a batch of overflow pages. + * * We do this by writing a page of zeroes at the end of the splitpoint range. * We expect that the filesystem will ensure that the intervening pages read * as zeroes too. On many filesystems this "hole" will not be allocated @@ -991,7 +991,7 @@ fail: * Returns true if successful, or false if allocation failed due to * BlockNumber overflow. */ -static bool +bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks) { BlockNumber lastblock; diff --git a/src/backend/access/rmgrdesc/hashdesc.c b/src/backend/access/rmgrdesc/hashdesc.c index 6b410b80638..5c69006d732 100644 --- a/src/backend/access/rmgrdesc/hashdesc.c +++ b/src/backend/access/rmgrdesc/hashdesc.c @@ -64,8 +64,9 @@ hash_desc(StringInfo buf, XLogReaderState *record) { xl_hash_add_ovfl_page *xlrec = (xl_hash_add_ovfl_page *) rec; - appendStringInfo(buf, "bmsize %d, bmpage_found %c", - xlrec->bmsize, (xlrec->bmpage_found) ? 'T' : 'F'); + appendStringInfo(buf, "bmsize %d, bmpage_found %c, nadded %u", + xlrec->bmsize, (xlrec->bmpage_found) ? 'T' : 'F', + xlrec->nadded); break; } case XLOG_HASH_SPLIT_ALLOCATE_PAGE: diff --git a/src/include/access/hash.h b/src/include/access/hash.h index a8702f0e5ea..6b90ec5d98b 100644 --- a/src/include/access/hash.h +++ b/src/include/access/hash.h @@ -440,6 +440,8 @@ extern void _hash_init_metabuffer(Buffer buf, double num_tuples, RegProcedure procid, uint16 ffactor, bool initpage); extern void _hash_pageinit(Page page, Size size); extern void _hash_expandtable(Relation rel, Buffer metabuf); +extern bool _hash_alloc_buckets(Relation rel, BlockNumber firstblock, + uint32 nblocks); extern void _hash_finish_split(Relation rel, Buffer metabuf, Buffer obuf, Bucket obucket, uint32 maxbucket, uint32 highmask, uint32 lowmask); diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h index af65e5b5ce3..b0ce31cd0ce 100644 --- a/src/include/access/hash_xlog.h +++ b/src/include/access/hash_xlog.h @@ -74,13 +74,18 @@ typedef struct xl_hash_insert * * Backup Blk 0: newly allocated overflow page * Backup Blk 1: page before new overflow page in the bucket chain - * Backup Blk 2: bitmap page + * Backup Blk 2: bitmap page. If bmpage_found, data is the bit to set for + * the reused page; otherwise data is the first of the + * nadded - 1 bits to clear for the added pages left free * Backup Blk 3: new bitmap page - * Backup Blk 4: metapage + * Backup Blk 4: metapage, present only if it was modified (data is + * hashm_firstfree) */ typedef struct xl_hash_add_ovfl_page { uint16 bmsize; + uint16 nadded; /* pages added to the overflow area, if + * !bmpage_found */ bool bmpage_found; } xl_hash_add_ovfl_page; @@ -156,7 +161,7 @@ typedef struct xl_hash_move_page_contents * Backup Blk 3: page previous to the freed overflow page * Backup Blk 4: page next to the freed overflow page * Backup Blk 5: bitmap page containing info of freed overflow page - * Backup Blk 6: meta page + * Backup Blk 6: meta page (not written by current servers) */ typedef struct xl_hash_squeeze_page { diff --git a/src/test/modules/hash/Makefile b/src/test/modules/hash/Makefile index 8d8b798ebac..7d9302b1d25 100644 --- a/src/test/modules/hash/Makefile +++ b/src/test/modules/hash/Makefile @@ -2,7 +2,7 @@ EXTRA_INSTALL = src/test/modules/injection_points contrib/pageinspect -REGRESS = hash_deferred_count hash_incomplete_splits +REGRESS = hash_deferred_count hash_incomplete_splits hash_overflow_pages TAP_TESTS = 1 diff --git a/src/test/modules/hash/expected/hash_overflow_pages.out b/src/test/modules/hash/expected/hash_overflow_pages.out new file mode 100644 index 00000000000..e4868f70c02 --- /dev/null +++ b/src/test/modules/hash/expected/hash_overflow_pages.out @@ -0,0 +1,108 @@ +-- Test allocation and freeing of hash index overflow pages. +-- +-- Searching for a free overflow page and freeing one don't lock the +-- metapage. When there is no free page, the index is extended by a batch +-- of 64 overflow pages, all but the first left free in the bitmap. +CREATE EXTENSION pageinspect; +-- Size the index for 6000 rows, so the rows added below don't split it +CREATE TABLE hash_ovfl_test (v int4) WITH (autovacuum_enabled = false); +INSERT INTO hash_ovfl_test SELECT g FROM generate_series(1, 6000) g; +ANALYZE hash_ovfl_test; +CREATE INDEX hash_ovfl_index ON hash_ovfl_test USING hash (v); +-- The pages after the bitmap page are all overflow pages: their type, and +-- whether their bit is set +CREATE FUNCTION hash_ovfl_pages(OUT type text, OUT bit_set bool, OUT pages int8) +RETURNS SETOF record LANGUAGE sql AS $$ + SELECT hash_page_type(get_raw_page('hash_ovfl_index', b::int)), + (hash_bitmap_info('hash_ovfl_index', b)).bitstatus, + count(*) + FROM generate_series( + (SELECT mapp[1] FROM hash_metapage_info(get_raw_page('hash_ovfl_index', 0))) + 1, + pg_relation_size('hash_ovfl_index') / current_setting('block_size')::int - 1) b + GROUP BY 1, 2 ORDER BY 1, 2 +$$; +CREATE FUNCTION hash_ovfl_meta(OUT maxbucket int8, OUT firstfree int8, + OUT ovflpages int8, OUT index_pages int8) +LANGUAGE sql AS $$ + SELECT maxbucket, firstfree, spares[ovflpoint + 1], + pg_relation_size('hash_ovfl_index') / current_setting('block_size')::int + FROM hash_metapage_info(get_raw_page('hash_ovfl_index', 0)) +$$; +-- Only the bitmap page counts as an overflow page so far +SELECT * FROM hash_ovfl_meta(); + maxbucket | firstfree | ovflpages | index_pages +-----------+-----------+-----------+------------- + 31 | 0 | 1 | 34 +(1 row) + +SELECT * FROM hash_ovfl_pages(); + type | bit_set | pages +------+---------+------- +(0 rows) + +-- Fill one bucket past its primary page. The first overflow page extends +-- the index by 64 pages, and the others come from that batch. +INSERT INTO hash_ovfl_test SELECT 1 FROM generate_series(1, 2000); +SELECT * FROM hash_ovfl_meta(); + maxbucket | firstfree | ovflpages | index_pages +-----------+-----------+-----------+------------- + 31 | 2 | 65 | 98 +(1 row) + +SELECT * FROM hash_ovfl_pages(); + type | bit_set | pages +----------+---------+------- + overflow | t | 5 + unused | f | 59 +(2 rows) + +-- Freeing overflow pages clears their bits, and leaves firstfree alone +DELETE FROM hash_ovfl_test WHERE v = 1; +VACUUM hash_ovfl_test; +SELECT * FROM hash_ovfl_meta(); + maxbucket | firstfree | ovflpages | index_pages +-----------+-----------+-----------+------------- + 31 | 2 | 65 | 98 +(1 row) + +SELECT * FROM hash_ovfl_pages(); + type | bit_set | pages +--------+---------+------- + unused | f | 64 +(1 row) + +-- New overflow pages reuse free ones, so the index does not grow +INSERT INTO hash_ovfl_test SELECT 1 FROM generate_series(1, 1500); +SELECT * FROM hash_ovfl_meta(); + maxbucket | firstfree | ovflpages | index_pages +-----------+-----------+-----------+------------- + 31 | 2 | 65 | 98 +(1 row) + +SELECT * FROM hash_ovfl_pages(); + type | bit_set | pages +----------+---------+------- + overflow | t | 4 + unused | f | 60 +(2 rows) + +-- The index still finds every row +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM hash_ovfl_test WHERE v = 1; + count +------- + 1500 +(1 row) + +RESET enable_seqscan; +RESET enable_bitmapscan; +SELECT count(*) FROM hash_ovfl_test WHERE v = 1; + count +------- + 1500 +(1 row) + +DROP TABLE hash_ovfl_test; +DROP FUNCTION hash_ovfl_pages, hash_ovfl_meta; +DROP EXTENSION pageinspect; diff --git a/src/test/modules/hash/meson.build b/src/test/modules/hash/meson.build index 9b009607396..d46eb5a035e 100644 --- a/src/test/modules/hash/meson.build +++ b/src/test/modules/hash/meson.build @@ -12,6 +12,7 @@ tests += { 'sql': [ 'hash_deferred_count', 'hash_incomplete_splits', + 'hash_overflow_pages', ], }, 'tap': { diff --git a/src/test/modules/hash/sql/hash_overflow_pages.sql b/src/test/modules/hash/sql/hash_overflow_pages.sql new file mode 100644 index 00000000000..553b5aaf5b3 --- /dev/null +++ b/src/test/modules/hash/sql/hash_overflow_pages.sql @@ -0,0 +1,65 @@ +-- Test allocation and freeing of hash index overflow pages. +-- +-- Searching for a free overflow page and freeing one don't lock the +-- metapage. When there is no free page, the index is extended by a batch +-- of 64 overflow pages, all but the first left free in the bitmap. +CREATE EXTENSION pageinspect; + +-- Size the index for 6000 rows, so the rows added below don't split it +CREATE TABLE hash_ovfl_test (v int4) WITH (autovacuum_enabled = false); +INSERT INTO hash_ovfl_test SELECT g FROM generate_series(1, 6000) g; +ANALYZE hash_ovfl_test; +CREATE INDEX hash_ovfl_index ON hash_ovfl_test USING hash (v); + +-- The pages after the bitmap page are all overflow pages: their type, and +-- whether their bit is set +CREATE FUNCTION hash_ovfl_pages(OUT type text, OUT bit_set bool, OUT pages int8) +RETURNS SETOF record LANGUAGE sql AS $$ + SELECT hash_page_type(get_raw_page('hash_ovfl_index', b::int)), + (hash_bitmap_info('hash_ovfl_index', b)).bitstatus, + count(*) + FROM generate_series( + (SELECT mapp[1] FROM hash_metapage_info(get_raw_page('hash_ovfl_index', 0))) + 1, + pg_relation_size('hash_ovfl_index') / current_setting('block_size')::int - 1) b + GROUP BY 1, 2 ORDER BY 1, 2 +$$; +CREATE FUNCTION hash_ovfl_meta(OUT maxbucket int8, OUT firstfree int8, + OUT ovflpages int8, OUT index_pages int8) +LANGUAGE sql AS $$ + SELECT maxbucket, firstfree, spares[ovflpoint + 1], + pg_relation_size('hash_ovfl_index') / current_setting('block_size')::int + FROM hash_metapage_info(get_raw_page('hash_ovfl_index', 0)) +$$; + +-- Only the bitmap page counts as an overflow page so far +SELECT * FROM hash_ovfl_meta(); +SELECT * FROM hash_ovfl_pages(); + +-- Fill one bucket past its primary page. The first overflow page extends +-- the index by 64 pages, and the others come from that batch. +INSERT INTO hash_ovfl_test SELECT 1 FROM generate_series(1, 2000); +SELECT * FROM hash_ovfl_meta(); +SELECT * FROM hash_ovfl_pages(); + +-- Freeing overflow pages clears their bits, and leaves firstfree alone +DELETE FROM hash_ovfl_test WHERE v = 1; +VACUUM hash_ovfl_test; +SELECT * FROM hash_ovfl_meta(); +SELECT * FROM hash_ovfl_pages(); + +-- New overflow pages reuse free ones, so the index does not grow +INSERT INTO hash_ovfl_test SELECT 1 FROM generate_series(1, 1500); +SELECT * FROM hash_ovfl_meta(); +SELECT * FROM hash_ovfl_pages(); + +-- The index still finds every row +SET enable_seqscan = off; +SET enable_bitmapscan = off; +SELECT count(*) FROM hash_ovfl_test WHERE v = 1; +RESET enable_seqscan; +RESET enable_bitmapscan; +SELECT count(*) FROM hash_ovfl_test WHERE v = 1; + +DROP TABLE hash_ovfl_test; +DROP FUNCTION hash_ovfl_pages, hash_ovfl_meta; +DROP EXTENSION pageinspect; -- 2.50.1