On 28.01.2020 15:14, Kyotaro Horiguchi wrote:
At Tue, 28 Jan 2020 17:45:47 +0800, Craig Ringer <cr...@2ndquadrant.com> wrote
in
On Tue, 28 Jan 2020 at 16:01, Michael Paquier <mich...@paquier.xyz> 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.
Any objections?
LGTM. Thankyou.
I agree not to save slots immediately. The code is wrtten as described
above. The TAP test is correct.
+1, removing this broken saving code path from
pg_replication_slot_advance and marking slot as dirty looks good to me.
It solves the issue and does not add any unnecessary complexity.
But the doc part looks a bit too detailed to me. Couldn't we explain
that without the word 'dirty'?
- and it will not be moved beyond the current insert location. Returns
- name of the slot and real position to which it was advanced to.
+ and it will not be moved beyond the current insert location. Returns
+ name of the slot and real position to which it was advanced to. The
+ updated slot is marked as dirty if any advancing is done, with its
+ information being written out at the follow-up checkpoint. In the
+ event of a crash, the slot may return to an earlier position.
and it will not be moved beyond the current insert location. Returns
name of the slot and real position to which it was advanced to. The
information of the updated slot is scheduled to be written out at the
follow-up checkpoint if any advancing is done. In the event of a
crash, the slot may return to an earlier position.
Just searched through the *.sgml files, we already use terms 'dirty' and
'flush' applied to writing out pages during checkpoints. Here we are
trying to describe the very similar process, but in relation to
replication slots, so it looks fine for me. In the same time, the term
'schedule' is used for VACUUM, constraint check or checkpoint itself.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
Russian Postgres Company
