From 0f84ed807be883f799610c5a3655c53ca87a2e62 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 17 Dec 2024 19:09:56 +0300 Subject: [PATCH] Avoid re-assigning the global pid when we cannot retrieve local node id without unsafe catalog access (cherry picked from commit d06c0cd95812f0c3af679d521dcc23bc645a2ac0) --- src/backend/distributed/metadata/metadata_cache.c | 10 ++++++++++ src/backend/distributed/shared_library_init.c | 10 +++++++++- src/include/distributed/metadata_cache.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/metadata/metadata_cache.c b/src/backend/distributed/metadata/metadata_cache.c index 4f1b942a0..3bf525dd8 100644 --- a/src/backend/distributed/metadata/metadata_cache.c +++ b/src/backend/distributed/metadata/metadata_cache.c @@ -4545,6 +4545,16 @@ GetLocalNodeId(void) } +/* + * CachedLocalNodeIdIsValid return true if the cached local node id is valid. + */ +bool +CachedLocalNodeIdIsValid(void) +{ + return LocalNodeId != -1; +} + + /* * RegisterLocalGroupIdCacheCallbacks registers the callbacks required to * maintain LocalGroupId at a consistent value. It's separate from diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index bd65fa60c..28a2da7b5 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -2899,6 +2899,13 @@ ApplicationNameAssignHook(const char *newval, void *extra) * So we set the FinishedStartupCitusBackend flag in StartupCitusBackend to * indicate when this responsibility handoff has happened. * + * On the other hand, even if now it's this hook's responsibility to update + * the global pid, we cannot do so if the cached local node id is invalidated + * and we're not allowed to access the catalog tables. Within a transaction + * block, we can access the catalog tables. For this reason, in addition to + * checking FinishedStartupCitusBackend, we also require either being in a + * transaction block or the cached local node id to be valid. + * * Another solution to the catalog table acccess problem would be to update * global pid lazily, like we do for HideShards. But that's not possible * for the global pid, since it is stored in shared memory instead of in a @@ -2907,7 +2914,8 @@ ApplicationNameAssignHook(const char *newval, void *extra) * as reasonably possible, which is also why we extract global pids in the * AuthHook already (extracting doesn't require catalog access). */ - if (FinishedStartupCitusBackend) + if (FinishedStartupCitusBackend && + (IsTransactionState() || CachedLocalNodeIdIsValid())) { AssignGlobalPID(newval); } diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index f1120497b..d4779a98a 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -181,6 +181,7 @@ extern CitusTableCacheEntry * LookupCitusTableCacheEntry(Oid relationId); extern DistObjectCacheEntry * LookupDistObjectCacheEntry(Oid classid, Oid objid, int32 objsubid); extern int32 GetLocalGroupId(void); +extern bool CachedLocalNodeIdIsValid(void); extern int32 GetLocalNodeId(void); extern void CitusTableCacheFlushInvalidatedEntries(void); extern Oid LookupShardRelationFromCatalog(int64 shardId, bool missing_ok);