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 01876c9d18ba263544d2f54c56dd9fab2bf2874e Author: wuchengwen <[email protected]> AuthorDate: Wed Oct 25 09:39:28 2023 +0800 Fix orphaned temp table on coordinator When session exits within a transaction block, temp table created in this session will be left. This is because ResetAllGangs will reset myTempNamespace, normal temp table catalog clean up won't be called. More details could ref https://github.com/greenplum-db/gpdb/issues/16506 Noted that, when a session is going to exit on coordinator, there is no need to drop temp tables by calling GpDropTempTables, callback function will do that at the end. --- src/backend/cdb/cdbtm.c | 41 ++++++++++++++++------ src/test/isolation2/expected/orphan_temp_table.out | 21 ++++++++++- src/test/isolation2/sql/orphan_temp_table.sql | 12 ++++++- 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/src/backend/cdb/cdbtm.c b/src/backend/cdb/cdbtm.c index e22c5c27a0..e32d7060ea 100644 --- a/src/backend/cdb/cdbtm.c +++ b/src/backend/cdb/cdbtm.c @@ -961,13 +961,30 @@ rollbackDtxTransaction(void) break; case DTX_STATE_NOTIFYING_ABORT_NO_PREPARED: - /* - * By deallocating the gang, we will force a new gang to connect - * to all the segment instances. And, we will abort the - * transactions in the segments. - */ - elog(NOTICE, "Releasing segworker groups to finish aborting the transaction."); - ResetAllGangs(); + if (!proc_exit_inprogress) + { + /* + * By deallocating the gang, we will force a new gang to connect + * to all the segment instances. And, we will abort the + * transactions in the segments. + * + * Reset session ID and drop temp tables only when process does *not* exits, + * because otherwise, proc_exit will do that eventually anyway. + */ + elog(NOTICE, "Releasing segworker groups to finish aborting the transaction."); + ResetAllGangs(); + } + else + { + /* + * Destroy all gangs early, so that they won't block any other QEs due to 2PC lock + * when QD might be just retrying `rollbackDtxTransaction` for a prolonged time. + * + * Do not reset session just yet, because we want to keep myTempNamespace untouched + * and let RemoveTempRelationsCallback() drops temp tables as part of proc_exit. + */ + DisconnectAndDestroyAllGangs(false); + } return; case DTX_STATE_NOTIFYING_ABORT_SOME_PREPARED: @@ -1024,11 +1041,13 @@ rollbackDtxTransaction(void) Assert(MyTmGxactLocal->state == DTX_STATE_NOTIFYING_ABORT_NO_PREPARED); /* - * By deallocating the gang, we will force a new gang to connect to - * all the segment instances. And, we will abort the transactions in - * the segments. + * Destroy all gangs early, so that they won't block any other QEs due to 2PC lock + * when QD might be just retrying `rollbackDtxTransaction` for a prolonged time. + * + * Do not reset session just yet, because we want to keep myTempNamespace untouched + * and let RemoveTempRelationsCallback() drops temp tables as part of proc_exit. */ - ResetAllGangs(); + DisconnectAndDestroyAllGangs(false); return; } diff --git a/src/test/isolation2/expected/orphan_temp_table.out b/src/test/isolation2/expected/orphan_temp_table.out index 19dfd7160b..85cbe16500 100644 --- a/src/test/isolation2/expected/orphan_temp_table.out +++ b/src/test/isolation2/expected/orphan_temp_table.out @@ -1,6 +1,6 @@ -- Test orphan temp table on coordinator. --- Before the fix, when backend process panic on the segment, the temp table will be left on the coordinator. +-- case 1: Before the fix, when backend process panic on the segment, the temp table will be left on the coordinator. -- create a temp table 1: CREATE TEMP TABLE test_temp_table_cleanup(a int); CREATE @@ -47,3 +47,22 @@ ERROR: fault triggered, fault name:'before_exec_scan' fault type:'panic' (seg0 ----------------- Success: (1 row) +1q: ... <quitting> + +-- case 2: Test if temp table will be left on the coordinator, when session exits in coordinator within a transaction block. +2: CREATE TEMP TABLE test_temp_table_cleanup(a int); +CREATE +2: begin; +BEGIN +2: select * from test_temp_table_cleanup; + a +--- +(0 rows) +2q: ... <quitting> + +3: select count(*) from pg_class where relname = 'test_temp_table_cleanup'; + count +------- + 0 +(1 row) +3q: ... <quitting> diff --git a/src/test/isolation2/sql/orphan_temp_table.sql b/src/test/isolation2/sql/orphan_temp_table.sql index 3a4db41b69..d4681bdaa6 100644 --- a/src/test/isolation2/sql/orphan_temp_table.sql +++ b/src/test/isolation2/sql/orphan_temp_table.sql @@ -1,6 +1,6 @@ -- Test orphan temp table on coordinator. --- Before the fix, when backend process panic on the segment, the temp table will be left on the coordinator. +-- case 1: Before the fix, when backend process panic on the segment, the temp table will be left on the coordinator. -- create a temp table 1: CREATE TEMP TABLE test_temp_table_cleanup(a int); @@ -23,3 +23,13 @@ 1U: SELECT oid, relname, relnamespace FROM pg_class where relname = 'test_temp_table_cleanup'; 1: SELECT gp_inject_fault('before_exec_scan', 'reset', dbid) FROM gp_segment_configuration WHERE role='p' AND content = 0; +1q: + +-- case 2: Test if temp table will be left on the coordinator, when session exits in coordinator within a transaction block. +2: CREATE TEMP TABLE test_temp_table_cleanup(a int); +2: begin; +2: select * from test_temp_table_cleanup; +2q: + +3: select count(*) from pg_class where relname = 'test_temp_table_cleanup'; +3q: --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
