From 93116320bd9ffbcddc83a1fc253cba90c56cb47e Mon Sep 17 00:00:00 2001 From: Vignesh C Date: Thu, 1 Feb 2024 09:46:40 +0530 Subject: [PATCH v7] Table sync missed due to race condition in subscription cache handling. Table sync was missed if the invalidation of table sync occurs while the non ready tables were getting prepared. This occurrs because the table state was being set to valid at the end of non ready table list preparation irrespective of any inavlidations occurred while preparing the list. Fixed it by changing the boolean variable to an tri-state enum and by setting table state to valid only if no invalidations have been occurred while the list is being prepared. --- src/backend/replication/logical/tablesync.c | 25 +++++++++++++++++---- src/tools/pgindent/typedefs.list | 1 + 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 81fff1919d..7d4ee48206 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -122,7 +122,14 @@ #include "utils/syscache.h" #include "utils/usercontext.h" -static bool table_states_valid = false; +typedef enum +{ + SYNC_TABLE_STATE_NEEDS_REBUILD, + SYNC_TABLE_STATE_REBUILD_STARTED, + SYNC_TABLE_STATE_VALID, +} SyncingTablesState; + +static SyncingTablesState table_states_validity = SYNC_TABLE_STATE_NEEDS_REBUILD; static List *table_states_not_ready = NIL; static bool FetchTableStates(bool *started_tx); @@ -272,7 +279,7 @@ wait_for_worker_state_change(char expected_state) void invalidate_syncing_table_states(Datum arg, int cacheid, uint32 hashvalue) { - table_states_valid = false; + table_states_validity = SYNC_TABLE_STATE_NEEDS_REBUILD; } /* @@ -1556,13 +1563,15 @@ FetchTableStates(bool *started_tx) *started_tx = false; - if (!table_states_valid) + if (table_states_validity != SYNC_TABLE_STATE_VALID) { MemoryContext oldctx; List *rstates; ListCell *lc; SubscriptionRelState *rstate; + table_states_validity = SYNC_TABLE_STATE_REBUILD_STARTED; + /* Clean the old lists. */ list_free_deep(table_states_not_ready); table_states_not_ready = NIL; @@ -1596,7 +1605,15 @@ FetchTableStates(bool *started_tx) has_subrels = (table_states_not_ready != NIL) || HasSubscriptionRelations(MySubscription->oid); - table_states_valid = true; + /* + * If the subscription relation cache has been invalidated since we + * entered this routine, we still use and return the relations we just + * finished constructing, to avoid infinite loops, but we leave the + * table states marked as stale so that we'll rebuild it again on next + * access. Otherwise, we mark the table states as valid. + */ + if (table_states_validity == SYNC_TABLE_STATE_REBUILD_STARTED) + table_states_validity = SYNC_TABLE_STATE_VALID; } return has_subrels; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 4ec490e362..5a9353c493 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2676,6 +2676,7 @@ SupportRequestSelectivity SupportRequestSimplify SupportRequestWFuncMonotonic Syn +SyncingTablesState SyncOps SyncRepConfigData SyncRepStandbyData -- 2.34.1