Refactor tests which use debug_dtm_action to use faultinjector by vbwagner · Pull Request #1967 · arenadata/gpdb

vbwagner · 2025-08-22T06:41:20Z

Refactor tests which use debug_dtm_action to use faultinjector

gpdb tests use two mechanisms to generate artificial faults.

gp_fault_injector extension with corresponding backend code;
set of GUC variables with names beginning with debug_dtm_action.

This patch changes all tests from debug_dtm_action to faultinjector.

It includes following extensions to faultinjector:

Additional condition to choose whether to throw fault or not —
subtransaction nesting level;
Compatibility SQL function which allows avoiding changes in old test which
use gp_inject_fault without that new condition;
Extending DDLStatement_e enumeration to include all SQL operators and DTX
protocol messages used in tests, which previously used debug_dtm_action;
wrappers to fault injection point function which translate textual command
tags and DTX protocol message code to DDLStatement_e;
Improved README for gp_inject_fault extension, which documents these
changes and also documents some already existing functionality which was
left undocumented.

Apart from faultinjector changes patch includes following changes:

Replacing of code blocks which check debug_dtm_action variables with
calls to faultinjector macros;
Removal of debug_dtm_action_* GUC variables from guc_gp.cl
Replacing of SET debug_dtm_action* in the tests to calls to
gp_inject_fault function;
Modifications of expected test results which include changed text of
injected error and echo of SQL statements which inject an error.

gpcontrib/gp_inject_fault/gp_inject_fault.c

gpcontrib/gp_inject_fault/README

src/backend/tcop/postgres.c

src/backend/utils/misc/faultinjector.c

gpcontrib/gp_inject_fault/gp_inject_fault--1.0.sql

whitehawk · 2025-08-25T07:02:15Z

In the PR description:
'to generate artifitial faults' -> 'to generate artificial faults'
'Removal of debug_dtm_action_* GUC variables form guc_gp.c' -> 'Removal of debug_dtm_action_* GUC variables from guc_gp.c'
?

src/test/isolation2/sql/udf_exception_blocks_panic_scenarios.sql

src/test/regress/greenplum_schedule

src/test/regress/sql/udf_exception_blocks.sql

src/test/isolation2/sql/udf_exception_blocks_panic_scenarios.sql

src/test/regress/expected/distributed_transactions.out

src/backend/utils/misc/faultinjector.c

src/include/utils/faultinjector_lists.h

src/backend/utils/misc/faultinjector.c

src/include/utils/faultinjector.h

src/test/regress/sql/distributed_transactions.sql

gpcontrib/gp_inject_fault/README

vbwagner · 2025-08-27T07:52:52Z

Names in guc.h are still presented.

Thanks for pointing to guc.h. Fixed

src/backend/tcop/postgres.c

src/include/utils/guc.h

src/test/regress/sql/distributed_transactions.sql

src/include/utils/faultinjector_lists.h

Following changes are made to faultinjector code 1. In the gp_inject_fault extension, one extra parameter added to C language function gp_inject_fault - nestingLevel for nesting subtransaction 2. Compatibility SQL function with same number of argumetns as former C function is created to avoid incompatibilites. 3. DDLStatement_e enumeration is extended to cover all SQL and DTX statements which are used in tests with debug_dtm_action (they are not all ddl, but...) 4. Since string names of some of these statements as they are used in the code, contain spaces, scanf format in HandleFaultMessage is changed to handle strings with spaces. 5. Parameter nestingLevel is added to function FaultInjector_InjectFaultIfSet_out_of line. 6. Since SQL Statement Tag present in the context where fault injection code is called as string, created new function FaultInjector_InjectFaultIfSet_SQL_out_of_line and corresponding macro which wraps basic fault injection function and translates string statement info DDLStatement_e value 7. Since DTXProtocol command is available as enumeration, but not same as DLLStatement_e, recoding table is added to faultinjector.c and function FaultInjector_InjectFaultIfSet_DTX_out of_line, which analogous to prevoius step. 8. To make definition of DtxtProtocolCommand enumeration available in the context of faultinjector.h (where function from previous point is declared) header files postgres.h and cdb/cdbtm.h are included into this file.

There were three functions where debug_dtm_action GUC variables were checked and faults thrown: exec_mpp_query exec_simple_query and exec_mpp_dtx_protocol_command First two have two ereport calls at start and at end of functions. Last have three ereport calls because at start it was possible to throw either ERROR or PANIC. Faultinjector is able to throw both error and panic from any incection point, so there are now just two injection points in all three functions. Functions exec_mpp_query and exec_simple_query have string variable commandTag with printable of command. So we use this value to define ddlStatement condition For exec_mpp_dtx_query we use dtxProtocolCommand with enumerated type DtxProtocolCommand. Injection points are named dtm_exec_mpp_query_start dtm_exec_mpp_query_end dtm_exec_simple_command_start dtm_exec_simple_command_end exec_mpp_dtx_protocol_start exec_mpp_dtx_protocol_end Old static helper functions CheckDebugDtmActionSqlCommandTag and CheckDebugDtmActionProtocol are removed.

This commit removes all debug_dtm_action* GUC variables and corresponing global variables, defines, string-to-enum recoding tables. It is made separate commit because we have to be sure that our replacement of these variables with gp_inject_fault function is complete.

This commit replaces all references for debug_dtm_action* GUC variables in the tests (including helper C program twophase_pqexecparams.c) with calls to gp_inject_fault function. This also requires changes to src/test/regress/greenplum_schedule because there were no check that tests which inject faults using debug_dtm_action GUC variables are not executed as part of parallel group, but such check present for tests which use gp_inject_fault. And it is probaly right thing because injected fault may unexpectedly disrupt parallel sessions. If GUC variables are very seldom reset before next test, gp_inject_fault requires reset each time before injecting to same injection point with different parameters.

Expected results of these tests have to be altered, because expeced results contain all SQL commands and their output, so SET debug_dtm_action* have to be removed and SELECT gp_inject_fault(...) FROM... added. Also, when injected fault message not caught, it also have to be changed.

Documented additional fault injection macros and parameters required to replace debug_dtm_action variables, and documented some previously undocumented parts of faultinjector.

Refactoring of faultinjector and removing GUC variables cause ABI changes. This change is harmless because this GUC variables are never used outside tests and faultinjector is not compiled in in production builds. Moreover, compatibility macros are provided to preserve to FaultInjector_InjectFaultIfSet behavoir

Improve formatting of calls to gp_inject_fault, remove diffs with old expected results which are ignored during computation of regression.diffs

src/backend/utils/misc/faultinjector.c

src/include/utils/faultinjector.h

src/test/isolation2/expected/udf_exception_blocks_panic_scenarios.out

src/test/regress/sql/udf_exception_blocks.sql

src/test/regress/sql/not_out_of_shmem_exit_slots.sql

Remove erroneuously added empty line in src/test/regress/sql/not_out_of_shmem_exit_slots.sql and return back end-of-line space in src/test/regress/sql/udf_exception_blocks.sql Revert changes of line order in query output in src/test/isolation2/expected/udf_exception_blocks_panic_scenarios.out because this line order is ignored during diff.

Leave only brief usage instructions in faultinjector.h Also reformat function FaultInjector_InjectFault_out_of_line_SQL.

vbwagner · 2025-09-05T09:28:59Z

The PR description still requires adjustments according to our check-list.

Fixed paragraph formatting to 80 chars/per line. Any other suggestions?

vbwagner · 2025-09-05T09:29:16Z

In the PR description: 'patch include' -> 'patch includes' 'form guc_gp.c' -> 'from guc_gp.c'

Fixed

This comment was marked as resolved.

Sign in to view