From 501c7b94bcb00cfa0faad60135cf6af82fd56a3a Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Tue, 13 Dec 2016 12:16:02 -0500 Subject: [PATCH] Fix bug in hashbulkdelete. Commit 6d46f4783efe457f74816a75173eb23ed8930020 failed to account for the possibility that hashbulkdelete() might encounter a bucket that has been split since it began scanning the bucket array. Repair. Extracted from a larger pathc by Amit Kapila; I rewrote the comment. --- src/backend/access/hash/hash.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index 6806e327ec..f1511d0d82 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -523,7 +523,8 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, orig_maxbucket = metap->hashm_maxbucket; orig_ntuples = metap->hashm_ntuples; memcpy(&local_metapage, metap, sizeof(local_metapage)); - _hash_relbuf(rel, metabuf); + /* release the lock, but keep pin */ + _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK); /* Scan the buckets that we know exist */ cur_bucket = 0; @@ -563,8 +564,23 @@ loop_top: */ if (!H_BUCKET_BEING_SPLIT(bucket_opaque) && H_NEEDS_SPLIT_CLEANUP(bucket_opaque)) + { split_cleanup = true; + /* + * This bucket might have been split since we last held a lock on + * the metapage. If so, hashm_maxbucket, hashm_highmask and + * hashm_lowmask might be old enough to cause us to fail to remove + * tuples left behind by the most recent split. To prevent that, + * now that the primary page of the target bucket has been locked + * (and thus can't be further split), update our cached metapage + * data. + */ + _hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_READ); + memcpy(&local_metapage, metap, sizeof(local_metapage)); + _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK); + } + bucket_buf = buf; hashbucketcleanup(rel, cur_bucket, bucket_buf, blkno, info->strategy, @@ -581,7 +597,7 @@ loop_top: } /* Write-lock metapage and check for split since we started */ - metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_WRITE, LH_META_PAGE); + _hash_chgbufaccess(rel, metabuf, HASH_NOLOCK, HASH_WRITE); metap = HashPageGetMeta(BufferGetPage(metabuf)); if (cur_maxbucket != metap->hashm_maxbucket) @@ -589,7 +605,7 @@ loop_top: /* There's been a split, so process the additional bucket(s) */ cur_maxbucket = metap->hashm_maxbucket; memcpy(&local_metapage, metap, sizeof(local_metapage)); - _hash_relbuf(rel, metabuf); + _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK); goto loop_top; } -- 2.40.0