Use sync commit for logical replication apply in TAP tests
Hi,
The commit 887227a1c changed the defaults for subscriptions to do async
commit. But since the tests often wait for disk flush and there is no
concurrent activity this has increased the amount of time needed for
each test. So the attached patch changes the subscriptions create in tab
tests to use sync commit which improves performance there (because we
also replicate only very few transactions in the tests).
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Attachments:
0001-Change-logical-replication-TAP-tests-to-use-sync-com.patchtext/plain; charset=UTF-8; name=0001-Change-logical-replication-TAP-tests-to-use-sync-com.patchDownload
From a4cebdf95d1f8a58d12f2f3824dffaae1dbf435d Mon Sep 17 00:00:00 2001
From: Petr Jelinek <pjmodos@pjmodos.net>
Date: Wed, 19 Apr 2017 03:55:17 +0200
Subject: [PATCH] Change logical replication TAP tests to use sync commit
This speeds up the test run since we are often waiting on flush to disk.
---
src/test/subscription/t/001_rep_changes.pl | 2 +-
src/test/subscription/t/002_types.pl | 2 +-
src/test/subscription/t/003_constraints.pl | 2 +-
src/test/subscription/t/004_sync.pl | 8 ++++----
src/test/subscription/t/005_encoding.pl | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index d1817f5..8f791f1 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -48,7 +48,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub, tap_pub_ins_only");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub, tap_pub_ins_only WITH (synchronous_commit = local)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/002_types.pl b/src/test/subscription/t/002_types.pl
index ad15e85..2f89fdc 100644
--- a/src/test/subscription/t/002_types.pl
+++ b/src/test/subscription/t/002_types.pl
@@ -103,7 +103,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (SLOT NAME = tap_sub_slot)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (SLOT NAME = tap_sub_slot, synchronous_commit = local)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/003_constraints.pl b/src/test/subscription/t/003_constraints.pl
index 11b8254..2b43263 100644
--- a/src/test/subscription/t/003_constraints.pl
+++ b/src/test/subscription/t/003_constraints.pl
@@ -34,7 +34,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA, synchronous_commit = local)");
# Wait for subscriber to finish initialization
my $caughtup_query =
diff --git a/src/test/subscription/t/004_sync.pl b/src/test/subscription/t/004_sync.pl
index 87541a0..57c9e4c 100644
--- a/src/test/subscription/t/004_sync.pl
+++ b/src/test/subscription/t/004_sync.pl
@@ -32,7 +32,7 @@ $node_publisher->safe_psql('postgres',
my $appname = 'tap_sub';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (synchronous_commit = local)");
# Wait for subscriber to finish initialization
my $caughtup_query =
@@ -58,7 +58,7 @@ $node_publisher->safe_psql('postgres',
# recreate the subscription, it will try to do initial copy
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (synchronous_commit = local)");
# but it will be stuck on data copy as it will fail on constraint
my $started_query =
@@ -81,7 +81,7 @@ is($result, qq(20), 'initial data synced for second sub');
# now check another subscription for the same node pair
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA)");
+ "CREATE SUBSCRIPTION tap_sub2 CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (NOCOPY DATA, synchronous_commit = local)");
# wait for it to start
$node_subscriber->poll_query_until('postgres', "SELECT pid IS NOT NULL FROM pg_stat_subscription WHERE subname = 'tap_sub2' AND relid IS NULL")
@@ -102,7 +102,7 @@ $node_subscriber->safe_psql('postgres',
# recreate the subscription again
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub");
+ "CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION tap_pub WITH (synchronous_commit = local)");
# and wait for data sync to finish again
$node_subscriber->poll_query_until('postgres', $synced_query)
diff --git a/src/test/subscription/t/005_encoding.pl b/src/test/subscription/t/005_encoding.pl
index 42a4eee..da39e1e 100644
--- a/src/test/subscription/t/005_encoding.pl
+++ b/src/test/subscription/t/005_encoding.pl
@@ -30,7 +30,7 @@ my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
my $appname = 'encoding_test';
$node_publisher->safe_psql('postgres', "CREATE PUBLICATION mypub FOR ALL TABLES;");
-$node_subscriber->safe_psql('postgres', "CREATE SUBSCRIPTION mysub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION mypub;");
+$node_subscriber->safe_psql('postgres', "CREATE SUBSCRIPTION mysub CONNECTION '$publisher_connstr application_name=$appname' PUBLICATION mypub WITH (synchronous_commit = local);");
wait_for_caught_up($node_publisher, $appname);
--
2.7.4
On 4/18/17 22:25, Petr Jelinek wrote:
The commit 887227a1c changed the defaults for subscriptions to do async
commit. But since the tests often wait for disk flush and there is no
concurrent activity this has increased the amount of time needed for
each test. So the attached patch changes the subscriptions create in tab
tests to use sync commit which improves performance there (because we
also replicate only very few transactions in the tests).
I see only a 1.5% (1s/70s) improvement in the run time of the whole test
suite from this. Is that what you were expecting?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20/04/17 14:57, Peter Eisentraut wrote:
On 4/18/17 22:25, Petr Jelinek wrote:
The commit 887227a1c changed the defaults for subscriptions to do async
commit. But since the tests often wait for disk flush and there is no
concurrent activity this has increased the amount of time needed for
each test. So the attached patch changes the subscriptions create in tab
tests to use sync commit which improves performance there (because we
also replicate only very few transactions in the tests).I see only a 1.5% (1s/70s) improvement in the run time of the whole test
suite from this. Is that what you were expecting?
Hmm, on my machine the difference is around 50% (~1m vs ~2m).
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/20/17 09:46, Petr Jelinek wrote:
On 20/04/17 14:57, Peter Eisentraut wrote:
On 4/18/17 22:25, Petr Jelinek wrote:
The commit 887227a1c changed the defaults for subscriptions to do async
commit. But since the tests often wait for disk flush and there is no
concurrent activity this has increased the amount of time needed for
each test. So the attached patch changes the subscriptions create in tab
tests to use sync commit which improves performance there (because we
also replicate only very few transactions in the tests).I see only a 1.5% (1s/70s) improvement in the run time of the whole test
suite from this. Is that what you were expecting?Hmm, on my machine the difference is around 50% (~1m vs ~2m).
Double hmm, I ran a few more tests and different machines and get
consistent but underwhelming improvements.
I don't mind applying the patch nonetheless, but maybe we can get a few
more test results from others.
(Instructions: apply the patch and time make -C src/test/subscription check)
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
Double hmm, I ran a few more tests and different machines and get
consistent but underwhelming improvements.
I don't mind applying the patch nonetheless, but maybe we can get a few
more test results from others.
(Instructions: apply the patch and time make -C src/test/subscription check)
OK, here's some results. All are typical debug builds (--enable-cassert
in particular), and each number cited is best result of three runs:
UNPATCHED PATCHED
sss1 1m13.828s 1m12.763s (H/W RAID controller + spinning rust)
laptop 1m9.236s 1m7.969s (Macbook Pro, SSD)
gaur 11m25.58s 11m14.04s (1990s-era spinning rust)
prairiedog 8m37.848s 9m26.256s (ATA/66, 5400RPM spinning rust)
(For context, about 2m10s of gaur's cycle time is the temp install
preparation, and 32s of prairiedog's; it's just a few seconds on the
newer machines.)
I have little faith in the numbers from gaur because I saw run-to-run
variations of a couple of minutes; I suspect that the bgworker launching
bug I described yesterday is making itself felt in this test too.
prairiedog also had rather more variance than one would expect, making
me wonder if something similar is happening there.
But based on these results, I would say that as a test-speed enhancement,
this patch is a failure. I'd also question the wisdom of testing only
a non-default code path. There could be an argument for running some
of the subscription tests with async commit and some without, just to
improve code coverage.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers