On 2014-05-14 15:17:39 +0200, Andres Freund wrote:
> On 2014-05-14 15:08:08 +0200, Tomas Vondra wrote:
> > Apparently there's something wrong with 'test-decoding-check':
> Man. I shouldn't have asked... My code. There's some output in there
> that's probably triggered by the extraordinarily long runtimes, but
> there's definitely something else wrong.
> My gut feeling says it's in RelationGetIndexList().

Nearly right. It's in RelationGetIndexAttrBitmap(). Fix attached.

Tomas, thanks for that. I've never (and probably will never) run
CLOBBER_CACHE_RECURSIVELY during development. Having a machine do that
regularly is really helpful. How long does a single testrun take? It
takes hundreds of seconds here to do a single UPDATE?

There were some more differences but those are all harmless and caused
by the extraordinarily long runtime (autovacuums). I think we need to
add a feature to test_decoding to suppress displaying transactions
without changes. Ick.


Andres Freund

 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From 9dfe879d2b6940b6072e277b0104e9cbe4af691e Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 14 May 2014 17:12:57 +0200
Subject: [PATCH] Fix cache invalidation hazard in

When a cache invalidation arrived inside RelationGetIndexAttrBitmap()
inbetween the call to RelationGetIndexList() and one of the
index_open() calls relation->rd_replidindex would be reset leading to
a corrupted INDEX_ATTR_BITMAP_IDENTITY_KEY return value. That in turn
could lead to the old REPLICA IDENTITY not being logged if set to
 src/backend/utils/cache/relcache.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 5ff0d9e..4fc18b5 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -3976,6 +3976,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 	List	   *indexoidlist;
 	ListCell   *l;
 	MemoryContext oldcxt;
+	Oid			relreplindex;
 	/* Quick exit if we already computed the result. */
 	if (relation->rd_indexattr != NULL)
@@ -4005,6 +4006,12 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 		return NULL;
+	 * Store after computing the index list above, to be safe against cache
+	 * flushes inside index_open() below.
+	 */
+	relreplindex = relation->rd_replidindex;
+	/*
 	 * For each index, add referenced attributes to indexattrs.
 	 * Note: we consider all indexes returned by RelationGetIndexList, even if
@@ -4038,7 +4045,7 @@ RelationGetIndexAttrBitmap(Relation relation, IndexAttrBitmapKind attrKind)
 			indexInfo->ii_Predicate == NIL;
 		/* Is this index the configured (or default) replica identity? */
-		isIDKey = indexOid == relation->rd_replidindex;
+		isIDKey = indexOid == relreplindex;
 		/* Collect simple attribute references */
 		for (i = 0; i < indexInfo->ii_NumIndexAttrs; i++)

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

Reply via email to