Re: Physical replication slot advance is not persistent

From: Andres Freund <andres(at)anarazel(dot)de>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Dimitri Fontaine <dim(at)tapoueh(dot)org>
Cc: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, simon(at)2ndquadrant(dot)com
Subject: Re: Physical replication slot advance is not persistent
Date: 2020-06-09 17:19:04
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2020-01-28 17:01:14 +0900, Michael Paquier wrote:
> So attached is an updated patch which addresses the problem just by
> marking a physical slot as dirty if any advancing is done. Some
> documentation is added about the fact that an advance is persistent
> only at the follow-up checkpoint. And the tests are fixed to not use
> a fake LSN but instead advance to the latest LSN position produced.

> - /* Update the on disk state when lsn was updated. */
> - if (XLogRecPtrIsInvalid(endlsn))
> - {
> - ReplicationSlotMarkDirty();
> - ReplicationSlotsComputeRequiredXmin(false);
> - ReplicationSlotsComputeRequiredLSN();
> - ReplicationSlotSave();
> - }
> -

I am quite confused by the wholesale removal of these lines. That wasn't
in previous versions of the patch. As far as I can tell not calling
ReplicationSlotsComputeRequiredLSN() for the physical slot leads to the
global minimum LSN never beeing advanced, and thus WAL reserved by the
slot not being removable. Only if there's some independent call to
ReplicationSlotsComputeRequiredLSN() XLogSetReplicationSlotMinimumLSN()
will be called, allowing for slots to advance.

I realize this stuff has been broken since the introduction in
9c7d06d6068 (due to the above being if (XLogRecPtrIsInvalid()) rather
than if (!XLogRecPtrIsInvalid()) , but this seems to make it even worse?

I find it really depressing how much obviously untested stuff gets
added in this area.


Andres Freund

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2020-06-09 17:20:25 Re: Bump default wal_level to logical
Previous Message Jelte Fennema 2020-06-09 16:56:16 Re: Verifying embedded oids in *recv is a bad idea