References: <20180629225055.cy3e4ecayibwr5rd@alvherre.pgsql>
 <27276.1530313324@sss.pgh.pa.us>
User-agent: mu4e 1.1.0; emacs 26.0.50
From: Arseny Sher <a.sher@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>,
 pgsql-hackers@lists.postgresql.org, 2q-andrew@alvin.alvh.no-ip.org
Subject: Re: pgsql: Fix "base" snapshot handling in logical decoding
In-reply-to: <27276.1530313324@sss.pgh.pa.us>
Date: Sat, 30 Jun 2018 18:00:21 +0300
Message-ID: <87h8lkuxoa.fsf@ars-thinkpad>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Precedence: bulk

--=-=-=
Content-Type: text/plain


Tom Lane <tgl@sss.pgh.pa.us> writes:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>>> On 2018-Jun-28, Tom Lane wrote:
>>>> According to buildfarm member friarbird, and as confirmed here,
>>>> the contrib/test_decoding/specs/oldest_xmin.spec test added by this
>>>> commit fails under CLOBBER_CACHE_ALWAYS.
>
>> I suppose 60 seconds (isolationtester's default timeout) is just not
>> enough time for those machines.  We could increase it to 180 seconds and
>> see if that's enough to make them pass ...
>
> What I want to know is why this test is doing a database-wide VACUUM FULL
> in the first place.  If that isn't profligate wastage of testing cycles,
> why not?
>
> 			regards, tom lane

Oh, that's my fault. I think just VACUUM pg_attribute is enough there --
it takes 42ms on my laptop with CLOBBER_CACHE_ALWAYS ('vacuum full'
occupies 1 minute), patch is attached. The test is still steadily fails
without the main patch.

There is also one thing that puzzles me as I don't know much about
vacuum internals. If I do plain VACUUM of pg_attribute in the test, it
shouts "catalog is missing 1 attribute(s) for relid" error (which is
quite expected), while with 'VACUUM FULL pg_attribute' the tuple is
silently (and wrongly, with dropped column missing) decoded. Moreover,
if I perform the test manually, and do 'VACUUM FULL;', sometimes test
becomes useless -- that is, tuple is successfully decoded with all three
columns, as though VACUUM was not actually executed. All this is without
the main patch, of course. I think I will look into this soon.

--
Arseny Sher
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=less_agressive_vacuum_in_oldest_xmin_test.patch

diff --git a/contrib/test_decoding/expected/oldest_xmin.out b/contrib/test_decoding/expected/oldest_xmin.out
index d09342c4be..a5d7d3c2ca 100644
--- a/contrib/test_decoding/expected/oldest_xmin.out
+++ b/contrib/test_decoding/expected/oldest_xmin.out
@@ -15,7 +15,7 @@ step s0_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slo
 data           
 
 step s1_commit: COMMIT;
-step s0_vacuum: VACUUM FULL;
+step s0_vacuum: VACUUM pg_attribute;
 step s0_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1');
 data           
 
diff --git a/contrib/test_decoding/specs/oldest_xmin.spec b/contrib/test_decoding/specs/oldest_xmin.spec
index 4f8af70aa2..cd2b69cae2 100644
--- a/contrib/test_decoding/specs/oldest_xmin.spec
+++ b/contrib/test_decoding/specs/oldest_xmin.spec
@@ -22,7 +22,7 @@ step "s0_getxid" { SELECT txid_current() IS NULL; }
 step "s0_alter" { ALTER TYPE basket DROP ATTRIBUTE mangos; }
 step "s0_commit" { COMMIT; }
 step "s0_checkpoint" { CHECKPOINT; }
-step "s0_vacuum" { VACUUM FULL; }
+step "s0_vacuum" { VACUUM pg_attribute; }
 step "s0_get_changes" { SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); }
 
 session "s1"

--=-=-=--