This is an automated email from the ASF dual-hosted git repository. yjhjstz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 621015af4422c09822c93956d1ab987384623f90 Author: wuchengwen <[email protected]> AuthorDate: Tue Sep 26 10:30:57 2023 +0800 Fix orphaned temp namespace catalog entry left on coordinator Note that, commit e88ceb8 fixed orphaned temp table left on coordinator, but it did't handle orphaned namespace catalog entry left on coordinator. This commit fix that. --- src/backend/catalog/namespace.c | 22 ++++++++++++++++++++ src/backend/cdb/dispatcher/cdbgang.c | 24 +++++++++++++++++++++- src/backend/cdb/dispatcher/test/cdbgang_test.c | 3 +++ src/include/catalog/namespace.h | 1 + src/test/isolation2/expected/orphan_temp_table.out | 6 ++++++ src/test/isolation2/sql/orphan_temp_table.sql | 2 ++ 6 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 42465ef89a..6f3c4e1574 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -4355,6 +4355,28 @@ DropTempTableNamespaceForResetSession(Oid namespaceOid) CommitTransactionCommand(); } +/* + * Remove temp namespace entry from pg_namespace. + */ +void +DropTempTableNamespaceEntryForResetSession(Oid namespaceOid, Oid toastNamespaceOid) +{ + if (IsTransactionOrTransactionBlock()) + elog(ERROR, "Called within a transaction"); + + StartTransactionCommand(); + + /* Make sure the temp namespace is valid. */ + if (SearchSysCacheExists1(NAMESPACEOID, + ObjectIdGetDatum(namespaceOid))) + { + RemoveSchemaById(namespaceOid); + RemoveSchemaById(toastNamespaceOid); + } + + CommitTransactionCommand(); +} + /* * Called by CreateSchemaCommand when creating a temporary schema */ diff --git a/src/backend/cdb/dispatcher/cdbgang.c b/src/backend/cdb/dispatcher/cdbgang.c index c73192f0ad..8fc1bfbe0d 100644 --- a/src/backend/cdb/dispatcher/cdbgang.c +++ b/src/backend/cdb/dispatcher/cdbgang.c @@ -81,6 +81,7 @@ CreateGangFunc pCreateGangFunc = cdbgang_createGang_async; static bool NeedResetSession = false; static Oid OldTempNamespace = InvalidOid; +static Oid OldTempToastNamespace = InvalidOid; /* * cdbgang_createGang: @@ -786,6 +787,7 @@ GpDropTempTables(void) int oldSessionId = 0; int newSessionId = 0; Oid dropTempNamespaceOid; + Oid dropTempToastNamespaceOid; /* No need to reset session or drop temp tables */ if (!NeedResetSession && OldTempNamespace == InvalidOid) @@ -827,14 +829,20 @@ GpDropTempTables(void) } dropTempNamespaceOid = OldTempNamespace; + dropTempToastNamespaceOid = OldTempToastNamespace; OldTempNamespace = InvalidOid; + OldTempToastNamespace = InvalidOid; NeedResetSession = false; if (dropTempNamespaceOid != InvalidOid) { + MemoryContext oldcontext = CurrentMemoryContext; + PG_TRY(); { DropTempTableNamespaceForResetSession(dropTempNamespaceOid); + /* drop pg_temp_N schema entry from pg_namespace */ + DropTempTableNamespaceEntryForResetSession(dropTempNamespaceOid, dropTempToastNamespaceOid); } PG_CATCH(); { /* @@ -847,7 +855,10 @@ GpDropTempTables(void) } EmitErrorReport(); + + MemoryContextSwitchTo(oldcontext); FlushErrorState(); + AbortCurrentTransaction(); } PG_END_TRY(); } } @@ -855,7 +866,13 @@ GpDropTempTables(void) void resetSessionForPrimaryGangLoss(void) { - if (ProcCanSetMppSessionId()) + /* + * resetSessionForPrimaryGangLoss could be called twice in a transacion, + * we need to use NeedResetSession to double check if we should do the + * real work to avoid that OldTempToastNamespace be makred invalid before + * cleaning up the temp namespace. + */ + if (ProcCanSetMppSessionId() && !NeedResetSession) { /* * Not too early. @@ -871,6 +888,8 @@ resetSessionForPrimaryGangLoss(void) */ if (TempNamespaceOidIsValid()) { + + OldTempToastNamespace = GetTempToastNamespace(); /* * Here we indicate we don't have a temporary table namespace * anymore so all temporary tables of the previous session will be @@ -886,7 +905,10 @@ resetSessionForPrimaryGangLoss(void) gp_session_id); } else + { OldTempNamespace = InvalidOid; + OldTempToastNamespace = InvalidOid; + } } } diff --git a/src/backend/cdb/dispatcher/test/cdbgang_test.c b/src/backend/cdb/dispatcher/test/cdbgang_test.c index 9535e84552..3a6438d8f3 100644 --- a/src/backend/cdb/dispatcher/test/cdbgang_test.c +++ b/src/backend/cdb/dispatcher/test/cdbgang_test.c @@ -104,10 +104,13 @@ test__resetSessionForPrimaryGangLoss(void **state) /* Assum we have created a temporary namespace. */ will_return(TempNamespaceOidIsValid, true); + will_return(GetTempToastNamespace, 9998); will_return(ResetTempNamespace, 9999); OldTempNamespace = InvalidOid; + OldTempToastNamespace = InvalidOid; resetSessionForPrimaryGangLoss(); + assert_int_equal(OldTempToastNamespace, 9998); assert_int_equal(OldTempNamespace, 9999); } diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 1d8c4340f4..13c03e65ce 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -143,6 +143,7 @@ extern Oid LookupExplicitNamespace(const char *nspname, bool missing_ok); extern Oid get_namespace_oid(const char *nspname, bool missing_ok); extern void DropTempTableNamespaceForResetSession(Oid namespaceOid); +extern void DropTempTableNamespaceEntryForResetSession(Oid namespaceOid, Oid toastNamespaceOid); extern void SetTempNamespace(Oid namespaceOid, Oid toastNamespaceOid); extern Oid ResetTempNamespace(void); extern bool TempNamespaceOidIsValid(void); /* GPDB only: used by cdbgang.c */ diff --git a/src/test/isolation2/expected/orphan_temp_table.out b/src/test/isolation2/expected/orphan_temp_table.out index 51e02e6423..19dfd7160b 100644 --- a/src/test/isolation2/expected/orphan_temp_table.out +++ b/src/test/isolation2/expected/orphan_temp_table.out @@ -21,6 +21,12 @@ ERROR: fault triggered, fault name:'before_exec_scan' fault type:'panic' (seg0 oid | relname | relnamespace -----+---------+-------------- (0 rows) +-- we should not see the temp namespace on the coordinator +1: SELECT count(*) FROM pg_namespace where (nspname like '%pg_temp_%' or nspname like '%pg_toast_temp_%') and oid > 16386; + count +------- + 0 +(1 row) -- the temp table is left on segment 0, it should be dropped by autovacuum later diff --git a/src/test/isolation2/sql/orphan_temp_table.sql b/src/test/isolation2/sql/orphan_temp_table.sql index a6a7548718..3a4db41b69 100644 --- a/src/test/isolation2/sql/orphan_temp_table.sql +++ b/src/test/isolation2/sql/orphan_temp_table.sql @@ -12,6 +12,8 @@ -- we should not see the temp table on the coordinator 1: SELECT oid, relname, relnamespace FROM pg_class where relname = 'test_temp_table_cleanup'; +-- we should not see the temp namespace on the coordinator +1: SELECT count(*) FROM pg_namespace where (nspname like '%pg_temp_%' or nspname like '%pg_toast_temp_%') and oid > 16386; -- the temp table is left on segment 0, it should be dropped by autovacuum later --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
