Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit8f3c577

Browse files
committed
Fix bogus concurrent use of _hash_getnewbuf() in bucket split code.
_hash_splitbucket() obtained the base page of the new bucket by calling_hash_getnewbuf(), but it held no exclusive lock that would prevent someother process from calling _hash_getnewbuf() at the same time. This iscontrary to _hash_getnewbuf()'s API spec and could in fact cause failures.In practice, we must only call that function while holding write lock onthe hash index's metapage.An additional problem was that we'd already modified the metapage's bucketmapping data, meaning that failure to extend the index would leave us witha corrupt index.Fix both issues by moving the _hash_getnewbuf() call to just before wemodify the metapage in _hash_expandtable().Unfortunately there's still a large problem here, which is that we couldalso incur ENOSPC while trying to get an overflow page for the new bucket.That would leave the index corrupt in a more subtle way, namely that someindex tuples that should be in the new bucket might still be in the oldone. Fixing that seems substantially more difficult; even preallocating asmany pages as we could possibly need wouldn't entirely guarantee that thebucket split would complete successfully. So for today let's just dealwith the base case.Per report from Antonin Houska. Back-patch to all active branches.
1 parent152c946 commit8f3c577

File tree

1 file changed

+26
-4
lines changed

1 file changed

+26
-4
lines changed

‎src/backend/access/hash/hashpage.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
staticbool_hash_alloc_buckets(Relationrel,BlockNumberfirstblock,
4141
uint32nblocks);
4242
staticvoid_hash_splitbucket(Relationrel,Buffermetabuf,
43+
Buffernbuf,
4344
Bucketobucket,Bucketnbucket,
4445
BlockNumberstart_oblkno,
4546
BlockNumberstart_nblkno,
@@ -179,7 +180,9 @@ _hash_getinitbuf(Relation rel, BlockNumber blkno)
179180
*EOF but before updating the metapage to reflect the added page.)
180181
*
181182
*It is caller's responsibility to ensure that only one process can
182-
*extend the index at a time.
183+
*extend the index at a time. In practice, this function is called
184+
*only while holding write lock on the metapage, because adding a page
185+
*is always associated with an update of metapage data.
183186
*/
184187
Buffer
185188
_hash_getnewbuf(Relationrel,BlockNumberblkno)
@@ -506,6 +509,7 @@ _hash_expandtable(Relation rel, Buffer metabuf)
506509
uint32spare_ndx;
507510
BlockNumberstart_oblkno;
508511
BlockNumberstart_nblkno;
512+
Bufferbuf_nblkno;
509513
uint32maxbucket;
510514
uint32highmask;
511515
uint32lowmask;
@@ -618,6 +622,13 @@ _hash_expandtable(Relation rel, Buffer metabuf)
618622
}
619623
}
620624

625+
/*
626+
* Physically allocate the new bucket's primary page. We want to do this
627+
* before changing the metapage's mapping info, in case we can't get the
628+
* disk space.
629+
*/
630+
buf_nblkno=_hash_getnewbuf(rel,start_nblkno);
631+
621632
/*
622633
* Okay to proceed with split. Update the metapage bucket mapping info.
623634
*
@@ -671,7 +682,8 @@ _hash_expandtable(Relation rel, Buffer metabuf)
671682
_hash_droplock(rel,0,HASH_EXCLUSIVE);
672683

673684
/* Relocate records to the new bucket */
674-
_hash_splitbucket(rel,metabuf,old_bucket,new_bucket,
685+
_hash_splitbucket(rel,metabuf,buf_nblkno,
686+
old_bucket,new_bucket,
675687
start_oblkno,start_nblkno,
676688
maxbucket,highmask,lowmask);
677689

@@ -754,10 +766,16 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
754766
* The caller must hold a pin, but no lock, on the metapage buffer.
755767
* The buffer is returned in the same state. (The metapage is only
756768
* touched if it becomes necessary to add or remove overflow pages.)
769+
*
770+
* In addition, the caller must have created the new bucket's base page,
771+
* which is passed in buffer nbuf, pinned and write-locked. The lock
772+
* and pin are released here. (The API is set up this way because we must
773+
* do _hash_getnewbuf() before releasing the metapage write lock.)
757774
*/
758775
staticvoid
759776
_hash_splitbucket(Relationrel,
760777
Buffermetabuf,
778+
Buffernbuf,
761779
Bucketobucket,
762780
Bucketnbucket,
763781
BlockNumberstart_oblkno,
@@ -769,7 +787,6 @@ _hash_splitbucket(Relation rel,
769787
BlockNumberoblkno;
770788
BlockNumbernblkno;
771789
Bufferobuf;
772-
Buffernbuf;
773790
Pageopage;
774791
Pagenpage;
775792
HashPageOpaqueoopaque;
@@ -786,7 +803,7 @@ _hash_splitbucket(Relation rel,
786803
oopaque= (HashPageOpaque)PageGetSpecialPointer(opage);
787804

788805
nblkno=start_nblkno;
789-
nbuf=_hash_getnewbuf(rel,nblkno);
806+
Assert(nblkno==BufferGetBlockNumber(nbuf));
790807
npage=BufferGetPage(nbuf);
791808

792809
/* initialize the new bucket's primary page */
@@ -835,6 +852,11 @@ _hash_splitbucket(Relation rel,
835852
* insert the tuple into the new bucket. if it doesn't fit on
836853
* the current page in the new bucket, we must allocate a new
837854
* overflow page and place the tuple on that page instead.
855+
*
856+
* XXX we have a problem here if we fail to get space for a
857+
* new overflow page: we'll error out leaving the bucket split
858+
* only partially complete, meaning the index is corrupt,
859+
* since searches may fail to find entries they should find.
838860
*/
839861
itemsz=IndexTupleDSize(*itup);
840862
itemsz=MAXALIGN(itemsz);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp