From: Alexey Kondratov <a.kondratov@postgrespro.ru>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: michael@paquier.xyz
Cc: andres@anarazel.de
Cc: dim@tapoueh.org
Cc: pgsql-hackers@postgresql.org
Cc: simon@2ndquadrant.com
Subject: Re: Physical replication slot advance is not persistent
Date: Wed, 10 Jun 2020 20:57:17 +0300
Message-ID: <876a782d320a52b0c54d6d48372de17d@postgrespro.ru> (raw)
In-Reply-To: <20200610.173844.78906193765823414.horikyota.ntt@gmail.com>
References: <20200609171904.kpltxxvjzislidks@alap3.anarazel.de>
<82163ef4a380c5255e5004659d02559e@postgrespro.ru>
<20200610065353.GA358163@paquier.xyz>
<20200610.173844.78906193765823414.horikyota.ntt@gmail.com>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: a.kondratov@postgrespro.ru, horikyota.ntt@gmail.com, michael@paquier.xyz, andres@anarazel.de, dim@tapoueh.org, simon@2ndquadrant.com
Subject: Re: Physical replication slot advance is not persistent
In-Reply-To: <876a782d320a52b0c54d6d48372de17d@postgrespro.ru>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox