On Tue, Jan 28, 2020 at 06:06:06PM +0300, Alexey Kondratov wrote: > On 28.01.2020 15:14, Kyotaro Horiguchi wrote: >> 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.
Ok, good. So I am seeing no objections on that part :D >> 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. Honestly, I was a bit on the fence for the term "dirty" when typing this paragraph, so I kind of agree with Horiguchi-san's point that it could be confusing when applied to replication slots, because there is no other reference in the docs about the link between the two concepts. So, I would go for a more simplified sentence for the first part, keeping the second sentence intact: "The information of the updated slot is 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." -- Michael
signature.asc
Description: PGP signature
