From 5b6fcda548e4cd8f6ea026c88e4859bd658b9005 Mon Sep 17 00:00:00 2001 From: Abhijit Subramanya Date: Fri, 3 Mar 2017 16:35:05 -0800 Subject: [PATCH] Add new fault injector to make the transaction management test more stable. The fault `twophase_transaction_commit_prepared` would cause the segment to PANIC while writing a `COMMIT PREPARED` record on the segment. The master would then retry the `COMMIT PREPARED` while the postmaster was still resetting on the segment causing the master itself to `PANIC`. This patch introduces a new fault injector `finish_prepared_before_commit` which will not cause a `PANIC`, but just error out since the intent of the test is to ensure that the retry works correctly. --- gpMgmt/bin/gppylib/programs/clsInjectFault.py | 1 + src/backend/access/transam/twophase.c | 2 ++ src/backend/utils/misc/faultinjector.c | 2 ++ src/include/utils/faultinjector.h | 1 + src/test/tinc/Makefile | 2 +- .../transaction_management/mpp23395/test_mpp23395.py | 8 +++----- 6 files changed, 10 insertions(+), 6 deletions(-) diff --git a/gpMgmt/bin/gppylib/programs/clsInjectFault.py b/gpMgmt/bin/gppylib/programs/clsInjectFault.py index 36abe8b1c7..18bd13f9fc 100644 --- a/gpMgmt/bin/gppylib/programs/clsInjectFault.py +++ b/gpMgmt/bin/gppylib/programs/clsInjectFault.py @@ -424,6 +424,7 @@ class GpInjectFaultProgram: "cursor_qe_reader_after_snapshot (inject fault after QE READER has populated snashot for cursor)" \ "fsync_counter (inject fault to count buffers fsync'ed by checkpoint process), " \ "bg_buffer_sync_default_logic (inject fault to 'skip' in order to flush all buffers in BgBufferSync()), " \ + "finish_prepared_after_record_commit_prepared (inject fault in FinishPreparedTransaction() after recording the commit prepared record), " \ "all (affects all faults injected, used for 'status' and 'reset'), ") addTo.add_option("-c", "--ddl_statement", dest="ddlStatement", type="string", metavar="ddlStatement", diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index a07c049592..82143857c6 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -1646,6 +1646,8 @@ FinishPreparedTransaction(const char *gid, bool isCommit, bool raiseErrorIfNotFo END_CRIT_SECTION(); + SIMPLE_FAULT_INJECTOR(FinishPreparedAfterRecordCommitPrepared); + /* Need to figure out the memory allocation and deallocationfor "buffer". For now, just let it leak. */ return true; diff --git a/src/backend/utils/misc/faultinjector.c b/src/backend/utils/misc/faultinjector.c index 1ac56f0344..9dbfda551e 100644 --- a/src/backend/utils/misc/faultinjector.c +++ b/src/backend/utils/misc/faultinjector.c @@ -331,6 +331,8 @@ FaultInjectorIdentifierEnumToString[] = { /* inject fault to 'skip' in order to flush all buffers in BgBufferSync() */ _("bg_buffer_sync_default_logic"), /* inject fault to count buffers fsync'ed by checkpoint process */ + _("finish_prepared_after_record_commit_prepared"), + /* inject fault in FinishPreparedTransaction() after recording the commit prepared record */ _("not recognized"), }; diff --git a/src/include/utils/faultinjector.h b/src/include/utils/faultinjector.h index 1613cc15b2..35257021ba 100644 --- a/src/include/utils/faultinjector.h +++ b/src/include/utils/faultinjector.h @@ -221,6 +221,7 @@ typedef enum FaultInjectorIdentifier_e { FsyncCounter, BgBufferSyncDefaultLogic, + FinishPreparedAfterRecordCommitPrepared, /* INSERT has to be done before that line */ FaultInjectorIdMax, diff --git a/src/test/tinc/Makefile b/src/test/tinc/Makefile index fc7cbb2379..f2d2d436ac 100644 --- a/src/test/tinc/Makefile +++ b/src/test/tinc/Makefile @@ -65,7 +65,7 @@ non_discover_targets: aoco_compression filerep_end_to_end fts # stages for configuration requirements. storage: storage_vacuum_xidlimits aocoalter_catalog_loaders \ - storage_queryfinish_and_transactionmanagement \ + storage_uao_and_transactionmanagement \ storage_persistent_accessmethods_and_vacuum \ storage_filerep diff --git a/src/test/tinc/tincrepo/mpp/gpdb/tests/storage/transaction_management/mpp23395/test_mpp23395.py b/src/test/tinc/tincrepo/mpp/gpdb/tests/storage/transaction_management/mpp23395/test_mpp23395.py index c9314badf9..5513067744 100644 --- a/src/test/tinc/tincrepo/mpp/gpdb/tests/storage/transaction_management/mpp23395/test_mpp23395.py +++ b/src/test/tinc/tincrepo/mpp/gpdb/tests/storage/transaction_management/mpp23395/test_mpp23395.py @@ -181,19 +181,17 @@ class mpp23395(MPPTestCase): SET debug_dtm_action = "fail_begin_command"; CREATE TABLE mpp23395(a int); ''' - self.run_sequence(sql, 'twophase_transaction_commit_prepared', 'error', 2, False); + self.run_sequence(sql, 'finish_prepared_after_record_commit_prepared', 'error', 2, False); - # QE panics after writing prepare xlog record. This should cause - # master to broadcast abort but QEs handle the abort in + # Scenario 8: QE panics after writing prepare xlog record. This should + # cause master to broadcast abort but QEs handle the abort in # DTX_CONTEXT_LOCAL_ONLY context. sql = ''' DROP TABLE IF EXISTS mpp23395; CREATE TABLE mpp23395(a int); INSERT INTO mpp23395 VALUES(1), (2), (3); - BEGIN; SET debug_abort_after_segment_prepared = true; DELETE FROM mpp23395; - COMMIT; ''' # No prepared transactions should remain lingering -- GitLab