| From: | Japin Li <japinli(at)hotmail(dot)com> |
|---|---|
| To: | Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> |
| Cc: | wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, Sami Imseih <samimseih(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Report oldest xmin source when autovacuum cannot remove tuples |
| Date: | 2026-05-23 01:40:39 |
| Message-ID: | SY7PR01MB10921CD524617DCF4FE8E2565B60C2@SY7PR01MB10921.ausprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, 22 May 2026 at 22:05, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
> Thank you for your feedback!
>
> On Mon, Mar 16, 2026 at 8:19 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>>
>> On Mon, 16 Mar 2026 at 15:59, wenhui qiu <qiuwenhuifx(at)gmail(dot)com> wrote:
>> > HI Shinya
>> >> typedef enum XidHorizonBlockerType
>> >> {
>> >> XHB_NONE = 0,
>> >> XHB_ACTIVE_TRANSACTION,
>> >> XHB_IDLE_IN_TRANSACTION,
>> >> XHB_PREPARED_TRANSACTION,
>> >> XHB_XMIN_ACTIVE_TRANSACTION,
>> >> XHB_XMIN_IDLE_IN_TRANSACTION,
>> >> XHB_HOT_STANDBY_FEEDBACK,
>> >> XHB_REPLICATION_SLOT,
>> >> }
>> > Thank you for your working on this ,I have another small suggestion
>> > The priority ordering encoded in XidHorizonBlockerType determines which blocker gets reported when multiple candidates
>> > exist. In particular:
>> >
>> > ACTIVE_TRANSACTION
>> > IDLE_IN_TRANSACTION
>> > PREPARED_TRANSACTION
>> >
>> > Prepared transactions are currently ranked after idle-in-transaction sessions. Operationally, prepared transactions are
>> > often harder for DBAs to resolve than idle sessions, so it might be worth clarifying the rationale behind this ordering
>> > or reconsidering whether prepared transactions should have higher priority.
>>
>> Agreed. Explaining the reason for this priority is very helpful.
>
> We always pick a blocker from the xid-match group first (it is the
> transaction actually holding the horizon, while the xmin-match entries
> are just held back by it). Within the xid-match group, the
> active/idle/prepared order never matters: a given xid is owned by only
> one backend, so when the horizon equals a proc's xid there is only one
> matching entry, and it is exactly one of active, idle, or prepared. So
> moving prepared ahead of idle would not change which blocker we
> report.
>
>> >> typedef enum XidHorizonBlockerType
>> >> {
>> >> XHB_NONE = 0,
>> >> XHB_ACTIVE_TRANSACTION,
>> >> XHB_PREPARED_TRANSACTION,
>> >> XHB_IDLE_IN_TRANSACTION,
>> >> XHB_XMIN_ACTIVE_TRANSACTION,
>> >> XHB_XMIN_IDLE_IN_TRANSACTION,
>> >> XHB_HOT_STANDBY_FEEDBACK,
>> >> XHB_REPLICATION_SLOT,
>> >> }
>> > Another one:
>> > Currently GetXidHorizonBlocker() selects only one blocker (based on the enum priority) even though multiple independent
>> > sources could hold back the xmin horizon simultaneously. For example, it is possible to have both a prepared transaction
>> > and a replication slot preventing the horizon from advancing.
>> > Have you considered reporting all detected blockers instead of just the highest-priority one? Returning only a single
>> > entry might hide other relevant blockers from the user.
>> >
>>
>> I'm also curious — why don't we list all the blockers? Did I miss anything?
>
> I did think about this, but I would like to keep reporting one blocker
> in the VACUUM log, for two reasons.
>
> First, the log can get very large. In Sami's earlier example [0], a
> pgbench run had many backends all sharing the same xmin while only one
> idle-in-transaction backend actually owned the cutoff xid. Reporting
> every blocker would print 20+ lines, almost all of them just victims
> of the same root cause, which makes the log harder to read, not
> easier.
>
> Second, the one blocker we report is the root cause (the xid owner).
> Once the DBA resolves it, the next VACUUM will show the next blocker
> if one remains.
>
> This is also why the code is split into GetXidHorizonBlockers(), which
> already collects every candidate, and GetXidHorizonBlocker(), which
> picks the highest-priority one for the log. The "show everything" case
> is what I would like to expose later through a dynamic statistics
> view, where a full list makes more sense than in a VACUUM log line.
>
>
> I've rebased the patch.
>
Thanks for updating the patch. LGTM. Just one nitpick.
+ int pid; /* backend pid (0 for slots) */
Per the code, the prepared transaction is also associated with a PID of zero.
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-05-23 01:58:40 | Re: postgres_fdw: use_scram_passthrough on user mapping is ignored when also set on server |
| Previous Message | Lukas Fittl | 2026-05-23 01:15:00 | Re: Improve pg_stat_statements scalability |