WAL Consistency checking for hash indexes

Started by Kuntal Ghoshalmost 9 years ago12 messages

kuntalghosh.2007@gmail.com

almost 9 years ago

1 attachment(s)

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.

Note that, this patch should be applied on top of the following WAL
logging for hash index patch:
/messages/by-id/CAA4eK1++P+jVZC7MC3xzw5uLCpva9+gsBpd3semuWffWAftr5Q@mail.gmail.com

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Attachments:

0001-wal_consistency_checking-for-hash-index.patchapplication/x-download; name=0001-wal_consistency_checking-for-hash-index.patchDownload

From ee59bb8639459373080b3b30904aca4484857bb0 Mon Sep 17 00:00:00 2001
From: Kuntal Ghosh <kuntal.ghosh@enterprisedb.com>
Date: Tue, 28 Feb 2017 10:51:27 +0530
Subject: [PATCH] wal_consistency_checking for hash index

This patch implements the feature of wal consistency checking for
hash index. It includes a hash_mask function to mask the hash pages
before performing any consistency check.
---
 src/backend/access/hash/hash_xlog.c | 37 +++++++++++++++++++++++++++++++++++++
 src/include/access/hash_xlog.h      |  1 +
 src/include/access/rmgrlist.h       |  2 +-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index aff9b53..9c86036 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "access/bufmask.h"
 #include "access/hash.h"
 #include "access/hash_xlog.h"
 #include "access/xlogutils.h"
@@ -972,3 +973,39 @@ hash_redo(XLogReaderState *record)
 			elog(PANIC, "hash_redo: unknown op code %u", info);
 	}
 }
+
+/*
+ * Mask a Hash page before performing consistency checks on it.
+ */
+void
+hash_mask(char *pagedata, BlockNumber blkno)
+{
+	Page		page = (Page) pagedata;
+	HashPageOpaque		opaque;
+
+	mask_page_lsn(page);
+
+	mask_page_hint_bits(page);
+	mask_unused_space(page);
+
+	opaque = (HashPageOpaque) PageGetSpecialPointer(page);
+
+	if (opaque->hasho_flag & LH_UNUSED_PAGE)
+	{
+		/*
+		 * Mask everything on a UNUSED page.
+		 */
+		mask_page_content(page);
+	}
+	else if ((opaque->hasho_flag & LH_BUCKET_PAGE) ||
+						(opaque->hasho_flag & LH_OVERFLOW_PAGE))
+	{
+		/*
+		 * In btree bucket and overflow pages, it is possible to modify the
+		 * LP_FLAGS without emitting any WAL record. Hence, mask the line
+		 * pointer flags.
+		 * See hashgettuple() for details.
+		 */
+		mask_lp_flags(page);
+	}
+}
diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h
index 8400022..6620dee 100644
--- a/src/include/access/hash_xlog.h
+++ b/src/include/access/hash_xlog.h
@@ -253,5 +253,6 @@ typedef struct xl_hash_init_bitmap_page
 extern void hash_redo(XLogReaderState *record);
 extern void hash_desc(StringInfo buf, XLogReaderState *record);
 extern const char *hash_identify(uint8 info);
+extern void hash_mask(char *pagedata, BlockNumber blkno);
 
 #endif   /* HASH_XLOG_H */
diff --git a/src/include/access/rmgrlist.h b/src/include/access/rmgrlist.h
index b892aea..2f43c19 100644
--- a/src/include/access/rmgrlist.h
+++ b/src/include/access/rmgrlist.h
@@ -37,7 +37,7 @@ PG_RMGR(RM_STANDBY_ID, "Standby", standby_redo, standby_desc, standby_identify,
 PG_RMGR(RM_HEAP2_ID, "Heap2", heap2_redo, heap2_desc, heap2_identify, NULL, NULL, heap_mask)
 PG_RMGR(RM_HEAP_ID, "Heap", heap_redo, heap_desc, heap_identify, NULL, NULL, heap_mask)
 PG_RMGR(RM_BTREE_ID, "Btree", btree_redo, btree_desc, btree_identify, NULL, NULL, btree_mask)
-PG_RMGR(RM_HASH_ID, "Hash", hash_redo, hash_desc, hash_identify, NULL, NULL, NULL)
+PG_RMGR(RM_HASH_ID, "Hash", hash_redo, hash_desc, hash_identify, NULL, NULL, hash_mask)
 PG_RMGR(RM_GIN_ID, "Gin", gin_redo, gin_desc, gin_identify, gin_xlog_startup, gin_xlog_cleanup, gin_mask)
 PG_RMGR(RM_GIST_ID, "Gist", gist_redo, gist_desc, gist_identify, gist_xlog_startup, gist_xlog_cleanup, gist_mask)
 PG_RMGR(RM_SEQ_ID, "Sequence", seq_redo, seq_desc, seq_identify, NULL, NULL, seq_mask)
-- 
1.8.3.1

Amit Kapila

amit.kapila16@gmail.com

almost 9 years ago

In reply to: Kuntal Ghosh (#1)

Re: WAL Consistency checking for hash indexes

On Tue, Feb 28, 2017 at 11:06 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.

I think it is better if you base your patch on (Microvacuum support
for hash index - https://commitfest.postgresql.org/13/835/).

1.
There are some hints which we might want to mask that are used in that
patch.  For ex, I think you need to take care of Dead marking at page
level. Refer below code in patch "Microvacuum support for hash index".
+ if (killedsomething)
+ {
+ opaque->hasho_flag |= LH_PAGE_HAS_DEAD_TUPLES;

2.
+ else if ((opaque->hasho_flag & LH_BUCKET_PAGE) ||
+ (opaque->hasho_flag & LH_OVERFLOW_PAGE))
+ {
+ /*
+ * In btree bucket and overflow pages, it is possible to modify the
+ * LP_FLAGS without emitting any WAL record. Hence, mask the line
+ * pointer flags.
+ * See hashgettuple() for details.
+ */
+ mask_lp_flags(page);
+ }

Again, this mechanism is also modified by patch "Microvacuum support
for hash index", so above changes needs to be adjusted accordingly.
Comment referring to btree is wrong, you need to refer hash.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas

robertmhaas@gmail.com

almost 9 years ago

In reply to: Amit Kapila (#2)

Re: WAL Consistency checking for hash indexes

On Fri, Mar 3, 2017 at 9:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Feb 28, 2017 at 11:06 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.

I think it is better if you base your patch on (Microvacuum support
for hash index - https://commitfest.postgresql.org/13/835/).

I'd rather have this based on top of the WAL logging patch, and have
any subsequent patches that tinker with the WAL logging include the
necessary consistency checking changes also.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Amit Kapila

amit.kapila16@gmail.com

almost 9 years ago

In reply to: Robert Haas (#3)

Re: WAL Consistency checking for hash indexes

On Sat, Mar 4, 2017 at 2:29 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Mar 3, 2017 at 9:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Feb 28, 2017 at 11:06 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.

I think it is better if you base your patch on (Microvacuum support
for hash index - https://commitfest.postgresql.org/13/835/).

I'd rather have this based on top of the WAL logging patch, and have
any subsequent patches that tinker with the WAL logging include the
necessary consistency checking changes also.

Fair point. I thought as the other patch has been proposed before
this patch, so it might be better to tackle the changes related to
that patch in this patch. However, changing the MicroVacuum or any
other patch to consider what needs to be masked for that patch sounds
sensible.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kuntal Ghosh

kuntalghosh.2007@gmail.com

almost 9 years ago

In reply to: Amit Kapila (#2)

1 attachment(s)

Re: WAL Consistency checking for hash indexes

On Fri, Mar 3, 2017 at 9:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Tue, Feb 28, 2017 at 11:06 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.
2.
+ else if ((opaque->hasho_flag & LH_BUCKET_PAGE) ||
+ (opaque->hasho_flag & LH_OVERFLOW_PAGE))
+ {
+ /*
+ * In btree bucket and overflow pages, it is possible to modify the
+ * LP_FLAGS without emitting any WAL record. Hence, mask the line
+ * pointer flags.
+ * See hashgettuple() for details.
+ */
+ mask_lp_flags(page);
+ }
Again, this mechanism is also modified by patch "Microvacuum support
for hash index", so above changes needs to be adjusted accordingly.
Comment referring to btree is wrong, you need to refer hash.

I've corrected the text in the comment and re-based the patch on the
latest hash index patch for WAL logging[1]/messages/by-id/CAA4eK1+mvCucroWQwX3S7aBR=0yBJGF+jQz4x4Cx9QVsMFTZUw@mail.gmail.com -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com. As discussed in the
thread, Microvacuum patch can be re-based on top of this patch.

[1]: /messages/by-id/CAA4eK1+mvCucroWQwX3S7aBR=0yBJGF+jQz4x4Cx9QVsMFTZUw@mail.gmail.com -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Attachments:

0001-wal_consistency_checking-for-hash-index_v1.patchbinary/octet-stream; name=0001-wal_consistency_checking-for-hash-index_v1.patchDownload

From 27e25042644bf48ae5f0e997782fac859e08e9ff Mon Sep 17 00:00:00 2001
From: Kuntal Ghosh <kuntal.ghosh@enterprisedb.com>
Date: Tue, 28 Feb 2017 10:51:27 +0530
Subject: [PATCH] wal_consistency_checking for hash index

This patch implements the feature of wal consistency checking for
hash index. It includes a hash_mask function to mask the hash pages
before performing any consistency check.
---
 src/backend/access/hash/hash_xlog.c | 37 +++++++++++++++++++++++++++++++++++++
 src/include/access/hash_xlog.h      |  1 +
 src/include/access/rmgrlist.h       |  2 +-
 3 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index aff9b53..5c46174 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -14,6 +14,7 @@
  */
 #include "postgres.h"
 
+#include "access/bufmask.h"
 #include "access/hash.h"
 #include "access/hash_xlog.h"
 #include "access/xlogutils.h"
@@ -972,3 +973,39 @@ hash_redo(XLogReaderState *record)
 			elog(PANIC, "hash_redo: unknown op code %u", info);
 	}
 }
+
+/*
+ * Mask a Hash page before performing consistency checks on it.
+ */
+void
+hash_mask(char *pagedata, BlockNumber blkno)
+{
+	Page		page = (Page) pagedata;
+	HashPageOpaque		opaque;
+
+	mask_page_lsn(page);
+
+	mask_page_hint_bits(page);
+	mask_unused_space(page);
+
+	opaque = (HashPageOpaque) PageGetSpecialPointer(page);
+
+	if (opaque->hasho_flag & LH_UNUSED_PAGE)
+	{
+		/*
+		 * Mask everything on a UNUSED page.
+		 */
+		mask_page_content(page);
+	}
+	else if ((opaque->hasho_flag & LH_BUCKET_PAGE) ||
+						(opaque->hasho_flag & LH_OVERFLOW_PAGE))
+	{
+		/*
+		 * In hash bucket and overflow pages, it is possible to modify the
+		 * LP_FLAGS without emitting any WAL record. Hence, mask the line
+		 * pointer flags.
+		 * See hashgettuple() for details.
+		 */
+		mask_lp_flags(page);
+	}
+}
diff --git a/src/include/access/hash_xlog.h b/src/include/access/hash_xlog.h
index 8400022..6620dee 100644
--- a/src/include/access/hash_xlog.h
+++ b/src/include/access/hash_xlog.h
@@ -253,5 +253,6 @@ typedef struct xl_hash_init_bitmap_page
 extern void hash_redo(XLogReaderState *record);
 extern void hash_desc(StringInfo buf, XLogReaderState *record);
 extern const char *hash_identify(uint8 info);
+extern void hash_mask(char *pagedata, BlockNumber blkno);
 
 #endif   /* HASH_XLOG_H */
diff --git a/src/include/access/rmgrlist.h b/src/include/access/rmgrlist.h
index b892aea..2f43c19 100644
--- a/src/include/access/rmgrlist.h
+++ b/src/include/access/rmgrlist.h
@@ -37,7 +37,7 @@ PG_RMGR(RM_STANDBY_ID, "Standby", standby_redo, standby_desc, standby_identify,
 PG_RMGR(RM_HEAP2_ID, "Heap2", heap2_redo, heap2_desc, heap2_identify, NULL, NULL, heap_mask)
 PG_RMGR(RM_HEAP_ID, "Heap", heap_redo, heap_desc, heap_identify, NULL, NULL, heap_mask)
 PG_RMGR(RM_BTREE_ID, "Btree", btree_redo, btree_desc, btree_identify, NULL, NULL, btree_mask)
-PG_RMGR(RM_HASH_ID, "Hash", hash_redo, hash_desc, hash_identify, NULL, NULL, NULL)
+PG_RMGR(RM_HASH_ID, "Hash", hash_redo, hash_desc, hash_identify, NULL, NULL, hash_mask)
 PG_RMGR(RM_GIN_ID, "Gin", gin_redo, gin_desc, gin_identify, gin_xlog_startup, gin_xlog_cleanup, gin_mask)
 PG_RMGR(RM_GIST_ID, "Gist", gist_redo, gist_desc, gist_identify, gist_xlog_startup, gist_xlog_cleanup, gist_mask)
 PG_RMGR(RM_SEQ_ID, "Sequence", seq_redo, seq_desc, seq_identify, NULL, NULL, seq_mask)
-- 
1.8.3.1

Ashutosh Sharma

ashu.coek88@gmail.com

almost 9 years ago

In reply to: Kuntal Ghosh (#5)

Re: WAL Consistency checking for hash indexes

Couple of review comments,,

You may also need to update the documentation as now we are also going
to support wal consistency check for hash index. The current
documentation does not include hash index.

+        only records originating from those resource managers.  Currently,
+        the supported resource managers are <literal>heap</>,
+        <literal>heap2</>, <literal>btree</>, <literal>gin</>,
+        <literal>gist</>, <literal>sequence</>, <literal>spgist</>,
+        <literal>brin</>, and <literal>generic</>. Only

Following comment in hash_mask() may require changes if patch for
'Microvacuum support for Hash Index - [1]/messages/by-id/CAE9k0PmXyQpHX8=L_hFV7HfPV8qrit19xoUB86waQ87rKYzmYQ@mail.gmail.com' gets committed.

+       /*
+        * In hash bucket and overflow pages, it is possible to modify the
+        * LP_FLAGS without emitting any WAL record. Hence, mask the line
+        * pointer flags.
+        * See hashgettuple() for details.
+        */

[1]: /messages/by-id/CAE9k0PmXyQpHX8=L_hFV7HfPV8qrit19xoUB86waQ87rKYzmYQ@mail.gmail.com

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On Wed, Mar 8, 2017 at 2:32 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:

On Fri, Mar 3, 2017 at 9:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, Feb 28, 2017 at 11:06 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Hello everyone,

I've attached a patch which implements WAL consistency checking for
hash indexes. This feature is going to be useful for developing and
testing of WAL logging for hash index.
2.
+ else if ((opaque->hasho_flag & LH_BUCKET_PAGE) ||
+ (opaque->hasho_flag & LH_OVERFLOW_PAGE))
+ {
+ /*
+ * In btree bucket and overflow pages, it is possible to modify the
+ * LP_FLAGS without emitting any WAL record. Hence, mask the line
+ * pointer flags.
+ * See hashgettuple() for details.
+ */
+ mask_lp_flags(page);
+ }
Again, this mechanism is also modified by patch "Microvacuum support
for hash index", so above changes needs to be adjusted accordingly.
Comment referring to btree is wrong, you need to refer hash.
I've corrected the text in the comment and re-based the patch on the
latest hash index patch for WAL logging[1]. As discussed in the
thread, Microvacuum patch can be re-based on top of this patch.

[1] /messages/by-id/CAA4eK1+mvCucroWQwX3S7aBR=0yBJGF+jQz4x4Cx9QVsMFTZUw@mail.gmail.com
--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas

robertmhaas@gmail.com

almost 9 years ago

In reply to: Ashutosh Sharma (#6)

Re: WAL Consistency checking for hash indexes

On Mon, Mar 13, 2017 at 10:36 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Couple of review comments,,

You may also need to update the documentation as now we are also going
to support wal consistency check for hash index. The current
documentation does not include hash index.
+        only records originating from those resource managers.  Currently,
+        the supported resource managers are <literal>heap</>,
+        <literal>heap2</>, <literal>btree</>, <literal>gin</>,
+        <literal>gist</>, <literal>sequence</>, <literal>spgist</>,
+        <literal>brin</>, and <literal>generic</>. Only

Did that, committed this. Also ran pgindent over hash_mask() and
fixed an instance of dubious capitalization.

Following comment in hash_mask() may require changes if patch for
'Microvacuum support for Hash Index - [1]' gets committed.
+       /*
+        * In hash bucket and overflow pages, it is possible to modify the
+        * LP_FLAGS without emitting any WAL record. Hence, mask the line
+        * pointer flags.
+        * See hashgettuple() for details.
+        */

If that's so, then that patch is responsible for updating these
comments (and the code, if necessary).

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Kuntal Ghosh

kuntalghosh.2007@gmail.com

almost 9 years ago

In reply to: Robert Haas (#7)

Re: WAL Consistency checking for hash indexes

On Wed, Mar 15, 2017 at 12:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Mon, Mar 13, 2017 at 10:36 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
Couple of review comments,,

You may also need to update the documentation as now we are also going
to support wal consistency check for hash index. The current
documentation does not include hash index.
+        only records originating from those resource managers.  Currently,
+        the supported resource managers are <literal>heap</>,
+        <literal>heap2</>, <literal>btree</>, <literal>gin</>,
+        <literal>gist</>, <literal>sequence</>, <literal>spgist</>,
+        <literal>brin</>, and <literal>generic</>. Only
Did that, committed this. Also ran pgindent over hash_mask() and
fixed an instance of dubious capitalization.

Thanks Robert for the commit.

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Sharma

ashu.coek88@gmail.com

almost 9 years ago

In reply to: Kuntal Ghosh (#8)

1 attachment(s)

Re: WAL Consistency checking for hash indexes

Hi,

Attached is the patch that allows WAL consistency tool to mask
'LH_PAGE_HAS_DEAD_TUPLES' flag in hash index. The flag got added as a
part of 'Microvacuum support for Hash index' patch . I have already
tested it using Kuntal's WAL consistency tool and it works fine.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On Wed, Mar 15, 2017 at 11:27 AM, Kuntal Ghosh
<kuntalghosh.2007@gmail.com> wrote:

Show quoted text

On Wed, Mar 15, 2017 at 12:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Mar 13, 2017 at 10:36 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
Couple of review comments,,

You may also need to update the documentation as now we are also going
to support wal consistency check for hash index. The current
documentation does not include hash index.
+        only records originating from those resource managers.  Currently,
+        the supported resource managers are <literal>heap</>,
+        <literal>heap2</>, <literal>btree</>, <literal>gin</>,
+        <literal>gist</>, <literal>sequence</>, <literal>spgist</>,
+        <literal>brin</>, and <literal>generic</>. Only
Did that, committed this. Also ran pgindent over hash_mask() and
fixed an instance of dubious capitalization.
Thanks Robert for the commit.

--
Thanks & Regards,
Kuntal Ghosh
EnterpriseDB: http://www.enterprisedb.com

Attachments:

0001-Allow-WAL-consistency-tool-to-mask-LH_PAGE_HAS_DEAD_.patchapplication/x-download; name=0001-Allow-WAL-consistency-tool-to-mask-LH_PAGE_HAS_DEAD_.patchDownload

From 009e5d12451d3ba3e8e729a97b614a78af6aed10 Mon Sep 17 00:00:00 2001
From: ashu <ashu@localhost.localdomain>
Date: Thu, 16 Mar 2017 12:01:30 +0530
Subject: [PATCH] Allow WAL consistency tool to mask LH_PAGE_HAS_DEAD_TUPLES
 added as a part of 'Microvacuum support for Hash Index'.

---
 src/backend/access/hash/hash_xlog.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index 8647e8c..cabf0fd 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -1229,8 +1229,15 @@ hash_mask(char *pagedata, BlockNumber blkno)
 		/*
 		 * In hash bucket and overflow pages, it is possible to modify the
 		 * LP_FLAGS without emitting any WAL record. Hence, mask the line
-		 * pointer flags. See hashgettuple() for details.
+		 * pointer flags. See hashgettuple(), _hash_kill_items() for details.
 		 */
 		mask_lp_flags(page);
 	}
+
+	/*
+	 * It is possible that the hint bit LH_PAGE_HAS_DEAD_TUPLES may remain
+	 * unlogged. So, mask it. See _hash_kill_items(), MarkBufferDirtyHint()
+	 * for details.
+	 */
+	opaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES;
 }
-- 
1.8.3.1

#10

Amit Kapila

amit.kapila16@gmail.com

almost 9 years ago

In reply to: Ashutosh Sharma (#9)

Re: WAL Consistency checking for hash indexes

On Thu, Mar 16, 2017 at 1:15 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Hi,

Attached is the patch that allows WAL consistency tool to mask
'LH_PAGE_HAS_DEAD_TUPLES' flag in hash index. The flag got added as a
part of 'Microvacuum support for Hash index' patch . I have already
tested it using Kuntal's WAL consistency tool and it works fine.

+ * unlogged. So, mask it. See _hash_kill_items(), MarkBufferDirtyHint()
+ * for
details.
+ */

I think in above comment, a reference to _hash_kill_items is
sufficient. Other than that patch looks okay.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11

Ashutosh Sharma

ashu.coek88@gmail.com

almost 9 years ago

In reply to: Amit Kapila (#10)

1 attachment(s)

Re: WAL Consistency checking for hash indexes

On Fri, Mar 17, 2017 at 9:03 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Mar 16, 2017 at 1:15 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Hi,

Attached is the patch that allows WAL consistency tool to mask
'LH_PAGE_HAS_DEAD_TUPLES' flag in hash index. The flag got added as a
part of 'Microvacuum support for Hash index' patch . I have already
tested it using Kuntal's WAL consistency tool and it works fine.
+ * unlogged. So, mask it. See _hash_kill_items(), MarkBufferDirtyHint()
+ * for
details.
+ */
I think in above comment, a reference to _hash_kill_items is
sufficient. Other than that patch looks okay.

Okay, I have removed the reference to MarkBufferDirtyHint() from above
comment. Attached is the v2 version of patch.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Attachments:

0001-Allow-WAL-consistency-tool-to-mask-LH_PAGE_HAS_DEAD_.patchbinary/octet-stream; name=0001-Allow-WAL-consistency-tool-to-mask-LH_PAGE_HAS_DEAD_.patchDownload

From d191629fc66ac6ab64e80d96d7f0b9aba9743696 Mon Sep 17 00:00:00 2001
From: ashu <ashu@localhost.localdomain>
Date: Fri, 17 Mar 2017 11:50:47 +0530
Subject: [PATCH] Allow WAL consistency tool to mask LH_PAGE_HAS_DEAD_TUPLES
 flag

Patch by Ashutosh Sharma
---
 src/backend/access/hash/hash_xlog.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/hash/hash_xlog.c b/src/backend/access/hash/hash_xlog.c
index ac82092..de7522e 100644
--- a/src/backend/access/hash/hash_xlog.c
+++ b/src/backend/access/hash/hash_xlog.c
@@ -1250,8 +1250,14 @@ hash_mask(char *pagedata, BlockNumber blkno)
 		/*
 		 * In hash bucket and overflow pages, it is possible to modify the
 		 * LP_FLAGS without emitting any WAL record. Hence, mask the line
-		 * pointer flags. See hashgettuple() for details.
+		 * pointer flags. See hashgettuple(), _hash_kill_items() for details.
 		 */
 		mask_lp_flags(page);
 	}
+
+	/*
+	 * It is possible that the hint bit LH_PAGE_HAS_DEAD_TUPLES may remain
+	 * unlogged. So, mask it. See _hash_kill_items() for details.
+	 */
+	opaque->hasho_flag &= ~LH_PAGE_HAS_DEAD_TUPLES;
 }
-- 
1.8.3.1

#12

Amit Kapila

amit.kapila16@gmail.com

almost 9 years ago

In reply to: Ashutosh Sharma (#11)

Re: WAL Consistency checking for hash indexes

On Fri, Mar 17, 2017 at 12:30 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

On Fri, Mar 17, 2017 at 9:03 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Mar 16, 2017 at 1:15 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:

Hi,

Attached is the patch that allows WAL consistency tool to mask
'LH_PAGE_HAS_DEAD_TUPLES' flag in hash index. The flag got added as a
part of 'Microvacuum support for Hash index' patch . I have already
tested it using Kuntal's WAL consistency tool and it works fine.
+ * unlogged. So, mask it. See _hash_kill_items(), MarkBufferDirtyHint()
+ * for
details.
+ */
I think in above comment, a reference to _hash_kill_items is
sufficient. Other than that patch looks okay.
Okay, I have removed the reference to MarkBufferDirtyHint() from above
comment. Attached is the v2 version of patch.

Thanks, this version looks good to me.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers