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

From: Scott Ray <scott(at)scottray(dot)io>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, surya poondla <suryapoondla4(at)gmail(dot)com>
Subject: Re: pg_xmin_horizon: a system view of everything pinning the xmin horizon
Date: 2026-09-05 03:25:05
Message-ID: 3TmV7r48RekS9lI-goS5ZDrVOfRrrNwFY1nnghf05L3Ays6_km2Qu0LWqCg_-LhUJrUbKjhpm9VI6pjB7aZTpIMEe_9kAkpL_j4Dj08KazY=@scottray.io
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Bharath,

Thanks for the comments.

On Monday, August 31st, 2026 at 8:15 PM, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:

> I think having pg_xmin_horizon report similar information on
> the standby, say, the long-running transaction or long-running
> analytical query or replication slot, for example, could be a valuable
> addition and will be useful in many situations.

I agree, and in my first message in this thread, I suggested adding
standby support in a follow-up patch.

> There seems to be some degree of information
> bloat being added by this patch. I mean, most of the columns are
> already derivable from elsewhere today. We must try to address this
> point. This may be okay given we compute things in one pass of
> proc-array/replication slots. Let's see what others have to say about
> this.

Please explain specifically where you see information bloat.

> 2/ pg_xmin_horizon needs to have similar checks as
> HAS_PGSTAT_PERMISSIONS for disallowing roles that are not owners of
> the sessions (unprivileged?) from unnecessarily scanning the
> proc-array. I haven't spent enough time checking whether this can
> happen with pg_read_all_stats predefined roles, but worth verifying.

The view requires pg_read_all_stats or equivalent permissions, as I
noted in my first email, the commit message, and the view documentation.
src/test/regress/sql/xmin_horizon.sql verifies that accessing the view
without the required permissions raises an error. The SRF requires
the same.

> 3/ The patch was around 1400+ LOC and hard to review. Could you try to
> separate it out?
>

> 4/ I'm thinking of having a dedicated docs page, or as part of the
> routine vacuuming page, a separate section covering all the possible
> vacuum blockers that we know of so far (especially the two missing
> ones: effective xmins and backends with special proc flags) that this
> feature helps expose. This will be a good direction. I prefer to have
> this as the 0001 patch. Next patches would each add one blocker kind:
> 0002 transactions and tests, 0003 replication slots and tests, 0004
> prepared transactions and tests, 0005 standby support and tests.

According to the wiki, "Normally changes should be submitted as a
single patch that includes every file touched. If the patch is large
and can be logically separated into distinct and separately
commit-able sections for easier review, with a clear order they get
applied in described when applicable, that can be more straightforward
for reviewers to work with for more complicated patches." [1]
First, this patch is not large by PG standards. Second, the feature
contains no "separately commit-able sections", notwithstanding the
division you proposed. Your patch 0002 would introduce pg_xmin_horizon,
but the view would silently omit rows until at least 0004.

The patch documents "the two missing ones" you mentioned and every
holder class. For a broader reorganization of the vacuum
documentation, I recommend starting a new thread.

> And I
> prefer to keep all the tests in one TAP test file rather than
> spreading them across both SQL and TAP.

I followed the guidance in src/test/perl/README, which says
to prefer pg_regress when possible and reserve TAP for what pg_regress
cannot express.

> 5/ How about having a more impactful and user-readable name such as
> pg_vacuum_blockers or similar instead of pg_xmin_horizon?

The names of system views denote what they contain, and pg_xmin_horizon
reports horizon inputs, including rows that block nothing, so
pg_vacuum_blockers would misdescribe the contents.

> 6/ It's also better to report the top N blockers (replication slot,
> prepared transaction, long-running query, idle transaction, and so on)
> in one pass of proc-array/replication slots.

The view uses one pass over the procarray and another over the slot
array. A user who wants "the top N blockers" can write
ORDER BY age(shared_xmin) DESC NULLS LAST with a LIMIT.

[1] https://wiki.postgresql.org/wiki/Submitting_a_Patch

--
Scott Ray

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message shihao zhong 2026-09-05 03:36:43 Re: logical decoding: skip unnecessary snapshot distribution.
Previous Message shihao zhong 2026-09-05 03:24:25 Re: pg_createsubscriber: allow duplicate subscription names