On Wed, 23 Aug 2023 at 14:21, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh,
>
> > Here is a patch to persist to disk logical slots during a shutdown
> > checkpoint if the updated confirmed_flush_lsn has not yet been
> > persisted.
>
> Thanks for making the patch with different approach! Here are comments.
>
> 01. RestoreSlotFromDisk
>
> ```
> slot->candidate_xmin_lsn = InvalidXLogRecPtr;
> slot->candidate_restart_lsn = InvalidXLogRecPtr;
> slot->candidate_restart_valid = InvalidXLogRecPtr;
> + slot->last_persisted_confirmed_flush = InvalidXLogRecPtr;
> ```
>
> last_persisted_confirmed_flush was set to InvalidXLogRecPtr, but isn't it better
> to use cp.slotdata. confirmed_flush? Assuming that the server is shut down immediately,
> your patch forces to save.
Modified
> 02. t/002_always_persist.pl
>
> The original author of the patch is me, but I found that the test could pass
> without your patch. This is because pg_logical_slot_get_changes()->
> pg_logical_slot_get_changes_guts(confirm = true) always mark the slot as dirty.
> IIUC we must use the logical replication system to verify the persistence.
> Attached test can pass only when patch is applied.
Update the test based on your another_test with slight modifications.
Attached v4 version patch has the changes for the same.
Regards,
Vignesh