Re: [HACKERS] Use sync commit for logical replication apply in TAP tests

[Tom Lane]

Peter Eisentraut  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
sss11m13.828s   1m12.763s  (H/W RAID controller + spinning rust)
laptop  1m9.236s1m7.969s   (Macbook Pro, SSD)
gaur11m25.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

Re: [HACKERS] Use sync commit for logical replication apply in TAP tests

[Peter Eisentraut]

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

Re: [HACKERS] Use sync commit for logical replication apply in TAP tests

[Petr Jelinek]

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

Re: [HACKERS] Use sync commit for logical replication apply in TAP tests

[Peter Eisentraut]

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