MIME-Version: 1.0
References: 
 <CAOzEurSgy-gDtwFmEbj5+R9PL0_G3qYB6nnzJtNStyuf87VSVg@mail.gmail.com>
In-Reply-To: 
 <CAOzEurSgy-gDtwFmEbj5+R9PL0_G3qYB6nnzJtNStyuf87VSVg@mail.gmail.com>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Wed, 5 Nov 2025 00:58:51 +0900
Message-ID: 
 <CAHGQGwFVdccfhN7PFHU5gZcMNdQuyL8DhZSks2FuuTcZ5V_pGQ@mail.gmail.com>
Subject: Re: Report oldest xmin source when autovacuum cannot remove tuples
To: Shinya Kato <shinya11.kato@gmail.com>
Cc: 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/CAHGQGwFVdccfhN7PFHU5gZcMNdQuyL8DhZSks2FuuTcZ5V_pGQ%40mail.gmail.com>
Precedence: bulk

On Fri, Oct 31, 2025 at 3:32=E2=80=AFPM Shinya Kato <shinya11.kato@gmail.co=
m> wrote:
>
> Hi hackers,
>
> I am proposing to add the reason for the oldest xmin to VACUUM logs.
> This feature would be useful for identifying why dead tuples cannot be
> removed, thereby helping to diagnose and prevent table bloat.

+1

I like this idea. Thanks for working on this!


> This PoC patch addresses this problem. The implementation now outputs
> the reason for the oldest xmin and, where applicable, the backend PID.
> This information was originally discarded when calculating the oldest
> xmin horizon, and the computation required to retrieve these reasons
> is considered reasonable.
>
> The patch is attached. What do you think?

According to cfbot, the 010_autovacuum_oldest_xmin_reason regression test
passes on some platforms but fails on others (see [1]), so it doesn't
appear stable.


When I set up a primary and standby with hot_standby_feedback enabled,
then created an old prepared transaction expected to prevent dead tuples
from being vacuumed, VACUUM VERBOSE reported "hot standby feedback"
instead of "prepared transaction" as the oldest xmin source. This isn't a b=
ug
since both xmins are the same in this case. But it may be confusing?
Would it be better to report "prepared transaction" in such cases?


+ case OLDESTXMIN_SOURCE_ACTIVE_TRANSACTION:
+ msgfmt =3D include_pid ?
+ _("oldest xmin source: active transaction (pid=3D%d)\n") :
+ _("oldest xmin source: active transaction\n");
+ break;
+ case OLDESTXMIN_SOURCE_HOT_STANDBY_FEEDBACK:
+ msgfmt =3D include_pid ?
+ _("oldest xmin source: hot standby feedback (pid=3D%d)\n") :
+ _("oldest xmin source: hot standby feedback\n");

In these two cases, the PID should always be non-zero, so the message
formats without (pid=3D%d) might not be necessary.


+ /* Identify what forced each of the horizons above. */
+ OldestXminInfo shared_oldest_nonremovable_info;
+ OldestXminInfo catalog_oldest_nonremovable_info;
+ OldestXminInfo data_oldest_nonremovable_info;
+ OldestXminInfo temp_oldest_nonremovable_info;

It might be good to add a comment explaining why we track
sources only for these four oldest xmins, and not for others
like oldest_considered_running.


+ TransactionId old;
+ TransactionId new_horizon;
+
+ if (!TransactionIdIsValid(candidate))
+ return;

The TransactionIdIsValid(candidate) check may be redundant,
since TransactionIdOlder(old, candidate) already performs
the same validation.


- switch (GlobalVisHorizonKindForRel(rel))
+ kind =3D GlobalVisHorizonKindForRel(rel);
+ switch (kind)

This change doesn't seem necessary.

Regards,

[1] https://cirrus-ci.com/task/6063548834512896

--=20
Fujii Masao