Commit 501c7b94 authored by Robert Haas's avatar Robert Haas

Fix bug in hashbulkdelete.

Commit 6d46f478 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.
parent a2566508
...@@ -523,7 +523,8 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats, ...@@ -523,7 +523,8 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
orig_maxbucket = metap->hashm_maxbucket; orig_maxbucket = metap->hashm_maxbucket;
orig_ntuples = metap->hashm_ntuples; orig_ntuples = metap->hashm_ntuples;
memcpy(&local_metapage, metap, sizeof(local_metapage)); 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 */ /* Scan the buckets that we know exist */
cur_bucket = 0; cur_bucket = 0;
...@@ -563,8 +564,23 @@ loop_top: ...@@ -563,8 +564,23 @@ loop_top:
*/ */
if (!H_BUCKET_BEING_SPLIT(bucket_opaque) && if (!H_BUCKET_BEING_SPLIT(bucket_opaque) &&
H_NEEDS_SPLIT_CLEANUP(bucket_opaque)) H_NEEDS_SPLIT_CLEANUP(bucket_opaque))
{
split_cleanup = true; 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; bucket_buf = buf;
hashbucketcleanup(rel, cur_bucket, bucket_buf, blkno, info->strategy, hashbucketcleanup(rel, cur_bucket, bucket_buf, blkno, info->strategy,
...@@ -581,7 +597,7 @@ loop_top: ...@@ -581,7 +597,7 @@ loop_top:
} }
/* Write-lock metapage and check for split since we started */ /* 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)); metap = HashPageGetMeta(BufferGetPage(metabuf));
if (cur_maxbucket != metap->hashm_maxbucket) if (cur_maxbucket != metap->hashm_maxbucket)
...@@ -589,7 +605,7 @@ loop_top: ...@@ -589,7 +605,7 @@ loop_top:
/* There's been a split, so process the additional bucket(s) */ /* There's been a split, so process the additional bucket(s) */
cur_maxbucket = metap->hashm_maxbucket; cur_maxbucket = metap->hashm_maxbucket;
memcpy(&local_metapage, metap, sizeof(local_metapage)); memcpy(&local_metapage, metap, sizeof(local_metapage));
_hash_relbuf(rel, metabuf); _hash_chgbufaccess(rel, metabuf, HASH_READ, HASH_NOLOCK);
goto loop_top; goto loop_top;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment