Dear Amit,
> OTOH, if we don't want to adjust physical
> slot machinery, it seems saving the logical slots to disk immediately
> when its restart_lsn is updated is a waste of effort after your patch,
> no? If so, why are we okay with that?
I agree, that saving logical slots at advance is a possible waste of effort. But
I don't understand original ideas behind it. I haven't touched it to make
the minimal patch which should not break the existing functionality.
We trim WAL in checkpoint (or restart point) operation only. The slots'
restart_lsn is used to keep the wal from truncation. I believe, we need to
compute the slots' oldest lsn as the minimal value of restart_lsn values only
when executing checkpoint (or restart point). I guess, it doesn't depend on
slot's type (logical or physical). We have 0003 patch to fix it.
I haven't deeply investigated yet slot's xmin values but I guess the xmin values
are a different story than restart_lsn. It is used to avoid tuple deletions by
vacuum and it is updated by a different way. I can't say that
LogicalConfirmReceivedLocation is the right place to update saved on disk xmin
values. I would propose to update these values in SaveSlotToPath under some lock
to avoid concurrent reads of unsaved values or do in a checkpoint like as for
restart_lsn. We may investigate and improve it in an another patch.
With best regards,
Vitaly
