Fix Logical Replication of Truncate in synchronous commit mode.

The Truncate operation acquires an exclusive lock on the target relation
and indexes. It then waits for logical replication of the operation to
finish at commit. Now because we are acquiring the shared lock on the
target index to get index attributes in pgoutput while sending the
changes for the Truncate operation, it leads to a deadlock.

Actually, we don't need to acquire a lock on the target index as we build
the cache entry using a historic snapshot and all the later changes are
absorbed while decoding WAL. So, we wrote a special purpose function for
logical replication to get a bitmap of replica identity attribute numbers
where we get that information without locking the target index.

We decided not to backpatch this as there doesn't seem to be any field
complaint about this issue since it was introduced in commit 5dfd1e5a in
v11.

Reported-by: Haiying Tang
Author: Takamichi Osumi, test case by Li Japin
Reviewed-by: Amit Kapila, Ajin Cherian
Discussion: https://postgr.es/m/OS0PR01MB6113C2499C7DC70EE55ADB82FB759@OS0PR01MB6113.jpnprd01.prod.outlook.com

diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index 2a1f9830e05d3e1a9860549d0274d84ff2d3b05e..1cf59e0fb0fae89e99c6489af1a1359dd0fa225d 100644 (file)
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -668,8 +668,7 @@ logicalrep_write_attrs(StringInfo out, Relation rel)
    /* fetch bitmap of REPLICATION IDENTITY attributes */
    replidentfull = (rel->rd_rel->relreplident == REPLICA_IDENTITY_FULL);
    if (!replidentfull)
-       idattrs = RelationGetIndexAttrBitmap(rel,
-                                            INDEX_ATTR_BITMAP_IDENTITY_KEY);
+       idattrs = RelationGetIdentityKeyBitmap(rel);
 
    /* send the attributes */
    for (i = 0; i < desc->natts; i++)

diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 29702d6eab18f3fb614d281538146203ae5cb4a7..466c28d528775628ffc4b621ae6c755e2ef1a45b 100644 (file)
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5206,6 +5206,81 @@ restart:
    }
 }
 
+/*
+ * RelationGetIdentityKeyBitmap -- get a bitmap of replica identity attribute
+ * numbers
+ *
+ * A bitmap of index attribute numbers for the configured replica identity
+ * index is returned.
+ *
+ * See also comments of RelationGetIndexAttrBitmap().
+ *
+ * This is a special purpose function used during logical replication. Here,
+ * unlike RelationGetIndexAttrBitmap(), we don't acquire a lock on the required
+ * index as we build the cache entry using a historic snapshot and all the
+ * later changes are absorbed while decoding WAL. Due to this reason, we don't
+ * need to retry here in case of a change in the set of indexes.
+ */
+Bitmapset *
+RelationGetIdentityKeyBitmap(Relation relation)
+{
+   Bitmapset  *idindexattrs = NULL;    /* columns in the replica identity */
+   List       *indexoidlist;
+   Relation    indexDesc;
+   int         i;
+   MemoryContext oldcxt;
+
+   /* Quick exit if we already computed the result */
+   if (relation->rd_idattr != NULL)
+       return bms_copy(relation->rd_idattr);
+
+   /* Fast path if definitely no indexes */
+   if (!RelationGetForm(relation)->relhasindex)
+       return NULL;
+
+   /* Historic snapshot must be set. */
+   Assert(HistoricSnapshotActive());
+
+   indexoidlist = RelationGetIndexList(relation);
+
+   /* Fall out if no indexes (but relhasindex was set) */
+   if (indexoidlist == NIL)
+       return NULL;
+
+   /* Add referenced attributes to idindexattrs */
+   indexDesc = RelationIdGetRelation(relation->rd_replidindex);
+   for (i = 0; i < indexDesc->rd_index->indnatts; i++)
+   {
+       int         attrnum = indexDesc->rd_index->indkey.values[i];
+
+       /*
+        * We don't include non-key columns into idindexattrs bitmaps. See
+        * RelationGetIndexAttrBitmap.
+        */
+       if (attrnum != 0)
+       {
+           if (i < indexDesc->rd_index->indnkeyatts)
+               idindexattrs = bms_add_member(idindexattrs,
+                                             attrnum - FirstLowInvalidHeapAttributeNumber);
+       }
+   }
+
+   RelationClose(indexDesc);
+   list_free(indexoidlist);
+
+   /* Don't leak the old values of these bitmaps, if any */
+   bms_free(relation->rd_idattr);
+   relation->rd_idattr = NULL;
+
+   /* Now save copy of the bitmap in the relcache entry */
+   oldcxt = MemoryContextSwitchTo(CacheMemoryContext);
+   relation->rd_idattr = bms_copy(idindexattrs);
+   MemoryContextSwitchTo(oldcxt);
+
+   /* We return our original working copy for caller to play with */
+   return idindexattrs;
+}
+
 /*
  * RelationGetExclusionInfo -- get info about index's exclusion constraint
  *

diff --git a/src/include/utils/relcache.h b/src/include/utils/relcache.h
index 2fcdf793238b8df5080bdaf12cac849e4064d6b5..f772855ac69d62919eb74841206491b62c9a47ab 100644 (file)
--- a/src/include/utils/relcache.h
+++ b/src/include/utils/relcache.h
@@ -65,6 +65,8 @@ typedef enum IndexAttrBitmapKind
 extern Bitmapset *RelationGetIndexAttrBitmap(Relation relation,
                                             IndexAttrBitmapKind attrKind);
 
+extern Bitmapset *RelationGetIdentityKeyBitmap(Relation relation);
+
 extern void RelationGetExclusionInfo(Relation indexRelation,
                                     Oid **operators,
                                     Oid **procs,

diff --git a/src/test/subscription/t/010_truncate.pl b/src/test/subscription/t/010_truncate.pl
index be2c0bdc35e3d61993ef32865deff88b258c5568..7fc403e13855616ae8119c2db7648f6ab1c2966d 100644 (file)
--- a/src/test/subscription/t/010_truncate.pl
+++ b/src/test/subscription/t/010_truncate.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 9;
+use Test::More tests => 11;
 
 # setup
 
@@ -158,3 +158,28 @@ is($result, qq(0||), 'truncate of multiple tables some not published');
 $result = $node_subscriber->safe_psql('postgres',
    "SELECT count(*), min(a), max(a) FROM tab2");
 is($result, qq(3|1|3), 'truncate of multiple tables some not published');
+
+# Test that truncate works for synchronous logical replication
+
+$node_publisher->safe_psql('postgres',
+   "ALTER SYSTEM SET synchronous_standby_names TO 'sub1'");
+$node_publisher->safe_psql('postgres', "SELECT pg_reload_conf()");
+
+# insert data to truncate
+
+$node_publisher->safe_psql('postgres',
+   "INSERT INTO tab1 VALUES (1), (2), (3)");
+
+$node_publisher->wait_for_catchup('sub1');
+
+$result = $node_subscriber->safe_psql('postgres',
+   "SELECT count(*), min(a), max(a) FROM tab1");
+is($result, qq(3|1|3), 'check synchronous logical replication');
+
+$node_publisher->safe_psql('postgres', "TRUNCATE tab1");
+
+$node_publisher->wait_for_catchup('sub1');
+
+$result = $node_subscriber->safe_psql('postgres',
+   "SELECT count(*), min(a), max(a) FROM tab1");
+is($result, qq(0||), 'truncate replicated in synchronous logical replication');