Обсуждение: Decoding of (nearly) empty transactions and regression tests
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
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
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
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
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
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
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