Re: Physical replication slot advance is not persistent

On 25.12.2019 16:51, Alexey Kondratov wrote:
> On 25.12.2019 07:03, Kyotaro Horiguchi wrote:
>> As the result I think what is needed there is just checking if the
>> returned lsn is equal or larger than moveto. Doen't the following
>> change work?
>>
>> -    if (XLogRecPtrIsInvalid(endlsn))
>> +    if (moveto <= endlsn)
>
> Yep, it helps with physical replication slot persistence after 
> advance, but the whole validation (moveto <= endlsn) does not make 
> sense for me. The value of moveto should be >= than minlsn == 
> confirmed_flush / restart_lsn, while endlsn == retlsn is also always 
> initialized with confirmed_flush / restart_lsn. Thus, your condition 
> seems to be true in any case, even if it was no-op one, which we were 
> intended to catch.
>
> I will recheck everything again and try to come up with something 
> during this week.

If I get it correctly, then we already keep previous slot position in 
the minlsn, so we just have to compare endlsn with minlsn and treat 
endlsn <= minlsn as a no-op without slot state flushing.

Attached is a patch that does this, so it fixes the bug without 
affecting any user-facing behavior. Detailed comment section and DEBUG 
output are also added. What do you think now?

I have also forgotten to mention that all versions down to 11.0 should 
be affected with this bug.


Regards

-- 
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company