Hi,
> The documentation describes xmin as the oldest
> transaction that the slot needs the database to retain, so the view
> doesn't match the documentation in this case.
That's right, this is a gap. The pg_replication_slots.xmin column should also
show the in-memory xmin used while initializing logical decoding with a full
snapshot; otherwise it is not obvious from the slot view that this slot is one
of the things that can be holding the VACUUM cutoff back.
> + else if (slot_contents.effective_xmin != InvalidTransactionId)
> + values[i++] = TransactionIdGetDatum(...);
From what I can tell, we should only enter this fallback for normal logical
decoding slots while initializing a full snapshot. In the other cases,
data.xmin and
effective_xmin are set together.
The patch overall LGTM, and closes the documentation gap.
> I checked that the first case reproduces on all supported branches, so
> I think we can backpatch the fix. Please find the attached v1 patch.
>
> Thoughts?
I think backpatching is good here.
I was debating whether we should add a test for this, but the test would need
concurrent sessions, so it would likely have to be a Perl TAP test. I am not
sure that is worth it for this case.
> So in both phases of REPACK (CONCURRENTLY) a visible backend holds the
> same xmin as the slot.
Right, there can be multiple holders for the same cutoff. The
pg_xmin_horizon reporting work being done here [2] will be the
appropriate utility to identify all contributors holding the cutoff xmin back.
[2] https://postgr.es/m/CALj2ACV9FYmK9nHZ+DtNkOvN_z8uMvWF+MwsMgJu2V686vuYHg@mail.gmail.com
--
Sami Imseih
Amazon Web Services (AWS)
