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+mwsmgju2v686vu...@mail.gmail.com -- Sami Imseih Amazon Web Services (AWS)
