Skip to content

Commit 22c5e73

Browse files
committed
Remove lsn from HashScanPosData.
This was intended as infrastructure for weakening VACUUM's locking requirements, similar to what was done for btree indexes in commit 2ed5b87. However, for hash indexes, it seems that the improvements which are possible are actually extremely marginal. Furthermore, performing the LSN cross-check will end up skipping cleanup far more often than is necessary; we only care about page modifications due to a VACUUM, but the LSN check will fail if ANY modification has occurred. So, rather than pressing forward with that "optimization", just rip the LSN field out. Patch by me, reviewed by Ashutosh Sharma and Amit Kapila Discussion: http://postgr.es/m/CAA4eK1JxqqcuC5Un7YLQVhOYSZBS+t=3xqZuEkt5RyquyuxpwQ@mail.gmail.com
1 parent 79a4a66 commit 22c5e73

File tree

3 files changed

+7
-30
lines changed

3 files changed

+7
-30
lines changed

src/backend/access/hash/hashsearch.c

-8
Original file line numberDiff line numberDiff line change
@@ -463,12 +463,6 @@ _hash_readpage(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
463463
opaque = (HashPageOpaque) PageGetSpecialPointer(page);
464464

465465
so->currPos.buf = buf;
466-
467-
/*
468-
* We save the LSN of the page as we read it, so that we know whether it
469-
* is safe to apply LP_DEAD hints to the page later.
470-
*/
471-
so->currPos.lsn = PageGetLSN(page);
472466
so->currPos.currPage = BufferGetBlockNumber(buf);
473467

474468
if (ScanDirectionIsForward(dir))
@@ -508,7 +502,6 @@ _hash_readpage(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
508502
{
509503
so->currPos.buf = buf;
510504
so->currPos.currPage = BufferGetBlockNumber(buf);
511-
so->currPos.lsn = PageGetLSN(page);
512505
}
513506
else
514507
{
@@ -562,7 +555,6 @@ _hash_readpage(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
562555
{
563556
so->currPos.buf = buf;
564557
so->currPos.currPage = BufferGetBlockNumber(buf);
565-
so->currPos.lsn = PageGetLSN(page);
566558
}
567559
else
568560
{

src/backend/access/hash/hashutil.c

+7-20
Original file line numberDiff line numberDiff line change
@@ -532,12 +532,13 @@ _hash_get_newbucket_from_oldbucket(Relation rel, Bucket old_bucket,
532532
* We match items by heap TID before assuming they are the right ones to
533533
* delete.
534534
*
535-
* Note that we keep the pin on the bucket page throughout the scan. Hence,
536-
* there is no chance of VACUUM deleting any items from that page. However,
537-
* having pin on the overflow page doesn't guarantee that vacuum won't delete
538-
* any items.
539-
*
540-
* See _bt_killitems() for more details.
535+
* There are never any scans active in a bucket at the time VACUUM begins,
536+
* because VACUUM takes a cleanup lock on the primary bucket page and scans
537+
* hold a pin. A scan can begin after VACUUM leaves the primary bucket page
538+
* but before it finishes the entire bucket, but it can never pass VACUUM,
539+
* because VACUUM always locks the next page before releasing the lock on
540+
* the previous one. Therefore, we don't have to worry about accidentally
541+
* killing a TID that has been reused for an unrelated tuple.
541542
*/
542543
void
543544
_hash_kill_items(IndexScanDesc scan)
@@ -579,21 +580,7 @@ _hash_kill_items(IndexScanDesc scan)
579580
else
580581
buf = _hash_getbuf(rel, blkno, HASH_READ, LH_OVERFLOW_PAGE);
581582

582-
/*
583-
* If page LSN differs it means that the page was modified since the last
584-
* read. killedItems could be not valid so applying LP_DEAD hints is not
585-
* safe.
586-
*/
587583
page = BufferGetPage(buf);
588-
if (PageGetLSN(page) != so->currPos.lsn)
589-
{
590-
if (havePin)
591-
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
592-
else
593-
_hash_relbuf(rel, buf);
594-
return;
595-
}
596-
597584
opaque = (HashPageOpaque) PageGetSpecialPointer(page);
598585
maxoff = PageGetMaxOffsetNumber(page);
599586

src/include/access/hash.h

-2
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ typedef struct HashScanPosItem /* what we remember about each match */
117117
typedef struct HashScanPosData
118118
{
119119
Buffer buf; /* if valid, the buffer is pinned */
120-
XLogRecPtr lsn; /* pos in the WAL stream when page was read */
121120
BlockNumber currPage; /* current hash index page */
122121
BlockNumber nextPage; /* next overflow page */
123122
BlockNumber prevPage; /* prev overflow or bucket page */
@@ -153,7 +152,6 @@ typedef struct HashScanPosData
153152
#define HashScanPosInvalidate(scanpos) \
154153
do { \
155154
(scanpos).buf = InvalidBuffer; \
156-
(scanpos).lsn = InvalidXLogRecPtr; \
157155
(scanpos).currPage = InvalidBlockNumber; \
158156
(scanpos).nextPage = InvalidBlockNumber; \
159157
(scanpos).prevPage = InvalidBlockNumber; \

0 commit comments

Comments
 (0)