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.

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.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment: another_test.patch
Description: another_test.patch

Reply via email to