public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Dilip Kumar <[email protected]>
Cc: [email protected] <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Date: Mon, 26 Dec 2022 17:21:05 +0530
Message-ID: <CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com> (raw)
In-Reply-To: <OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
References: <CAA4eK1KdVRffYEYiv9=4tVWhP=Jkfh1h6FM0A1R0NEe8cLT8+g@mail.gmail.com>
<CAA4eK1JU4J3-Z=fcMcyQw-iG-DNr=u6w_g-iM_4xwg1pKupkFQ@mail.gmail.com>
<OS0PR01MB57163693482C5470C41914AF94EA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAHut+PuYJz-26QWMSRN_HwiJNfYG4ZxEV3RGE-VyZtXXeFm+cw@mail.gmail.com>
<OS0PR01MB5716C1C1D8244BA86311193894EB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CAA4eK1LrADDNXKRVYox4Q+zSFQvtiQu-7piyd_PCmVjg1gsN2w@mail.gmail.com>
<OS0PR01MB57169EF567017861EACBD10D94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS0PR01MB571623EFD8903C93424EFADC94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<OS0PR01MB57166C4E79559C9A5570838C94EC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
On Mon, Dec 26, 2022 at 9:52 AM [email protected]
<[email protected]> wrote:
>
> On Friday, December 23, 2022 5:20 PM [email protected] <[email protected]> wrote:
>
> Since the GUC used to force stream changes has been committed, I removed that
> patch from the patch set here and rebased the testcases based on that commit.
> Here is the rebased patch set.
>
Few comments on 0002 and 0001 patches
=================================
1.
+ if ($is_parallel)
+ {
+ $node_subscriber->append_conf('postgresql.conf',
+ "log_min_messages = debug1");
+ $node_subscriber->reload;
+ }
+
+ # Check the subscriber log from now on.
+ $offset = -s $node_subscriber->logfile;
+
+ $in .= q{
+ BEGIN;
+ INSERT INTO test_tab SELECT i, md5(i::text) FROM
generate_series(3, 5000) s(i);
How can we guarantee that reload would have taken place before this
next test? I see that 020_archive_status.pl is executing a query to
ensure the reload has been taken into consideration. Can we do the
same?
2. It is not very clear whether converting 017_stream_ddl and
019_stream_subxact_ddl_abort adds much value. They seem to be mostly
testing DDL/DML interaction of publisher side. We can probably check
the code coverage by removing the parallel version for these two files
and remove them unless it covers some extra code. If we decide to
remove parallel version for these two files then we can probably add a
comment atop these files indicating why we don't have a version that
parallel option for these tests.
3.
+# Drop the unique index on the subscriber, now it works.
+$node_subscriber->safe_psql('postgres', "DROP INDEX idx_tab");
+
+# Wait for this streaming transaction to be applied in the apply worker.
$node_publisher->wait_for_catchup($appname);
$result =
- $node_subscriber->safe_psql('postgres',
- "SELECT count(*), count(c), count(d = 999) FROM test_tab");
-is($result, qq(3334|3334|3334), 'check extra columns contain local defaults');
+ $node_subscriber->safe_psql('postgres', "SELECT count(*) FROM test_tab_2");
+is($result, qq(5001), 'data replicated to subscriber after dropping index');
-# Test the streaming in binary mode
+# Clean up test data from the environment.
+$node_publisher->safe_psql('postgres', "TRUNCATE TABLE test_tab_2");
+$node_publisher->wait_for_catchup($appname);
$node_subscriber->safe_psql('postgres',
- "ALTER SUBSCRIPTION tap_sub SET (binary = on)");
+ "CREATE UNIQUE INDEX idx_tab on test_tab_2(a)");
What is the need to first Drop the index and then recreate it after a few lines?
4. Attached, find some comment improvements atop v67-0002* patch.
Similar comments need to be changed in other test files.
5. Attached, find some comment improvements atop v67-0001* patch.
--
With Regards,
Amit Kapila.
Attachments:
[application/octet-stream] v67-0002-changes_amit_1.patch (1.9K, ../CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com/2-v67-0002-changes_amit_1.patch)
download | inline diff:
diff --git a/src/test/subscription/t/015_stream.pl b/src/test/subscription/t/015_stream.pl
index 7bc342d211..220ab0c25f 100644
--- a/src/test/subscription/t/015_stream.pl
+++ b/src/test/subscription/t/015_stream.pl
@@ -8,8 +8,8 @@ use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
-# Check the log that the streamed transaction was completed successfully
-# reported by parallel apply worker.
+# Check that the parallel apply worker has finished applying the streaming
+# transaction.
sub check_parallel_log
{
my ($node_subscriber, $offset, $is_parallel, $type) = @_;
@@ -22,9 +22,7 @@ sub check_parallel_log
}
}
-# Encapsulate all the common test steps which are related to "streaming"
-# parameter so the same code can be run both for the streaming=on and
-# streaming=parallel cases.
+# Common test steps for both the streaming=on and streaming=parallel cases.
sub test_streaming
{
my ($node_publisher, $node_subscriber, $appname, $is_parallel) = @_;
@@ -40,10 +38,8 @@ sub test_streaming
my $h = $node_publisher->background_psql('postgres', \$in, \$out, $timer,
on_error_stop => 0);
- # If "streaming" parameter is specified as "parallel", we need to check
- # that streamed transaction was applied using a parallel apply worker.
- # We have to look for the DEBUG1 log messages about that, so bump up the
- # log verbosity.
+ # We need to check DEBUG logs to ensure that the parallel apply worker has
+ # applied the transaction. So, bump up the log verbosity.
if ($is_parallel)
{
$node_subscriber->append_conf('postgresql.conf',
@@ -222,6 +218,8 @@ $node_publisher->poll_query_until('postgres',
test_streaming($node_publisher, $node_subscriber, $appname, 1);
+# Test that the deadlock is detected among leader and parallel apply workers.
+
$node_subscriber->append_conf('postgresql.conf', "log_min_messages = debug1");
$node_subscriber->append_conf('postgresql.conf', "deadlock_timeout = 1ms");
$node_subscriber->reload;
[application/octet-stream] v67-0001-changes_amit_1.patch (962B, ../CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com/3-v67-0001-changes_amit_1.patch)
download | inline diff:
diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c
index f8a0bb462a..2071fca01b 100644
--- a/src/backend/replication/logical/applyparallelworker.c
+++ b/src/backend/replication/logical/applyparallelworker.c
@@ -486,10 +486,10 @@ pa_allocate_worker(TransactionId xid)
}
/*
- * It's necessary to reread the subscription information before assigning
- * the transaction to a parallel apply worker. Otherwise, the leader may
- * not be able to reread the subscription information if streaming
- * transactions keep coming and are handled by parallel apply workers.
+ * It is good to check for any change in the subscription parameter to
+ * avoid the case where for a very long time the change doesn't get
+ * reflected. This can happen when there is a constant flow of streaming
+ * transactions that are handled by parallel apply workers.
*/
maybe_reread_subscription();
view thread (625+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
In-Reply-To: <CAA4eK1+NV9BDPjh5XB9PnZYNdnQht8+2JGRcerJpSUXtYPtQsg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox