On 2020-06-10 11:38, Kyotaro Horiguchi wrote:
At Wed, 10 Jun 2020 15:53:53 +0900, Michael Paquier
<mich...@paquier.xyz> wrote in
> > I find it really depressing how much obviously untested stuff gets
> > added in this area.
>
> Prior to this patch pg_replication_slot_advance was not being tested
> at all.
> Unfortunately, added tests appeared to be not enough to cover all
> cases. It
> seems that the whole machinery of WAL holding and trimming is worth
> to be
> tested more thoroughly.
I think that it would be interesting if we had a SQL representation of
the contents of XLogCtlData (wanted that a couple of times). Now we
are actually limited to use a checkpoint and check that past segments
are getting recycled by looking at the contents of pg_wal. Doing that
here does not cause the existing tests to be much more expensive as we
only need one extra call to pg_switch_wal(), mostly. Please see the
attached.
The test in the patch looks fine to me and worked well for me.
Using smaller wal_segment_size (1(MB) worked for me) reduces the cost
of the check, but I'm not sure it's worth doing.
New test reproduces this issue well. Left it running for a couple of
hours in repeat and it seems to be stable.
Just noted that we do not need to keep $phys_restart_lsn_pre:
my $phys_restart_lsn_pre = $node_master->safe_psql('postgres',
"SELECT restart_lsn from pg_replication_slots WHERE slot_name =
'$phys_slot';"
);
chomp($phys_restart_lsn_pre);
we can safely use $current_lsn used for pg_replication_slot_advance(),
since reatart_lsn is set as is there. It may make the test a bit simpler
as well.
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
