Clear metadatacache during abort for create extension (#5907)

* Bug fix for bug #5876. Memset MetadataCacheSystem every time there is an abort

* Created an ObjectAccessHook that saves the transactionlevel of when citus was created and will clear metadatacache if that transaction level is rolled back. Added additional tests to make sure metadatacache is cleared
pull/5910/head
Ying Xu 2022-05-20 13:47:58 -07:00 committed by GitHub
parent c692bb10f7
commit a1151c2395
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 232 additions and 0 deletions

View File

@ -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

View File

@ -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());
}
}

View File

@ -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;
}

View File

@ -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);

View File

@ -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,

View File

@ -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