MIME-Version: 1.0
References: 
 <CAOzEurSgy-gDtwFmEbj5+R9PL0_G3qYB6nnzJtNStyuf87VSVg@mail.gmail.com>
 <CAHGQGwFVdccfhN7PFHU5gZcMNdQuyL8DhZSks2FuuTcZ5V_pGQ@mail.gmail.com>
 <CAGjGUAL-qZ6Na-cH4RY5raF4NwuF47ZVNyP0uMYQcks3w5b7Dw@mail.gmail.com>
 <CAA5RZ0s+UUXekbeGcC-H71kW=MfeaUCOV=yEWX94NXViO2-=pA@mail.gmail.com>
In-Reply-To: 
 <CAA5RZ0s+UUXekbeGcC-H71kW=MfeaUCOV=yEWX94NXViO2-=pA@mail.gmail.com>
From: Shinya Kato <shinya11.kato@gmail.com>
Date: Fri, 26 Dec 2025 14:34:01 +0900
Message-ID: 
 <CAOzEurQVUsRa45aKdi1Qrc4wVsQKSmbLcN+2A9Eu1W1jm2Y5fw@mail.gmail.com>
Subject: Re: Report oldest xmin source when autovacuum cannot remove tuples
To: Sami Imseih <samimseih@gmail.com>
Cc: wenhui qiu <qiuwenhuifx@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAOzEurQVUsRa45aKdi1Qrc4wVsQKSmbLcN%2B2A9Eu1W1jm2Y5fw%40mail.gmail.com>
Precedence: bulk

Thank you all for the review comments, and sorry for the late reply.
I will address the review comments in order.

On Sat, Nov 15, 2025 at 9:25=E2=80=AFAM Sami Imseih <samimseih@gmail.com> w=
rote:
> More importantly:
>
> 3/ As mentioned earlier in the thread, the "idle-in-transaction"
> transactions is not being reported correctly, particularly for write
> tansactions. I think that is an important missing case. The reason
> for this is the cutoff xmin is not being looked up against the current
> list of xid's, so we are not blaming the correct pid.
>
> 4/
> Thinking about point 3 above, I began to wonder if this
> whole thing can be simplified with inspiration. Looking at the
> existing  BackendXidGetPid(), I think it can.
>
> Based on BackendXidGetPid(), I tried a new routine called
> BackendXidFindCutOffReason() which can take in the cutoff xmin,
> passed in by vacuum and can walk though the proc array and
> determine the reason. We don't need to touch ComputeXidHorizons()
> to make this work, it seems to me. This comes with an additional
> walk though the procarray holding a shared lock, but I don't think
> this will be an issue.
>
> Attached is a rough sketch of BackendXidFindCutOffReason()
> For now, I just added NOTICE messages which will log with
> VACUUM (verbose) for testing.

Thanks for the revised proposal! Your approach is clear and makes the
code easier to read. However, I=E2=80=99m hesitant to proceed with this ide=
a
for the following reasons:

- The original proposal extends ComputeXidHorizons(), which is always
calculated, so there is almost no additional overhead.

- Your proposal incurs additional cost. Furthermore, the time lag
between the execution of ComputeXidHorizons() and
BackendXidFindCutOffReason() could lead to inaccurate logging.

- I don't believe it is necessary to distinguish between active
transactions and "idle in transaction." These states can change
rapidly, and as long as we have the PID, we can check the current
status via pg_stat_activity.

- Your comment made me realize that it might be appropriate to expose
the oldest xmin in the pg_stat_{all,user,sys}_tables views, rather
than just logging it. In that case, we would need to calculate the
oldest xmin horizon every time. This might be a topic for a separate
thread, but we could consider adding columns such as:
  - pg_stat_{all,user,sys}_tables.last_vacuum_oldest_xmin (xid)
  - pg_stat_{all,user,sys}_tables.last_vacuum_oldest_xmin_source (text)


--
Best regards,
Shinya Kato
NTT OSS Center