Skip to content

Commit

Permalink
Revert "Avoid re-assigning the global pid when we cannot retrieve loc…
Browse files Browse the repository at this point in the history
…al node id without unsafe catalog access"

This reverts commit d06c0cd.
  • Loading branch information
onurctirtir committed Dec 18, 2024
1 parent 1754f9f commit 4745a88
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 20 deletions.
10 changes: 0 additions & 10 deletions src/backend/distributed/metadata/metadata_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -4545,16 +4545,6 @@ 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
Expand Down
10 changes: 1 addition & 9 deletions src/backend/distributed/shared_library_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -2899,13 +2899,6 @@ 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
Expand All @@ -2914,8 +2907,7 @@ 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 &&
(IsTransactionState() || CachedLocalNodeIdIsValid()))
if (FinishedStartupCitusBackend)
{
AssignGlobalPID(newval);
}
Expand Down
1 change: 0 additions & 1 deletion src/include/distributed/metadata_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ 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);
Expand Down

0 comments on commit 4745a88

Please sign in to comment.