From 751be8216317f1d996c7b4f9f0e915adff805567 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 20 Jan 2022 17:03:27 +0100 Subject: [PATCH] fixup! Add ALTER SUBSCRIPTION ... SKIP to skip the transaction on subscriber nodes --- doc/src/sgml/logical-replication.sgml | 12 ++++-------- doc/src/sgml/ref/alter_subscription.sgml | 16 ++++++++-------- src/backend/commands/subscriptioncmds.c | 2 +- src/backend/replication/logical/worker.c | 7 ------- src/test/regress/expected/subscription.out | 6 +++--- src/test/subscription/t/028_skip_xact.pl | 8 +++++--- 6 files changed, 21 insertions(+), 30 deletions(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index de4f83bbcc..873530db99 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -355,11 +355,10 @@ Conflicts The resolution can be done either by changing data or permissions on the subscriber so that it does not conflict with the incoming changes or by skipping the the transaction that conflicts with the existing data. When a conflict - produces an error, it is shown in + produces an error, it is shown in the pg_stat_subscription_workers view as follows: - - + postgres=# SELECT * FROM pg_stat_subscription_workers; -[ RECORD 1 ]------+----------------------------------------------------------- subid | 16391 @@ -373,9 +372,7 @@ Conflicts last_error_time | 2021-09-29 15:52:45.165754+00 - and it is also shown in subscriber's server log as follows: - ERROR: duplicate key value violates unique constraint "test_pkey" @@ -383,7 +380,6 @@ Conflicts CONTEXT: processing remote data during "INSERT" for replication target relation "public.test" in transaction 716 at 2021-09-29 15:52:45.165754+00 - The transaction ID that contains the change violating the constraint can be found from those outputs (transaction ID 716 in the above case). The transaction can be skipped by using ALTER SUBSCRIPTION ... SKIP on the @@ -401,9 +397,9 @@ Conflicts that it doesn't conflict with incoming changes, or dropping the conflicting constraint or unique index, or writing a trigger on the subscriber to suppress or redirect conflicting incoming changes, or as a last resort, by skipping the whole transaction. - Skipping the whole transaction includes skipping changes that may not violate + Skipping the whole transaction includes skipping changes that might not violate any constraint. This can easily make the subscriber inconsistent, especially if - a user specifies the wrong transaction ID or the position of origin. + a user specifies the wrong transaction ID or the wrong position of origin. diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml index 8b4568ddab..7e0eb55653 100644 --- a/doc/src/sgml/ref/alter_subscription.sgml +++ b/doc/src/sgml/ref/alter_subscription.sgml @@ -212,16 +212,16 @@ Parameters SKIP ( skip_option = value [, ... ] ) - Skips applying all changes of the specified transaction. If incoming data - violates any constraints, the logical replication will stop until it is + Skips applying all changes of the specified remote transaction. If incoming data + violates any constraints, logical replication will stop until it is resolved. The resolution can be done either by changing data on the subscriber so that it doesn't conflict with incoming changes or by skipping - the whole transaction. Using ALTER SUBSCRIPTION ... SKIP + the whole transaction. Using the ALTER SUBSCRIPTION ... SKIP command, the logical replication worker skips all data modification changes - within the specified transaction including changes that may not violate + within the specified transaction, including changes that might not violate the constraint, so, it should only be used as a last resort. This option has no effect on the transactions that are already prepared by enabling - two_phase on subscriber. After the logical replication + two_phase on subscriber. After logical replication successfully skips the transaction, the transaction ID (stored in pg_subscription.subskipxid) is cleared. See for @@ -237,9 +237,9 @@ Parameters xid (xid) - Specifies the ID of the transaction whose changes are to be skipped - by the logical replication worker. We don't support skipping - individual subtransactions. Setting NONE + Specifies the ID of the remote transaction whose changes are to be skipped + by the logical replication worker. Skipping + individual subtransactions is not supported. Setting NONE resets the transaction ID. diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 0ff0e00f19..b8fb7130a6 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -273,7 +273,7 @@ parse_subscription_options(ParseState *pstate, List *stmt_options, if (!TransactionIdIsNormal(xid)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("invalid transaction id: %s", xid_str))); + errmsg("invalid transaction ID: %s", xid_str))); } opts->specified_opts |= SUBOPT_XID; diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0264e30112..0b6d9a203a 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -814,9 +814,6 @@ apply_handle_begin(StringInfo s) remote_final_lsn = begin_data.final_lsn; - /* - * Enable skipping all changes of this transaction if specified. - */ maybe_start_skipping_changes(begin_data.xid); in_remote_transaction = true; @@ -871,9 +868,6 @@ apply_handle_begin_prepare(StringInfo s) remote_final_lsn = begin_data.prepare_lsn; - /* - * Enable skipping all changes of this transaction if specified - */ maybe_start_skipping_changes(begin_data.xid); in_remote_transaction = true; @@ -1429,7 +1423,6 @@ apply_spooled_messages(TransactionId xid, XLogRecPtr lsn) remote_final_lsn = lsn; - /* Enable skipping all changes of this transaction if specified */ maybe_start_skipping_changes(xid); /* diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index 892b6739bc..82733eed98 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -111,11 +111,11 @@ SELECT subname, subskipxid FROM pg_subscription WHERE subname = 'regress_testsub -- fail ALTER SUBSCRIPTION regress_testsub SKIP (xid = 0); -ERROR: invalid transaction id: 0 +ERROR: invalid transaction ID: 0 ALTER SUBSCRIPTION regress_testsub SKIP (xid = 1); -ERROR: invalid transaction id: 1 +ERROR: invalid transaction ID: 1 ALTER SUBSCRIPTION regress_testsub SKIP (xid = 2); -ERROR: invalid transaction id: 2 +ERROR: invalid transaction ID: 2 -- fail - must be superuser SET SESSION AUTHORIZATION 'regress_subscription_user2'; ALTER SUBSCRIPTION regress_testsub SKIP (xid = 100); diff --git a/src/test/subscription/t/028_skip_xact.pl b/src/test/subscription/t/028_skip_xact.pl index 4c107fc8f5..efe28c71af 100644 --- a/src/test/subscription/t/028_skip_xact.pl +++ b/src/test/subscription/t/028_skip_xact.pl @@ -8,8 +8,8 @@ use Test::More tests => 7; # Test skipping the transaction. This function must be called after the caller -# inserting data that conflict with the subscriber. After waiting for the -# subscription worker stats are updated, we skip the transaction in question +# has inserted data that conflicts with the subscriber. After waiting for the +# subscription worker stats to be updated, we skip the transaction in question # by ALTER SUBSCRIPTION ... SKIP. Then, check if logical replication can continue # working by inserting $nonconflict_data on the publisher. sub test_skip_xact @@ -17,6 +17,8 @@ sub test_skip_xact my ($node_publisher, $node_subscriber, $subname, $relname, $nonconflict_data, $expected, $xid, $msg) = @_; + local $Test::Builder::Level = $Test::Builder::Level + 1; + # Wait for worker error $node_subscriber->poll_query_until( 'postgres', @@ -83,7 +85,7 @@ sub test_skip_xact ]); $node_subscriber->start; -# Initial table setup on both publisher and subscriber. On subscriber we +# Initial table setup on both publisher and subscriber. On the subscriber, we # create the same tables but with primary keys. Also, insert some data that # will conflict with the data replicated from publisher later. $node_publisher->safe_psql( -- 2.34.1