Decoding of (nearly) empty transactions and regression tests

Started by Andres Freundalmost 12 years ago7 messageshackers
Jump to latest
#1Andres Freund
andres@anarazel.de

Hi,

As shown by the CLOBBER_CACHE_ALWAYS animal test_decoding's tests fail
if they take very long. The failures aren't bugs, but diffs like:
BEGIN
+ COMMIT
+ BEGIN
table public.tr_sub: INSERT: id[integer]:1 path[text]:'1-top-#1'
The added transaction is an analyze started by autovacuum.

So, I've been wondering for a while to get rid of this, but I haven't
come up with something I like.

To recap, currently the rules for visibly decoding a transaction
(i.e. calling the output plugin callbacks) are:
1) it has been WAL logged (duh)
2) it happened in the database we're decoding
3) it contains something. 'Something' currently means that a logged
table has changed, or the transaction contains invalidations.

Note that just because a transaction contains 'something', these changes
aren't necessarily shown as we don't decode system table changes. I
think that behaviour makes sense because otherwise something like CREATE
TABLE without further changes wouldn't show up in the change stream.

Solution I)
Change ReorderBufferCommit() to not call the begin()/commit() callbacks
if no change() callback has been called. Technically that's trivial, but
I don't think that'd end up being a better behaviour.

Solution II)
Disable autovacuum/analyze for the tables in the regression tests. We
test vacuum explicitly, so we wouldn't loose too much test coverage.

Solution III)
Mark transactions that happen purely internally as such, using a
xl_xact_commit->xinfo flag. Not particularly pretty and the most
invasive of the solutions.

I'm favoring II) so far... Other opinions?

Greetings,

Andres Freund

--
Andres Freund 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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#1)
Re: Decoding of (nearly) empty transactions and regression tests

Andres Freund <andres@2ndquadrant.com> writes:

Solution I)
Change ReorderBufferCommit() to not call the begin()/commit() callbacks
if no change() callback has been called. Technically that's trivial, but
I don't think that'd end up being a better behaviour.

Solution II)
Disable autovacuum/analyze for the tables in the regression tests. We
test vacuum explicitly, so we wouldn't loose too much test coverage.

Solution III)
Mark transactions that happen purely internally as such, using a
xl_xact_commit->xinfo flag. Not particularly pretty and the most
invasive of the solutions.

I'm favoring II) so far... Other opinions?

You mean disable autovac only in the contrib/test_decoding check run,
right? That's probably OK since it's tested in the main regression
runs.

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

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: Decoding of (nearly) empty transactions and regression tests

On 2014-06-29 10:36:22 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Solution I)
Change ReorderBufferCommit() to not call the begin()/commit() callbacks
if no change() callback has been called. Technically that's trivial, but
I don't think that'd end up being a better behaviour.

Solution II)
Disable autovacuum/analyze for the tables in the regression tests. We
test vacuum explicitly, so we wouldn't loose too much test coverage.

Solution III)
Mark transactions that happen purely internally as such, using a
xl_xact_commit->xinfo flag. Not particularly pretty and the most
invasive of the solutions.

I'm favoring II) so far... Other opinions?

You mean disable autovac only in the contrib/test_decoding check run,
right? That's probably OK since it's tested in the main regression
runs.

Actually I'd not even though of that, but just of disabling it on
relations with relevant amounts of changes in the test_decoding
tests. That way it'd work even when run against an existing server (via
installcheck-force) which I found useful during development.

Greetings,

Andres Freund

--
Andres Freund 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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#3)
Re: Decoding of (nearly) empty transactions and regression tests

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-06-29 10:36:22 -0400, Tom Lane wrote:

You mean disable autovac only in the contrib/test_decoding check run,
right? That's probably OK since it's tested in the main regression
runs.

Actually I'd not even though of that, but just of disabling it on
relations with relevant amounts of changes in the test_decoding
tests. That way it'd work even when run against an existing server (via
installcheck-force) which I found useful during development.

I think that a change that affects the behavior in any other test runs is
not acceptable. Our testing of autovac is weaker than I'd like already
(since the main regression runs are designed to not show visible changes
when it runs). I don't want it being turned off elsewhere for the benefit
of this test.

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

#5Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#4)
Re: Decoding of (nearly) empty transactions and regression tests

On 2014-06-29 11:08:39 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-06-29 10:36:22 -0400, Tom Lane wrote:

You mean disable autovac only in the contrib/test_decoding check run,
right? That's probably OK since it's tested in the main regression
runs.

Actually I'd not even though of that, but just of disabling it on
relations with relevant amounts of changes in the test_decoding
tests. That way it'd work even when run against an existing server (via
installcheck-force) which I found useful during development.

I think that a change that affects the behavior in any other test runs is
not acceptable. Our testing of autovac is weaker than I'd like already
(since the main regression runs are designed to not show visible changes
when it runs). I don't want it being turned off elsewhere for the benefit
of this test.

Hm? I think we're misunderstanding each other - I was thinking of tacking
ALTER TABLE .. SET (AUTOVACUUM_ENABLED = false) to the tables created in
test_decoding/sql/, not to something outside.

Since we ignore transactions in other databases and the tests run in a
database freshly created by pg_regress that should get rid of spurious
transactions. Unless somebody fiddled with the template database or is
close to a wraparound - but I'd be pretty surprised if that wouldn't
cause failures in other tests as wel.

Greetings,

Andres Freund

--
Andres Freund 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

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#5)
Re: Decoding of (nearly) empty transactions and regression tests

Andres Freund <andres@2ndquadrant.com> writes:

Hm? I think we're misunderstanding each other - I was thinking of tacking
ALTER TABLE .. SET (AUTOVACUUM_ENABLED = false) to the tables created in
test_decoding/sql/, not to something outside.

Ah, got it. Yeah, seems like an OK workaround.

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

#7Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#6)
Re: Decoding of (nearly) empty transactions and regression tests

On 2014-06-29 11:24:35 -0400, Tom Lane wrote:

Andres Freund <andres@2ndquadrant.com> writes:

Hm? I think we're misunderstanding each other - I was thinking of tacking
ALTER TABLE .. SET (AUTOVACUUM_ENABLED = false) to the tables created in
test_decoding/sql/, not to something outside.

Ah, got it. Yeah, seems like an OK workaround.

I'd tried that back then, but it didn't really work out very
well. Sometimes there's still autovacuums running on tables we don't
want to disable autovacuum on.

My new proposal (patch attached) is a skip-empty-xacts option for
test_decoding. It causes test_decoding not to output BEGIN/COMMIT for
transactions that don't output any rows.

I don't really like that approach, but it's surely better than
continually coloring the CLOBBER animal red. And the patch is, besides
the verbiage of regression test output changes, not that bad.

Unless somebody protests I'll apply that to master/9.4 later today.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Add-skip-empty-xacts-option-to-test_decoding-for-use.patchtext/x-patch; charset=us-asciiDownload+130-244