On Fri, 16 Apr 2021 at 17:19, osumi.takami...@fujitsu.com
<osumi.takami...@fujitsu.com> wrote:
> Hi
>
>
> On Friday, April 16, 2021 5:50 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>> On Fri, Apr 16, 2021 at 12:56 PM osumi.takami...@fujitsu.com
>> <osumi.takami...@fujitsu.com> wrote:
>> >
>> > > Thanks for your reminder. It might be a way to solve this problem.
>> > Yeah. I've made the 1st patch for this issue.
>> >
>> > In my env, with the patch
>> > the TRUNCATE in synchronous logical replication doesn't hang.
>> >
>>
>> Few initial comments:
>> =====================
>> 1.
>> + relreplindex = relation->rd_replidindex;
>> +
>> + /*
>> + * build attributes to idindexattrs.
>> + */
>> + idindexattrs = NULL;
>> + foreach(l, indexoidlist)
>> + {
>> + Oid indexOid = lfirst_oid(l);
>> + Relation indexDesc;
>> + int i;
>> + bool isIDKey; /* replica identity index */
>> +
>> + indexDesc = RelationIdGetRelation(indexOid);
>>
>> When you have oid of replica identity index (relreplindex) then what is the
>> need to traverse all the indexes?
> Ok. No need to traverse all the indexes. Will fix this part.
>
>> 2.
>> It is better to name the function as RelationGet...
> You are right. I'll modify this in my next version.
>
I took the liberty to address review comments and provide a v2 patch on top of
your's v1 patch, also merge the test case.
Sorry for attaching.
--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index 2a1f9830e0..1cf59e0fb0 100644
--- 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 29702d6eab..ace167f324 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -5206,6 +5206,94 @@ restart:
}
}
+Bitmapset *
+RelationGetIdentityKeyBitmap(Relation relation)
+{
+ Bitmapset *idindexattrs; /* columns in the replica identity */
+ List *indexoidlist;
+ List *newindexoidlist;
+ Oid relpkindex;
+ Oid relreplindex;
+ 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;
+
+ /*
+ * Get cached list of index OIDs. If we have to start over, we do so here.
+ */
+restart:
+ indexoidlist = RelationGetIndexList(relation);
+
+ /* Fall out if no indexes (but relhasindex was set) */
+ if (indexoidlist == NIL)
+ return NULL;
+
+ /* Save some values to compare after building attributes */
+ relpkindex = relation->rd_pkindex;
+ relreplindex = relation->rd_replidindex;
+
+ /*
+ * build attributes to idindexattrs.
+ */
+ idindexattrs = NULL;
+ indexDesc = RelationIdGetRelation(relreplindex);
+
+ /* Collect simple attribute references */
+ for (i = 0; i < indexDesc->rd_index->indnatts; i++)
+ {
+ int attrnum = indexDesc->rd_index->indkey.values[i];
+
+ /* Romove non-key columns here. */
+ if (attrnum != 0)
+ {
+ if (i < indexDesc->rd_index->indnkeyatts)
+ idindexattrs = bms_add_member(idindexattrs,
+ attrnum - FirstLowInvalidHeapAttributeNumber);
+ }
+ }
+ RelationClose(indexDesc);
+
+ /* Check if we need to redo */
+ newindexoidlist = RelationGetIndexList(relation);
+ if (equal(indexoidlist, newindexoidlist) &&
+ relpkindex == relation->rd_pkindex &&
+ relreplindex == relation->rd_replidindex)
+ {
+ /* Still the same index set, so proceed */
+ list_free(newindexoidlist);
+ list_free(indexoidlist);
+ }
+ else
+ {
+ /* Gotta do it over ... might as well not leak memory */
+ list_free(newindexoidlist);
+ list_free(indexoidlist);
+ bms_free(idindexattrs);
+
+ goto restart;
+ }
+
+ /* Don't leak the old values of these bitmaps, if any */
+ bms_free(relation->rd_idattr);
+ relation->rd_idattr = NULL;
+
+ /* Now save copies of the bitmaps 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 2fcdf79323..f772855ac6 100644
--- 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 be2c0bdc35..cfcee2f1a7 100644
--- 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');
+
+# setup 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');
