v35_0002_use_index_on_subs_when_pub_rep_ident_full.patch

From 69c41d0d0dda077a6fd40a8f93a16228ec382778 Mon Sep 17 00:00:00 2001
From: Onder Kalaci <onderkalaci@gmail.com>
Date: Fri, 3 Mar 2023 18:20:57 +0300
Subject: [PATCH v35 2/2] Optionally disable index scan when replica identity
 is full

When replica identitiy is full, the logical replication apply workers
for subscription is capable of using index scans. However, index scans
might have some overhead compared to sequential scan. The main use case
for disabling the index scan could be that the table has many dead
tuples as well as many duplicates, where index scan might be slower
compared to sequential scan.

We add a new option to table storage parameter that the user can
control the behavior.
---
 doc/src/sgml/ref/create_table.sgml            | 16 +++++++++-
 src/backend/access/common/reloptions.c        | 11 +++++++
 src/backend/replication/logical/relation.c    |  3 +-
 src/include/utils/rel.h                       | 12 +++++++
 .../subscription/t/032_subscribe_use_index.pl | 31 +++++++------------
 5 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index a03dee4afe..89eb154730 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1812,7 +1812,21 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     </listitem>
    </varlistentry>
 
-   </variablelist>
+  <varlistentry id="reloption-ri-index-scan" xreflabel="enable_replica_identity_full_index_scan">
+    <term><literal>enable_replica_identity_full_index_scan</literal> (<type>boolean</type>)
+    <indexterm>
+     <primary><varname>enable_replica_identity_full_index_scan</varname> storage parameter</primary>
+    </indexterm>
+    </term>
+    <listitem>
+     <para>
+      Controls using avaliable indexes on the apply worker when replica identity is full. See
+      <xref linkend="logical-replication-publication"/> for details. The default is true.
+     </para>
+    </listitem>
+   </varlistentry>
+
+  </variablelist>
 
   </refsect2>
  </refsect1>
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 14c23101ad..3e54e4a52b 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -168,6 +168,15 @@ static relopt_bool boolRelOpts[] =
 		},
 		true
 	},
+	{
+		{
+			"enable_replica_identity_full_index_scan",
+			"Enables index scan on logical replication apply worker",
+			RELOPT_KIND_HEAP | RELOPT_KIND_TOAST,
+			ShareUpdateExclusiveLock
+		},
+		true
+	},
 	/* list terminator */
 	{{NULL}}
 };
@@ -1877,6 +1886,8 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
 		offsetof(StdRdOptions, user_catalog_table)},
+		{"enable_replica_identity_full_index_scan", RELOPT_TYPE_BOOL,
+		offsetof(StdRdOptions, enable_replica_identity_full_index_scan)},
 		{"parallel_workers", RELOPT_TYPE_INT,
 		offsetof(StdRdOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_ENUM,
diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c
index 00a6328998..8f4414bab6 100644
--- a/src/backend/replication/logical/relation.c
+++ b/src/backend/replication/logical/relation.c
@@ -880,7 +880,8 @@ FindLogicalRepLocalIndex(Relation localrel, LogicalRepRelation *remoterel)
 	if (OidIsValid(idxoid))
 		return idxoid;
 
-	if (remoterel->replident == REPLICA_IDENTITY_FULL)
+	if (RelationReplicaIdentityFullIndexScanEnabled(localrel) &&
+		remoterel->replident == REPLICA_IDENTITY_FULL)
 	{
 		/*
 		 * We are looking for one more opportunity for using an index. If
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 67f994cb3e..fdd02130da 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -336,6 +336,8 @@ typedef struct StdRdOptions
 	int			toast_tuple_target; /* target for tuple toasting */
 	AutoVacOpts autovacuum;		/* autovacuum-related options */
 	bool		user_catalog_table; /* use as an additional catalog relation */
+	bool		enable_replica_identity_full_index_scan; /* enables index scan
+															for RI full */
 	int			parallel_workers;	/* max number of parallel workers */
 	StdRdOptIndexCleanup vacuum_index_cleanup;	/* controls index vacuuming */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
@@ -385,6 +387,16 @@ typedef struct StdRdOptions
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
 	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
 
+/*
+ * RelationReplicaIdentityFullIndexScanEnabled
+ *		Returns whether index scan should be enabled when replica
+ *		identity is full.
+ */
+#define RelationReplicaIdentityFullIndexScanEnabled(relation)	\
+	((relation)->rd_options && \
+	 (relation)->rd_rel->relkind == RELKIND_RELATION ? \
+	 ((StdRdOptions *) (relation)->rd_options)->enable_replica_identity_full_index_scan : true)
+
 /*
  * RelationGetParallelWorkers
  *		Returns the relation's parallel_workers reloption setting.
diff --git a/src/test/subscription/t/032_subscribe_use_index.pl b/src/test/subscription/t/032_subscribe_use_index.pl
index edb229357e..be005f8094 100644
--- a/src/test/subscription/t/032_subscribe_use_index.pl
+++ b/src/test/subscription/t/032_subscribe_use_index.pl
@@ -761,11 +761,12 @@ $node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
 # ====================================================================
 
 # ====================================================================
-# Testcase start: SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN
+# Testcase start: SUBSCRIPTION BEHAVIOR WITH enable_replica_identity_full_index_scan=false
 #
-# Even if enable_indexscan = false, we do use the primary keys, this
-# is the legacy behavior. However, we do not use non-primary/non replica
-# identity columns.
+# When the table has storage parameter enable_replica_identity_full_index_scan = false, even
+# if there is a suitable index to use.
+#
+# Note that this is not relevant when the table has PK/RI. In that case, index is always used.
 #
 
 # create tables pub and sub
@@ -774,13 +775,9 @@ $node_publisher->safe_psql('postgres',
 $node_publisher->safe_psql('postgres',
 	"ALTER TABLE test_replica_id_full REPLICA IDENTITY FULL;");
 $node_subscriber->safe_psql('postgres',
-	"CREATE TABLE test_replica_id_full (x int NOT NULL)");
+	"CREATE TABLE test_replica_id_full (x int NOT NULL) WITH (enable_replica_identity_full_index_scan = false)");
 $node_subscriber->safe_psql('postgres',
 	"CREATE INDEX test_replica_id_full_idx ON test_replica_id_full(x)");
-$node_subscriber->safe_psql('postgres',
-	"ALTER SYSTEM SET enable_indexscan TO off;");
-$node_subscriber->safe_psql('postgres',
-	"SELECT pg_reload_conf();");
 
 # insert some initial data
 $node_publisher->safe_psql('postgres',
@@ -800,10 +797,10 @@ $node_publisher->safe_psql('postgres',
 	"UPDATE test_replica_id_full SET x = x + 10000 WHERE x = 15;");
 $node_publisher->wait_for_catchup($appname);
 
-# show that index is not used when enable_indexscan=false
+# show that index is not used when enable_replica_identity_full_index_scan=false
 $result = $node_subscriber->safe_psql('postgres',
 	"select idx_scan from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_idx'");
-is($result, qq(0), 'ensure subscriber has not used index with enable_indexscan=false');
+is($result, qq(0), 'ensure subscriber has not used index with enable_replica_identity_full_index_scan=false');
 
 # we are done with this index, drop to simplify the tests
 $node_subscriber->safe_psql('postgres',
@@ -828,11 +825,11 @@ $node_publisher->wait_for_catchup($appname);
 # this is a legacy behavior
 $node_subscriber->poll_query_until(
 	'postgres', q{select (idx_scan=1) from pg_stat_all_indexes where indexrelname = 'test_replica_id_full_unique'}
-) or die "Timed out while waiting ensuring subscriber used unique index as replica identity even with enable_indexscan=false";
+) or die "Timed out while waiting ensuring subscriber used unique index as replica identity even with enable_replica_identity_full_index_scan=false";
 
 $result = $node_subscriber->safe_psql('postgres',
 	"SELECT count(*) FROM test_replica_id_full WHERE x IN (14,15)");
-is($result, qq(0), 'ensure the results are accurate even with enable_indexscan=false');
+is($result, qq(0), 'ensure the results are accurate even with enable_replica_identity_full_index_scan=false');
 
 # cleanup pub
 $node_publisher->safe_psql('postgres', "DROP PUBLICATION tap_pub_rep_full");
@@ -840,13 +837,7 @@ $node_publisher->safe_psql('postgres', "DROP TABLE test_replica_id_full");
 # cleanup sub
 $node_subscriber->safe_psql('postgres', "DROP SUBSCRIPTION tap_sub_rep_full");
 $node_subscriber->safe_psql('postgres', "DROP TABLE test_replica_id_full");
-
-$node_subscriber->safe_psql('postgres',
-	"ALTER SYSTEM RESET enable_indexscan;");
-$node_subscriber->safe_psql('postgres',
-	"SELECT pg_reload_conf();");
-
-# Testcase end: SUBSCRIPTION BEHAVIOR WITH ENABLE_INDEXSCAN
+# Testcase end: SUBSCRIPTION BEHAVIOR WITH enable_replica_identity_full_index_scan=false
 # ====================================================================
 
 # ====================================================================
-- 
2.34.1