On Wed, Jul 13, 2022 at 1:13 PM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Wed, Jul 13, 2022 at 12:22:06PM +0530, vignesh C wrote:
> > Most of the code is common between GetSubscriptionRelations and
> > GetSubscriptionNotReadyRelations. Added a parameter to
> > GetSubscriptionRelations which could provide the same functionality as
> > the existing GetSubscriptionRelations and
> > GetSubscriptionNotReadyRelations. Attached patch has the changes for
> > the same. Thoughts?
>
> Right. Using all_rels to mean that we'd filter relations that are not
> ready is a bit confusing, though. Perhaps this could use a bitmask as
> argument.
The attached v2 patch has the modified version which includes the
changes to make the argument as bitmask.
Regards,
Vignesh
From 1f67e4b5ff2d304323f262e8acf74d48893ceb06 Mon Sep 17 00:00:00 2001
From: Vigneshwaran C <vignes...@gmail.com>
Date: Wed, 13 Jul 2022 11:54:59 +0530
Subject: [PATCH v2] Refactor to make use of a common function for
GetSubscriptionRelations and GetSubscriptionNotReadyRelations.
Most of the code is common between GetSubscriptionRelations and
GetSubscriptionNotReadyRelations. Added a parameter to GetSubscriptionRelations
which could provide the same functionality for GetSubscriptionRelations
and GetSubscriptionNotReadyRelations.
---
src/backend/catalog/pg_subscription.c | 71 ++++-----------------
src/backend/commands/subscriptioncmds.c | 5 +-
src/backend/replication/logical/tablesync.c | 3 +-
src/include/catalog/pg_subscription.h | 3 +
src/include/catalog/pg_subscription_rel.h | 3 +-
5 files changed, 20 insertions(+), 65 deletions(-)
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 8856ce3b50..cd7dc00c79 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -525,65 +525,15 @@ HasSubscriptionRelations(Oid subid)
}
/*
- * Get all relations for subscription.
+ * Get the relations for subscription.
*
- * Returned list is palloc'ed in current memory context.
+ * If subrel_options is not set, return all the relations for subscription. If
+ * SUBSCRIPTION_REL_STATE_NOT_READY bit is set in subrel_options, return all
+ * the relations for subscription that are not in a ready state. Returned list
+ * is palloc'ed in current memory context.
*/
List *
-GetSubscriptionRelations(Oid subid)
-{
- List *res = NIL;
- Relation rel;
- HeapTuple tup;
- ScanKeyData skey[1];
- SysScanDesc scan;
-
- rel = table_open(SubscriptionRelRelationId, AccessShareLock);
-
- ScanKeyInit(&skey[0],
- Anum_pg_subscription_rel_srsubid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(subid));
-
- scan = systable_beginscan(rel, InvalidOid, false,
- NULL, 1, skey);
-
- while (HeapTupleIsValid(tup = systable_getnext(scan)))
- {
- Form_pg_subscription_rel subrel;
- SubscriptionRelState *relstate;
- Datum d;
- bool isnull;
-
- subrel = (Form_pg_subscription_rel) GETSTRUCT(tup);
-
- relstate = (SubscriptionRelState *) palloc(sizeof(SubscriptionRelState));
- relstate->relid = subrel->srrelid;
- relstate->state = subrel->srsubstate;
- d = SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,
- Anum_pg_subscription_rel_srsublsn, &isnull);
- if (isnull)
- relstate->lsn = InvalidXLogRecPtr;
- else
- relstate->lsn = DatumGetLSN(d);
-
- res = lappend(res, relstate);
- }
-
- /* Cleanup */
- systable_endscan(scan);
- table_close(rel, AccessShareLock);
-
- return res;
-}
-
-/*
- * Get all relations for subscription that are not in a ready state.
- *
- * Returned list is palloc'ed in current memory context.
- */
-List *
-GetSubscriptionNotReadyRelations(Oid subid)
+GetSubscriptionRelations(Oid subid, bits32 subrel_state_options)
{
List *res = NIL;
Relation rel;
@@ -599,10 +549,11 @@ GetSubscriptionNotReadyRelations(Oid subid)
BTEqualStrategyNumber, F_OIDEQ,
ObjectIdGetDatum(subid));
- ScanKeyInit(&skey[nkeys++],
- Anum_pg_subscription_rel_srsubstate,
- BTEqualStrategyNumber, F_CHARNE,
- CharGetDatum(SUBREL_STATE_READY));
+ if ((subrel_state_options & SUBSCRIPTION_REL_STATE_NOT_READY) != 0)
+ ScanKeyInit(&skey[nkeys++],
+ Anum_pg_subscription_rel_srsubstate,
+ BTEqualStrategyNumber, F_CHARNE,
+ CharGetDatum(SUBREL_STATE_READY));
scan = systable_beginscan(rel, InvalidOid, false,
NULL, nkeys, skey);
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index bdc1208724..d9fb235ce5 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -785,7 +785,7 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data,
pubrel_names = fetch_table_list(wrconn, sub->publications);
/* Get local table list. */
- subrel_states = GetSubscriptionRelations(sub->oid);
+ subrel_states = GetSubscriptionRelations(sub->oid, 0);
/*
* Build qsorted array of local table oids for faster lookup. This can
@@ -1457,7 +1457,8 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
* the apply and tablesync workers and they can't restart because of
* exclusive lock on the subscription.
*/
- rstates = GetSubscriptionNotReadyRelations(subid);
+ rstates = GetSubscriptionRelations(subid,
+ SUBSCRIPTION_REL_STATE_NOT_READY);
foreach(lc, rstates)
{
SubscriptionRelState *rstate = (SubscriptionRelState *) lfirst(lc);
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 670c6fcada..d0bb6382c0 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -1479,7 +1479,8 @@ FetchTableStates(bool *started_tx)
}
/* Fetch all non-ready tables. */
- rstates = GetSubscriptionNotReadyRelations(MySubscription->oid);
+ rstates = GetSubscriptionRelations(MySubscription->oid,
+ SUBSCRIPTION_REL_STATE_NOT_READY);
/* Allocate the tracking info in a permanent memory context. */
oldctx = MemoryContextSwitchTo(CacheMemoryContext);
diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index d1260f590c..5ff428bcba 100644
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -31,6 +31,9 @@
#define LOGICALREP_TWOPHASE_STATE_PENDING 'p'
#define LOGICALREP_TWOPHASE_STATE_ENABLED 'e'
+/* flag bits for getting the relevant subscription relations */
+#define SUBSCRIPTION_REL_STATE_NOT_READY 0x01 /* NOT READY relations */
+
/* ----------------
* pg_subscription definition. cpp turns this into
* typedef struct FormData_pg_subscription
diff --git a/src/include/catalog/pg_subscription_rel.h b/src/include/catalog/pg_subscription_rel.h
index 9df99c3418..8f6c602860 100644
--- a/src/include/catalog/pg_subscription_rel.h
+++ b/src/include/catalog/pg_subscription_rel.h
@@ -88,7 +88,6 @@ extern char GetSubscriptionRelState(Oid subid, Oid relid, XLogRecPtr *sublsn);
extern void RemoveSubscriptionRel(Oid subid, Oid relid);
extern bool HasSubscriptionRelations(Oid subid);
-extern List *GetSubscriptionRelations(Oid subid);
-extern List *GetSubscriptionNotReadyRelations(Oid subid);
+extern List *GetSubscriptionRelations(Oid subid, bits32 subrel_state_options);
#endif /* PG_SUBSCRIPTION_REL_H */
--
2.32.0
