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. Greetings, 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 RelationGetIndexAttrBitmap(). 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 DEFAULT or INDEX. --- 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++) -- 2.0.0.rc2.4.g1dc51c6.dirty
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
