From 953477ca3526e28f9aeeb41d23b16eed0084c7d2 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Mon, 20 Mar 2017 15:49:09 -0400 Subject: [PATCH] Fixes for single-page hash index vacuum. Clear LH_PAGE_HAS_DEAD_TUPLES during replay, similar to what gets done for btree. Update hashdesc.c for xl_hash_vacuum_one_page. Oversights in commit 6977b8b7f4dfb40896ff5e2175cad7fdbda862eb spotted by Amit Kapila. Patch by Ashutosh Sharma. Bump WAL version. The original patch to make hash indexes write-ahead logged probably should have done this, and the single page vacuuming patch probably should have done it again, but better late than never. Discussion: http://postgr.es/m/CAA4eK1Kd=mJ9xreovcsh0qMiAj-QqCphHVQ_Lfau1DR9oVjASQ@mail.gmail.com --- src/backend/access/hash/hash.c | 7 ++++++- src/backend/access/hash/hash_xlog.c | 21 +++++++++++++++++++++ src/backend/access/hash/hashinsert.c | 8 ++++++++ src/backend/access/rmgrdesc/hashdesc.c | 11 ++++++++++- src/include/access/hash_xlog.h | 2 ++ src/include/access/xlog_internal.h | 2 +- 6 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c index cfcec3475d4..34cc08f12d2 100644 --- a/src/backend/access/hash/hash.c +++ b/src/backend/access/hash/hash.c @@ -790,6 +790,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf, OffsetNumber deletable[MaxOffsetNumber]; int ndeletable = 0; bool retain_pin = false; + bool clear_dead_marking = false; vacuum_delay_point(); @@ -877,11 +878,14 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf, /* * Let us mark the page as clean if vacuum removes the DEAD tuples * from an index page. We do this by clearing LH_PAGE_HAS_DEAD_TUPLES - * flag. Clearing this flag is just a hint; replay won't redo this. + * flag. */ if (tuples_removed && *tuples_removed > 0 && opaque->hasho_flag & LH_PAGE_HAS_DEAD_TUPLES) + { opaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES; + clear_dead_marking = true; + } MarkBufferDirty(buf); @@ -891,6 +895,7 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf, xl_hash_delete xlrec; XLogRecPtr recptr; + xlrec.clear_dead_marking = clear_dead_marking; xlrec.is_primary_bucket_page = (buf == bucket_buf) ? true : false; XLogBeginInsert(); diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c index 8647e8c6adc..ac82092ab24 100644 --- a/src/backend/access/hash/hash_xlog.c +++ b/src/backend/access/hash/hash_xlog.c @@ -859,6 +859,19 @@ hash_xlog_delete(XLogReaderState *record) PageIndexMultiDelete(page, unused, unend - unused); } + /* + * Mark the page as not containing any LP_DEAD items only if + * clear_dead_marking flag is set to true. See comments in + * hashbucketcleanup() for details. + */ + if (xldata->clear_dead_marking) + { + HashPageOpaque pageopaque; + + pageopaque = (HashPageOpaque) PageGetSpecialPointer(page); + pageopaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES; + } + PageSetLSN(page, lsn); MarkBufferDirty(deletebuf); } @@ -1078,6 +1091,7 @@ hash_xlog_vacuum_one_page(XLogReaderState *record) Buffer metabuf; Page page; XLogRedoAction action; + HashPageOpaque pageopaque; xldata = (xl_hash_vacuum_one_page *) XLogRecGetData(record); @@ -1126,6 +1140,13 @@ hash_xlog_vacuum_one_page(XLogReaderState *record) PageIndexMultiDelete(page, unused, unend - unused); } + /* + * Mark the page as not containing any LP_DEAD items. See comments + * in _hash_vacuum_one_page() for details. + */ + pageopaque = (HashPageOpaque) PageGetSpecialPointer(page); + pageopaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES; + PageSetLSN(page, lsn); MarkBufferDirty(buffer); } diff --git a/src/backend/access/hash/hashinsert.c b/src/backend/access/hash/hashinsert.c index 8b6d0a0ff78..8640e85a5c6 100644 --- a/src/backend/access/hash/hashinsert.c +++ b/src/backend/access/hash/hashinsert.c @@ -374,6 +374,14 @@ _hash_vacuum_one_page(Relation rel, Buffer metabuf, Buffer buf, PageIndexMultiDelete(page, deletable, ndeletable); + /* + * Mark the page as not containing any LP_DEAD items. This is not + * certainly true (there might be some that have recently been marked, + * but weren't included in our target-item list), but it will almost + * always be true and it doesn't seem worth an additional page scan + * to check it. Remember that LH_PAGE_HAS_DEAD_TUPLES is only a hint + * anyway. + */ pageopaque = (HashPageOpaque) PageGetSpecialPointer(page); pageopaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES; diff --git a/src/backend/access/rmgrdesc/hashdesc.c b/src/backend/access/rmgrdesc/hashdesc.c index 5bd5c8dc010..5f5f4a02551 100644 --- a/src/backend/access/rmgrdesc/hashdesc.c +++ b/src/backend/access/rmgrdesc/hashdesc.c @@ -96,7 +96,8 @@ hash_desc(StringInfo buf, XLogReaderState *record) { xl_hash_delete *xlrec = (xl_hash_delete *) rec; - appendStringInfo(buf, "is_primary %c", + appendStringInfo(buf, "clear_dead_marking %c, is_primary %c", + xlrec->clear_dead_marking ? 'T' : 'F', xlrec->is_primary_bucket_page ? 'T' : 'F'); break; } @@ -104,6 +105,14 @@ hash_desc(StringInfo buf, XLogReaderState *record) { xl_hash_update_meta_page *xlrec = (xl_hash_update_meta_page *) rec; + appendStringInfo(buf, "ntuples %g", + xlrec->ntuples); + break; + } + case XLOG_HASH_VACUUM_ONE_PAGE: + { + xl_hash_vacuum_one_page *xlrec = (xl_hash_vacuum_one_page *) rec; + appendStringInfo(buf, "ntuples %g", xlrec->ntuples); break; diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h index dfd92378199..2e64cfa3eaf 100644 --- a/src/include/access/hash_xlog.h +++ b/src/include/access/hash_xlog.h @@ -197,6 +197,8 @@ typedef struct xl_hash_squeeze_page */ typedef struct xl_hash_delete { + bool clear_dead_marking; /* TRUE if this operation clears + * LH_PAGE_HAS_DEAD_TUPLES flag */ bool is_primary_bucket_page; /* TRUE if the operation is for * primary bucket page */ } xl_hash_delete; diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index b8b15f18752..7957cab98c6 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -31,7 +31,7 @@ /* * Each page of XLOG file has a header like this: */ -#define XLOG_PAGE_MAGIC 0xD095 /* can be used as WAL version indicator */ +#define XLOG_PAGE_MAGIC 0xD096 /* can be used as WAL version indicator */ typedef struct XLogPageHeaderData {