Re: pg_xmin_horizon: a system view of everything pinning the xmin horizon

Hi Scott,

I reviewed the v6 and have some comments.

pgstat_get_xact_start_by_proc_number() reads BackendStatusArray directly,
so the value is no longer taken from a cached snapshot that could predate
the procarray pass by an arbitrary amount.
The residual race you document in the header comment is a much smaller
window, and the change-count retry loop and the pid <= 0 guard both look
right. I also checked that p->procNumber = pgprocno is the right
ProcNumber for indexing the status array, and that a prepared xact's dummy
proc can't reach the array out of bounds, the pid <= 0 early return catches
it before the bounds check does.

The maintenance.sgml rewrite is better than what I suggested. Sorting all
rows by age(shared_xmin) DESC NULLS LAST, and explaining that
datfrozenxid can never advance past the shared horizon and that every ow's
shared_xmin feeds it including backends in other databases, avoids the
datid scoping question entirely rather than trying to describe it.
The datfrozenxid clarification is a good catch too.

One question. pg_stat_get_activity() gates st_xact_start_timestamp behind
HAS_PGSTAT_PERMISSIONS, but the new helper applies no such check.
That isn't reachable today, since both the view and the SRF require
pg_read_all_stats, and such a caller already passes that test for every
backend.
But it does mean the SRF omits a check pg_stat_activity applies. Bharath
pointed out one asymmetry already i.e pg_stat_activity shows pid, datid,
xid and xmin to unprivileged non-owners while
pg_xmin_horizon shows nothing, and this would be one in the other direction
if the ACL were ever loosened or EXECUTE granted directly.
Would it be worth applying HAS_PGSTAT_PERMISSIONS per row in the SRF?

Regards,
Surya Poondla