diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index 1aff542ec7fc5042b9b43808c32e8923d7222dd9..8276d27d9cda0ed80b7f96b4365f61e93137123d 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -263,6 +263,8 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN, bool commit) break; } + SIMPLE_FAULT_INJECTOR("sync_rep_query_die"); + /* * If a wait for synchronous replication is pending, we can neither * acknowledge the commit nor raise ERROR or FATAL. The latter would @@ -278,13 +280,14 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN, bool commit) if (ProcDiePending) { /* - * FATAL only for QE's which use 2PC and hence can handle the - * FATAL and retry. + * For QE we should have done FATAL here so that 2PC can retry, but + * FATAL here makes some shm exit callback functions panic or + * assert fail because the transaction is still not finished, so + * let's defer the quitting to exec_mpp_dtx_protocol_command(). */ - ereport(IS_QUERY_DISPATCHER() ? WARNING:FATAL, + ereport(WARNING, (errcode(ERRCODE_ADMIN_SHUTDOWN), - errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"), - errdetail("The transaction has already committed locally, but might not have been replicated to the standby."))); + errmsg("canceling the wait for synchronous replication and terminating connection due to administrator command"))); whereToSendOutput = DestNone; SyncRepCancelWait(); break; @@ -320,6 +323,9 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN, bool commit) if (!PostmasterIsAlive()) { ProcDiePending = true; + ereport(WARNING, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("canceling the wait for synchronous replication and terminating connection due to postmaster death"))); whereToSendOutput = DestNone; SyncRepCancelWait(); break; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 888c31e259e57b86d064ef0ff90547ec36a27fd3..a54ac7e3a3e826850bb5a58ec7971b98b4b54aea 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1523,6 +1523,16 @@ exec_mpp_dtx_protocol_command(DtxProtocolCommand dtxProtocolCommand, Debug_dtm_action, DtxProtocolCommandToString(dtxProtocolCommand)))); } + /* + * GPDB: There is a corner case that we need to delay connection + * termination to here. see SyncRepWaitForLSN() for details. + * */ + if (ProcDiePending) + ereport(FATAL, + (errcode(ERRCODE_ADMIN_SHUTDOWN), + errmsg("Terminating the connection (DTM protocol command '%s' " + "for gid=%s", loggingStr, gid))); + EndCommand(commandTag, dest); } diff --git a/src/test/isolation2/expected/segwalrep/die_commit_pending_replication.out b/src/test/isolation2/expected/segwalrep/die_commit_pending_replication.out new file mode 100644 index 0000000000000000000000000000000000000000..28d0e53b4a4f1061bef706ec04f3fa24c9f0ea3b --- /dev/null +++ b/src/test/isolation2/expected/segwalrep/die_commit_pending_replication.out @@ -0,0 +1,107 @@ +-- Adding `2` as first column as the distribution column. +-- `2` should be on the first segment. let's double check here. +create table store_session_id(a int, sess_id int); +CREATE +1: insert into store_session_id select 2, sess_id from pg_stat_activity where pid = pg_backend_pid(); +INSERT 1 +1: select gp_segment_id, a from store_session_id; + gp_segment_id | a +---------------+--- + 0 | 2 +(1 row) + +1: create table die_commit_pending_replication(a int, b int); +CREATE + +-- Suspend to hit commit-prepared point on segment (as we are +-- interested in testing Commit here and not really Prepare) +select gp_inject_fault_infinite('finish_prepared_start_of_function', 'suspend', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault_infinite +-------------------------- + Success: +(1 row) +1&: insert into die_commit_pending_replication values(2),(1); +select gp_wait_until_triggered_fault('finish_prepared_start_of_function', 1, dbid) from gp_segment_configuration where role='p' and content = 0; + gp_wait_until_triggered_fault +------------------------------- + Success: +(1 row) + +-- Now pause the wal sender on primary for content 0 +select gp_inject_fault_infinite('wal_sender_loop', 'suspend', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault_infinite +-------------------------- + Success: +(1 row) + +-- Insert fault in the ProcDiePending code block. +select gp_inject_fault_infinite('sync_rep_query_die', 'skip', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault_infinite +-------------------------- + Success: +(1 row) + +-- Let the transaction move forward with the commit +select gp_inject_fault('finish_prepared_start_of_function', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault +----------------- + Success: +(1 row) + +-- Ensure the ProcDiePending code block is called. It implies 'replication' in pg_stat_activity. +select gp_wait_until_triggered_fault('sync_rep_query_die', 1, dbid) from gp_segment_configuration where role='p' and content = 0; + gp_wait_until_triggered_fault +------------------------------- + Success: +(1 row) + +-- We can terminate the backend on QE now. +0U: select pg_terminate_backend(pid) from pg_stat_activity where waiting_reason='replication' and sess_id in (select sess_id from store_session_id); + pg_terminate_backend +---------------------- + t +(1 row) + +-- We expect two more occurrence: one for backend quitting and another for retry. +select gp_wait_until_triggered_fault('sync_rep_query_die', 3, dbid) from gp_segment_configuration where role='p' and content = 0; + gp_wait_until_triggered_fault +------------------------------- + Success: +(1 row) + +-- Verify that the sess_id changes due to retry. +0U: select pid,sess_id,waiting_reason,query from pg_stat_activity where sess_id in (select sess_id from store_session_id); + pid | sess_id | waiting_reason | query +-----+---------+----------------+------- +(0 rows) + +-- resume the primary wal replication so that retry could complete. +select gp_inject_fault('wal_sender_loop', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault +----------------- + Success: +(1 row) + +-- reset sync_rep_query_die +select gp_inject_fault('sync_rep_query_die', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + gp_inject_fault +----------------- + Success: +(1 row) + +1<: <... completed> +INSERT 2 + +-- check if the insert fails or not. +select gp_segment_id, * from die_commit_pending_replication; + gp_segment_id | a | b +---------------+---+--- + 0 | 2 | + 1 | 1 | +(2 rows) + +-- cleanup +drop table die_commit_pending_replication; +DROP +drop table store_session_id; +DROP diff --git a/src/test/isolation2/isolation2_schedule b/src/test/isolation2/isolation2_schedule index c68236ca23b55606bebaf466cf951f32729e1f04..05224adf1cdb2bc6c81de5e9233e3fd9a1ecd8bb 100644 --- a/src/test/isolation2/isolation2_schedule +++ b/src/test/isolation2/isolation2_schedule @@ -179,6 +179,8 @@ test: uao/insert_should_not_use_awaiting_drop_column test: add_column_after_vacuum_skip_drop_column test: vacuum_after_vacuum_skip_drop_column +test: segwalrep/die_commit_pending_replication + # Tests for FTS test: fts_errors test: segwalrep/commit_blocking diff --git a/src/test/isolation2/sql/segwalrep/die_commit_pending_replication.sql b/src/test/isolation2/sql/segwalrep/die_commit_pending_replication.sql new file mode 100644 index 0000000000000000000000000000000000000000..f34bde11b8fe89f583d4a62e014bd50980d12fa2 --- /dev/null +++ b/src/test/isolation2/sql/segwalrep/die_commit_pending_replication.sql @@ -0,0 +1,52 @@ +-- Adding `2` as first column as the distribution column. +-- `2` should be on the first segment. let's double check here. +create table store_session_id(a int, sess_id int); +1: insert into store_session_id select 2, sess_id from pg_stat_activity where pid = pg_backend_pid(); +1: select gp_segment_id, a from store_session_id; + +1: create table die_commit_pending_replication(a int, b int); + +-- Suspend to hit commit-prepared point on segment (as we are +-- interested in testing Commit here and not really Prepare) +select gp_inject_fault_infinite('finish_prepared_start_of_function', 'suspend', dbid) from gp_segment_configuration where role='p' and content = 0; +1&: insert into die_commit_pending_replication values(2),(1); +select gp_wait_until_triggered_fault('finish_prepared_start_of_function', 1, dbid) from gp_segment_configuration where role='p' and content = 0; + +-- Now pause the wal sender on primary for content 0 +select gp_inject_fault_infinite('wal_sender_loop', 'suspend', dbid) from gp_segment_configuration where role='p' and content = 0; + +-- Insert fault in the ProcDiePending code block. +select gp_inject_fault_infinite('sync_rep_query_die', 'skip', dbid) from gp_segment_configuration where role='p' and content = 0; + +-- Let the transaction move forward with the commit +select gp_inject_fault('finish_prepared_start_of_function', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + +-- Ensure the ProcDiePending code block is called. It implies 'replication' in pg_stat_activity. +select gp_wait_until_triggered_fault('sync_rep_query_die', 1, dbid) from gp_segment_configuration where role='p' and content = 0; + +-- We can terminate the backend on QE now. +0U: select pg_terminate_backend(pid) from pg_stat_activity + where waiting_reason='replication' and + sess_id in (select sess_id from store_session_id); + +-- We expect two more occurrence: one for backend quitting and another for retry. +select gp_wait_until_triggered_fault('sync_rep_query_die', 3, dbid) from gp_segment_configuration where role='p' and content = 0; + +-- Verify that the sess_id changes due to retry. +0U: select pid,sess_id,waiting_reason,query from pg_stat_activity + where sess_id in (select sess_id from store_session_id); + +-- resume the primary wal replication so that retry could complete. +select gp_inject_fault('wal_sender_loop', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + +-- reset sync_rep_query_die +select gp_inject_fault('sync_rep_query_die', 'reset', dbid) from gp_segment_configuration where role='p' and content = 0; + +1<: + +-- check if the insert fails or not. +select gp_segment_id, * from die_commit_pending_replication; + +-- cleanup +drop table die_commit_pending_replication; +drop table store_session_id;