On 2020-06-10 11:38, Kyotaro Horiguchi wrote:
> At Wed, 10 Jun 2020 15:53:53 +0900, Michael Paquier
> <michael@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