diff --git a/src/backend/distributed/metadata/metadata_cache.c b/src/backend/distributed/metadata/metadata_cache.c index 92d288b72..26371b45a 100644 --- a/src/backend/distributed/metadata/metadata_cache.c +++ b/src/backend/distributed/metadata/metadata_cache.c @@ -186,6 +186,9 @@ bool EnableVersionChecks = true; /* version checks are enabled */ static bool citusVersionKnownCompatible = false; +/* Variable to determine if we are in the process of creating citus */ +static int CreateCitusTransactionLevel = 0; + /* Hash table for informations about each partition */ static HTAB *DistTableCacheHash = NULL; static List *DistTableCacheExpired = NIL; @@ -1986,6 +1989,27 @@ CitusHasBeenLoadedInternal(void) } +/* + * GetCitusCreationLevel returns the level of the transaction creating citus + */ +int +GetCitusCreationLevel(void) +{ + return CreateCitusTransactionLevel; +} + + +/* + * Sets the value of CreateCitusTransactionLevel based on int received which represents the + * nesting level of the transaction that created the Citus extension + */ +void +SetCreateCitusTransactionLevel(int val) +{ + CreateCitusTransactionLevel = val; +} + + /* * CheckCitusVersion checks whether there is a version mismatch between the * available version and the loaded version or between the installed version diff --git a/src/backend/distributed/shared_library_init.c b/src/backend/distributed/shared_library_init.c index e72fd2fb3..65c12e92d 100644 --- a/src/backend/distributed/shared_library_init.c +++ b/src/backend/distributed/shared_library_init.c @@ -24,8 +24,11 @@ #include "safe_lib.h" #include "catalog/pg_authid.h" +#include "catalog/objectaccess.h" +#include "catalog/pg_extension.h" #include "citus_version.h" #include "commands/explain.h" +#include "commands/extension.h" #include "common/string.h" #include "executor/executor.h" #include "distributed/backend_data.h" @@ -141,9 +144,12 @@ static int ReplicationModel = REPLICATION_MODEL_STREAMING; /* we override the application_name assign_hook and keep a pointer to the old one */ static GucStringAssignHook OldApplicationNameAssignHook = NULL; +static object_access_hook_type PrevObjectAccessHook = NULL; void _PG_init(void); +static void CitusObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, int + subId, void *arg); static void DoInitialCleanup(void); static void ResizeStackToMaximumDepth(void); static void multi_log_hook(ErrorData *edata); @@ -381,6 +387,9 @@ _PG_init(void) DoInitialCleanup(); } + PrevObjectAccessHook = object_access_hook; + object_access_hook = CitusObjectAccessHook; + /* ensure columnar module is loaded at the right time */ load_file(COLUMNAR_MODULE_NAME, false); @@ -2327,3 +2336,29 @@ IsSuperuser(char *roleName) return isSuperuser; } + + +/* + * CitusObjectAccessHook is called when an object is created. + * + * We currently use it to track CREATE EXTENSION citus; operations to make sure we + * clear the metadata if the transaction is rolled back. + */ +static void +CitusObjectAccessHook(ObjectAccessType access, Oid classId, Oid objectId, int subId, + void *arg) +{ + if (PrevObjectAccessHook) + { + PrevObjectAccessHook(access, classId, objectId, subId, arg); + } + + /* Checks if the access is post_create and that it's an extension id */ + if (access == OAT_POST_CREATE && classId == ExtensionRelationId) + { + /* There's currently an engine bug that makes it difficult to check + * the provided objectId with extension oid so we will set the value + * regardless if it's citus being created */ + SetCreateCitusTransactionLevel(GetCurrentTransactionNestLevel()); + } +} diff --git a/src/backend/distributed/transaction/transaction_management.c b/src/backend/distributed/transaction/transaction_management.c index f4472ba95..a46a5f198 100644 --- a/src/backend/distributed/transaction/transaction_management.c +++ b/src/backend/distributed/transaction/transaction_management.c @@ -40,6 +40,7 @@ #include "distributed/version_compat.h" #include "distributed/worker_log_messages.h" #include "distributed/commands.h" +#include "distributed/metadata_cache.h" #include "utils/hsearch.h" #include "utils/guc.h" #include "utils/memutils.h" @@ -318,6 +319,17 @@ CoordinatedTransactionCallback(XactEvent event, void *arg) /* empty the CommitContext to ensure we're not leaking memory */ MemoryContextSwitchTo(previousContext); MemoryContextReset(CommitContext); + + /* Set CreateCitusTransactionLevel to 0 since original transaction is about to be + * committed. + */ + + if (GetCitusCreationLevel() > 0) + { + /* Check CitusCreationLevel was correctly decremented to 1 */ + Assert(GetCitusCreationLevel() == 1); + SetCreateCitusTransactionLevel(0); + } break; } @@ -362,6 +374,20 @@ CoordinatedTransactionCallback(XactEvent event, void *arg) ResetGlobalVariables(); + /* + * Clear MetadataCache table if we're aborting from a CREATE EXTENSION Citus + * so that any created OIDs from the table are cleared and invalidated. We + * also set CreateCitusTransactionLevel to 0 since that process has been aborted + */ + if (GetCitusCreationLevel() > 0) + { + /* Checks CitusCreationLevel correctly decremented to 1 */ + Assert(GetCitusCreationLevel() == 1); + + InvalidateMetadataSystemCache(); + SetCreateCitusTransactionLevel(0); + } + /* * Make sure that we give the shared connections back to the shared * pool if any. This operation is a no-op if the reserved connections @@ -609,6 +635,13 @@ CoordinatedSubTransactionCallback(SubXactEvent event, SubTransactionId subId, CoordinatedRemoteTransactionsSavepointRelease(subId); } PopSubXact(subId); + + /* Set CachedDuringCitusCreation to one level lower to represent citus creation is done */ + + if (GetCitusCreationLevel() == GetCurrentTransactionNestLevel()) + { + SetCreateCitusTransactionLevel(GetCitusCreationLevel() - 1); + } break; } @@ -631,6 +664,16 @@ CoordinatedSubTransactionCallback(SubXactEvent event, SubTransactionId subId, } PopSubXact(subId); + /* + * Clear MetadataCache table if we're aborting from a CREATE EXTENSION Citus + * so that any created OIDs from the table are cleared and invalidated. We + * also set CreateCitusTransactionLevel to 0 since subtransaction has been aborted + */ + if (GetCitusCreationLevel() == GetCurrentTransactionNestLevel()) + { + InvalidateMetadataSystemCache(); + SetCreateCitusTransactionLevel(0); + } break; } diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index f06924148..051a697c5 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -144,6 +144,8 @@ extern bool IsCitusTableType(Oid relationId, CitusTableType tableType); extern bool IsCitusTableTypeCacheEntry(CitusTableCacheEntry *tableEtnry, CitusTableType tableType); +extern void SetCreateCitusTransactionLevel(int val); +extern int GetCitusCreationLevel(void); extern bool IsCitusTable(Oid relationId); extern bool IsCitusTableViaCatalog(Oid relationId); extern char PgDistPartitionViaCatalog(Oid relationId); diff --git a/src/test/regress/expected/multi_drop_extension.out b/src/test/regress/expected/multi_drop_extension.out index 21d20ab47..2b2175367 100644 --- a/src/test/regress/expected/multi_drop_extension.out +++ b/src/test/regress/expected/multi_drop_extension.out @@ -74,6 +74,67 @@ DROP SCHEMA test_schema CASCADE; NOTICE: drop cascades to 2 other objects DROP EXTENSION citus CASCADE; \set VERBOSITY DEFAULT +-- Test if metadatacache is cleared after a rollback +BEGIN; +CREATE EXTENSION citus; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; +-- Test if metadatacache is cleared for rollback subtransations +BEGIN; +SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +ROLLBACK TO SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +COMMIT; +DROP EXTENSION citus; +-- Test if metadatacache is cleared if subtransaction commits but parent rollsback +BEGIN; +SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +RELEASE SAVEPOINT my_savepoint; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; +-- Test if metadatacache is cleared if we release a savepoint and rollback +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +RELEASE SAVEPOINT s1; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; +-- Test if metadatacache is cleared on a rollback in a nested subtransaction +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +ROLLBACK TO s1; +CREATE EXTENSION citus; +COMMIT; +DROP EXTENSION citus; +-- Test if metadatacache is cleared after columnar table is made and rollback happens +BEGIN; +SAVEPOINT s1; +CREATE EXTENSION citus; +SAVEPOINT s2; +CREATE TABLE foo1 (i int) using columnar; +SAVEPOINT s3; +ROLLBACK TO SAVEPOINT s1; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; +-- Test with a release and rollback in transactions +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +RELEASE SAVEPOINT s1; +SAVEPOINT s3; +SAVEPOINT s4; +ROLLBACK TO SAVEPOINT s3; +ROLLBACK; CREATE EXTENSION citus; -- this function is dropped in Citus10, added here for tests CREATE OR REPLACE FUNCTION pg_catalog.master_create_worker_shards(table_name text, shard_count integer, diff --git a/src/test/regress/sql/multi_drop_extension.sql b/src/test/regress/sql/multi_drop_extension.sql index 7df4a19c0..8fd1daf27 100644 --- a/src/test/regress/sql/multi_drop_extension.sql +++ b/src/test/regress/sql/multi_drop_extension.sql @@ -67,6 +67,73 @@ DROP SCHEMA test_schema CASCADE; DROP EXTENSION citus CASCADE; \set VERBOSITY DEFAULT +-- Test if metadatacache is cleared after a rollback +BEGIN; +CREATE EXTENSION citus; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; + +-- Test if metadatacache is cleared for rollback subtransations +BEGIN; +SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +ROLLBACK TO SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +COMMIT; +DROP EXTENSION citus; + +-- Test if metadatacache is cleared if subtransaction commits but parent rollsback +BEGIN; +SAVEPOINT my_savepoint; +CREATE EXTENSION citus; +RELEASE SAVEPOINT my_savepoint; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; + +-- Test if metadatacache is cleared if we release a savepoint and rollback +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +RELEASE SAVEPOINT s1; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; + +-- Test if metadatacache is cleared on a rollback in a nested subtransaction +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +ROLLBACK TO s1; +CREATE EXTENSION citus; +COMMIT; +DROP EXTENSION citus; + +-- Test if metadatacache is cleared after columnar table is made and rollback happens +BEGIN; +SAVEPOINT s1; +CREATE EXTENSION citus; +SAVEPOINT s2; +CREATE TABLE foo1 (i int) using columnar; +SAVEPOINT s3; +ROLLBACK TO SAVEPOINT s1; +ROLLBACK; +CREATE EXTENSION citus; +DROP EXTENSION citus; + +-- Test with a release and rollback in transactions +BEGIN; +SAVEPOINT s1; +SAVEPOINT s2; +CREATE EXTENSION citus; +RELEASE SAVEPOINT s1; +SAVEPOINT s3; +SAVEPOINT s4; +ROLLBACK TO SAVEPOINT s3; +ROLLBACK; CREATE EXTENSION citus; -- this function is dropped in Citus10, added here for tests