On Thu, Sep 7, 2023 at 4:11 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Sep 7, 2023 at 3:38 PM Ashutosh Bapat > <ashutosh.bapat....@gmail.com> wrote: > > > > * This needn't actually be part of a checkpoint, but it's a convenient > > - * location. > > + * location. is_shutdown is true in case of a shutdown checkpoint. > > > > Relying on the first sentence, if we decide to not persist the > > replication slot at the time of checkpoint, would that be OK? It > > doesn't look like a convenience thing to me any more. > > > > Instead of removing that comment, how about something like this: "This > needn't actually be part of a checkpoint except for shutdown > checkpoint, but it's a convenient location."? >
I find the wording a bit awkward. My version would be "Checkpoint is a convenient location to persist all the slots. But in a shutdown checkpoint, indicated by is_shutdown = true, we also update confirmed_flush." But please feel free to choose whichever version you are comfortable with. -- Best Wishes, Ashutosh Bapat