Re: Physical replication slot advance is not persistent

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, michael(at)paquier(dot)xyz, simon(at)2ndquadrant(dot)com
Subject: Re: Physical replication slot advance is not persistent
Date: 2019-12-29 12:12:16
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 2019-12-26 16:35, Alexey Kondratov wrote:
> Another concern is that ReplicationSlotIsDirty is added with the only
> one user. It also cannot be used by SaveSlotToPath due to the
> simultaneous usage of both flags dirty and just_dirtied there.
> In that way, I hope that we should call ReplicationSlotSave
> unconditionally in the pg_replication_slot_advance, so slot will be
> saved or not automatically based on the slot->dirty flag. In the same
> time, ReplicationSlotsComputeRequiredXmin and
> ReplicationSlotsComputeRequiredLSN should be called by anyone, who
> modifies xmin and LSN fields in the slot. Otherwise, currently we are
> getting some leaky abstractions.

It seems that there was even a race in the order of actions inside
pg_replication_slot_advance, it did following:

- ReplicationSlotMarkDirty();
- ReplicationSlotsComputeRequiredXmin(false);
- ReplicationSlotsComputeRequiredLSN();
- ReplicationSlotSave();

1) Mark slot as dirty, which actually does nothing immediately, but
setting dirty flag;
2) Do compute new global required LSN;
3) Flush slot state to disk.

If someone will utilise old WAL and after that crash will happen between
steps 2) and 3), then we start with old value of restart_lsn, but
without required WAL. I do not know how to properly reproduce it without
gdb and power off, so the chance is pretty low, but still it could be a

Logical slots were not affected again, since there was a proper
operations order (with comments) and slot flushing routines inside

Thus, in the attached patch I have decided to do not perform slot
flushing in the pg_replication_slot_advance at all and do it in the
pg_physical_replication_slot_advance instead, as it is done in the

Since this bugfix have not moved forward during the week, I will put it
on the 01.2020 commitfest. Kyotaro, if you do not object I will add you
as a reviewer, as you have already gave a lot of feedback, thank you for

Alexey Kondratov

Postgres Professional
Russian Postgres Company

Attachment Content-Type Size
v3-0001-Make-physical-slot-advance-to-be-persistent.patch text/x-diff 4.0 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2019-12-29 12:32:31 Re: [PATCH] fix a performance issue with multiple logical-decoding walsenders
Previous Message Tatsuo Ishii 2019-12-29 11:24:04 Re: Incremental View Maintenance: ERROR: out of shared memory