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

Commit25216c9

Browse files
committed
Remove _hash_wrtbuf() in favor of calling MarkBufferDirty().
The whole concept of _hash_wrtbuf() is that we need to know at thetime we're releasing the buffer lock (and pin) whether we dirtied thebuffer, but this is easy to get wrong. This patch actually fixes onenon-obvious bug of that form: hashbucketcleanup forgot to signal_hash_squeezebucket, which gets the primary bucket page alreadylocked, as to whether it had already dirtied the page. CallingMarkBufferDirty() at the places where we dirty the buffer is moreintuitive and lets us simplify the code in various places as well.On top of all that, the ultimate goal here is to make hash indexesWAL-logged, and as the comments to _hash_wrtbuf() note, it shouldgo away when that happens. Making it go away a little earlier thanthat seems like a good preparatory step.Report by Jeff Janes. Diagnosis by Amit Kapila, Kuntal Ghosh,and Dilip Kumar. Patch by me, after studying an alternative patchsubmitted by Amit Kapila.Discussion:http://postgr.es/m/CAA4eK1Kf6tOY0oVz_SEdngiNFkeXrA3xUSDPPORQvsWVPdKqnA@mail.gmail.com
1 parent4f5182e commit25216c9

File tree

5 files changed

+41
-63
lines changed

5 files changed

+41
-63
lines changed

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

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,8 @@ hashbulkdelete(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
635635
num_index_tuples=metap->hashm_ntuples;
636636
}
637637

638-
_hash_wrtbuf(rel,metabuf);
638+
MarkBufferDirty(metabuf);
639+
_hash_relbuf(rel,metabuf);
639640

640641
/* return statistics */
641642
if (stats==NULL)
@@ -724,7 +725,6 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
724725
OffsetNumberdeletable[MaxOffsetNumber];
725726
intndeletable=0;
726727
boolretain_pin= false;
727-
boolcurr_page_dirty= false;
728728

729729
vacuum_delay_point();
730730

@@ -805,7 +805,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
805805
{
806806
PageIndexMultiDelete(page,deletable,ndeletable);
807807
bucket_dirty= true;
808-
curr_page_dirty= true;
808+
MarkBufferDirty(buf);
809809
}
810810

811811
/* bail out if there are no more pages to scan. */
@@ -820,15 +820,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
820820
* release the lock on previous page after acquiring the lock on next
821821
* page
822822
*/
823-
if (curr_page_dirty)
824-
{
825-
if (retain_pin)
826-
_hash_chgbufaccess(rel,buf,HASH_WRITE,HASH_NOLOCK);
827-
else
828-
_hash_wrtbuf(rel,buf);
829-
curr_page_dirty= false;
830-
}
831-
elseif (retain_pin)
823+
if (retain_pin)
832824
_hash_chgbufaccess(rel,buf,HASH_READ,HASH_NOLOCK);
833825
else
834826
_hash_relbuf(rel,buf);
@@ -862,6 +854,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
862854
bucket_opaque= (HashPageOpaque)PageGetSpecialPointer(page);
863855

864856
bucket_opaque->hasho_flag &= ~LH_BUCKET_NEEDS_SPLIT_CLEANUP;
857+
MarkBufferDirty(bucket_buf);
865858
}
866859

867860
/*
@@ -873,7 +866,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
873866
_hash_squeezebucket(rel,cur_bucket,bucket_blkno,bucket_buf,
874867
bstrategy);
875868
else
876-
_hash_chgbufaccess(rel,bucket_buf,HASH_WRITE,HASH_NOLOCK);
869+
_hash_chgbufaccess(rel,bucket_buf,HASH_READ,HASH_NOLOCK);
877870
}
878871

879872
void

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,12 @@ _hash_doinsert(Relation rel, IndexTuple itup)
208208
(void)_hash_pgaddtup(rel,buf,itemsz,itup);
209209

210210
/*
211-
*write and release the modified page. if the page we modified was an
211+
*dirty and release the modified page. if the page we modified was an
212212
* overflow page, we also need to separately drop the pin we retained on
213213
* the primary bucket page.
214214
*/
215-
_hash_wrtbuf(rel,buf);
215+
MarkBufferDirty(buf);
216+
_hash_relbuf(rel,buf);
216217
if (buf!=bucket_buf)
217218
_hash_dropbuf(rel,bucket_buf);
218219

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

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,11 @@ _hash_addovflpage(Relation rel, Buffer metabuf, Buffer buf, bool retain_pin)
149149

150150
/* logically chain overflow page to previous page */
151151
pageopaque->hasho_nextblkno=BufferGetBlockNumber(ovflbuf);
152+
MarkBufferDirty(buf);
152153
if ((pageopaque->hasho_flag&LH_BUCKET_PAGE)&&retain_pin)
153-
_hash_chgbufaccess(rel,buf,HASH_WRITE,HASH_NOLOCK);
154+
_hash_chgbufaccess(rel,buf,HASH_READ,HASH_NOLOCK);
154155
else
155-
_hash_wrtbuf(rel,buf);
156+
_hash_relbuf(rel,buf);
156157

157158
returnovflbuf;
158159
}
@@ -304,7 +305,8 @@ _hash_getovflpage(Relation rel, Buffer metabuf)
304305

305306
/* mark page "in use" in the bitmap */
306307
SETBIT(freep,bit);
307-
_hash_wrtbuf(rel,mapbuf);
308+
MarkBufferDirty(mapbuf);
309+
_hash_relbuf(rel,mapbuf);
308310

309311
/* Reacquire exclusive lock on the meta page */
310312
_hash_chgbufaccess(rel,metabuf,HASH_NOLOCK,HASH_WRITE);
@@ -416,7 +418,8 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf, Buffer wbuf,
416418
* in _hash_pageinit() when the page is reused.)
417419
*/
418420
MemSet(ovflpage,0,BufferGetPageSize(ovflbuf));
419-
_hash_wrtbuf(rel,ovflbuf);
421+
MarkBufferDirty(ovflbuf);
422+
_hash_relbuf(rel,ovflbuf);
420423

421424
/*
422425
* Fix up the bucket chain. this is a doubly-linked list, so we must fix
@@ -445,7 +448,10 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf, Buffer wbuf,
445448
prevopaque->hasho_nextblkno=nextblkno;
446449

447450
if (prevblkno!=writeblkno)
448-
_hash_wrtbuf(rel,prevbuf);
451+
{
452+
MarkBufferDirty(prevbuf);
453+
_hash_relbuf(rel,prevbuf);
454+
}
449455
}
450456

451457
/* write and unlock the write buffer */
@@ -466,7 +472,8 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf, Buffer wbuf,
466472

467473
Assert(nextopaque->hasho_bucket==bucket);
468474
nextopaque->hasho_prevblkno=prevblkno;
469-
_hash_wrtbuf(rel,nextbuf);
475+
MarkBufferDirty(nextbuf);
476+
_hash_relbuf(rel,nextbuf);
470477
}
471478

472479
/* Note: bstrategy is intentionally not used for metapage and bitmap */
@@ -494,7 +501,8 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf, Buffer wbuf,
494501
freep=HashPageGetBitmap(mappage);
495502
Assert(ISSET(freep,bitmapbit));
496503
CLRBIT(freep,bitmapbit);
497-
_hash_wrtbuf(rel,mapbuf);
504+
MarkBufferDirty(mapbuf);
505+
_hash_relbuf(rel,mapbuf);
498506

499507
/* Get write-lock on metapage to update firstfree */
500508
_hash_chgbufaccess(rel,metabuf,HASH_NOLOCK,HASH_WRITE);
@@ -503,13 +511,9 @@ _hash_freeovflpage(Relation rel, Buffer ovflbuf, Buffer wbuf,
503511
if (ovflbitno<metap->hashm_firstfree)
504512
{
505513
metap->hashm_firstfree=ovflbitno;
506-
_hash_wrtbuf(rel,metabuf);
507-
}
508-
else
509-
{
510-
/* no need to change metapage */
511-
_hash_relbuf(rel,metabuf);
514+
MarkBufferDirty(metabuf);
512515
}
516+
_hash_relbuf(rel,metabuf);
513517

514518
returnnextblkno;
515519
}
@@ -559,8 +563,9 @@ _hash_initbitmap(Relation rel, HashMetaPage metap, BlockNumber blkno,
559563
freep=HashPageGetBitmap(pg);
560564
MemSet(freep,0xFF,BMPGSZ_BYTE(metap));
561565

562-
/* write out the new bitmap page (releasing write lock and pin) */
563-
_hash_wrtbuf(rel,buf);
566+
/* dirty the new bitmap page, and release write lock and pin */
567+
MarkBufferDirty(buf);
568+
_hash_relbuf(rel,buf);
564569

565570
/* add the new bitmap page to the metapage's list of bitmaps */
566571
/* metapage already has a write lock */
@@ -724,13 +729,8 @@ _hash_squeezebucket(Relation rel,
724729
* on next page
725730
*/
726731
if (wbuf_dirty)
727-
{
728-
if (retain_pin)
729-
_hash_chgbufaccess(rel,wbuf,HASH_WRITE,HASH_NOLOCK);
730-
else
731-
_hash_wrtbuf(rel,wbuf);
732-
}
733-
elseif (retain_pin)
732+
MarkBufferDirty(wbuf);
733+
if (retain_pin)
734734
_hash_chgbufaccess(rel,wbuf,HASH_READ,HASH_NOLOCK);
735735
else
736736
_hash_relbuf(rel,wbuf);
@@ -742,10 +742,9 @@ _hash_squeezebucket(Relation rel,
742742
{
743743
/* Delete tuples we already moved off read page */
744744
PageIndexMultiDelete(rpage,deletable,ndeletable);
745-
_hash_wrtbuf(rel,rbuf);
745+
MarkBufferDirty(rbuf);
746746
}
747-
else
748-
_hash_relbuf(rel,rbuf);
747+
_hash_relbuf(rel,rbuf);
749748
return;
750749
}
751750

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

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -289,25 +289,6 @@ _hash_dropscanbuf(Relation rel, HashScanOpaque so)
289289
so->hashso_buc_split= false;
290290
}
291291

292-
/*
293-
*_hash_wrtbuf() -- write a hash page to disk.
294-
*
295-
*This routine releases the lock held on the buffer and our refcount
296-
*for it. It is an error to call _hash_wrtbuf() without a write lock
297-
*and a pin on the buffer.
298-
*
299-
* NOTE: this routine should go away when/if hash indexes are WAL-ified.
300-
* The correct sequence of operations is to mark the buffer dirty, then
301-
* write the WAL record, then release the lock and pin; so marking dirty
302-
* can't be combined with releasing.
303-
*/
304-
void
305-
_hash_wrtbuf(Relationrel,Bufferbuf)
306-
{
307-
MarkBufferDirty(buf);
308-
UnlockReleaseBuffer(buf);
309-
}
310-
311292
/*
312293
* _hash_chgbufaccess() -- Change the lock type on a buffer, without
313294
*dropping our pin on it.
@@ -483,7 +464,8 @@ _hash_metapinit(Relation rel, double num_tuples, ForkNumber forkNum)
483464
pageopaque->hasho_bucket=i;
484465
pageopaque->hasho_flag=LH_BUCKET_PAGE;
485466
pageopaque->hasho_page_id=HASHO_PAGE_ID;
486-
_hash_wrtbuf(rel,buf);
467+
MarkBufferDirty(buf);
468+
_hash_relbuf(rel,buf);
487469
}
488470

489471
/* Now reacquire buffer lock on metapage */
@@ -495,7 +477,8 @@ _hash_metapinit(Relation rel, double num_tuples, ForkNumber forkNum)
495477
_hash_initbitmap(rel,metap,num_buckets+1,forkNum);
496478

497479
/* all done */
498-
_hash_wrtbuf(rel,metabuf);
480+
MarkBufferDirty(metabuf);
481+
_hash_relbuf(rel,metabuf);
499482

500483
returnnum_buckets;
501484
}
@@ -1075,7 +1058,10 @@ _hash_splitbucket_guts(Relation rel,
10751058
if (nbuf==bucket_nbuf)
10761059
_hash_chgbufaccess(rel,bucket_nbuf,HASH_WRITE,HASH_NOLOCK);
10771060
else
1078-
_hash_wrtbuf(rel,nbuf);
1061+
{
1062+
MarkBufferDirty(nbuf);
1063+
_hash_relbuf(rel,nbuf);
1064+
}
10791065

10801066
_hash_chgbufaccess(rel,bucket_obuf,HASH_NOLOCK,HASH_WRITE);
10811067
opage=BufferGetPage(bucket_obuf);

‎src/include/access/hash.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,6 @@ extern Buffer _hash_getbuf_with_strategy(Relation rel, BlockNumber blkno,
336336
externvoid_hash_relbuf(Relationrel,Bufferbuf);
337337
externvoid_hash_dropbuf(Relationrel,Bufferbuf);
338338
externvoid_hash_dropscanbuf(Relationrel,HashScanOpaqueso);
339-
externvoid_hash_wrtbuf(Relationrel,Bufferbuf);
340339
externvoid_hash_chgbufaccess(Relationrel,Bufferbuf,intfrom_access,
341340
intto_access);
342341
externuint32_hash_metapinit(Relationrel,doublenum_tuples,

0 commit comments

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