On Fri, 22 May 2026 at 22:05, Shinya Kato <[email protected]> wrote: > Thank you for your feedback! > > On Mon, Mar 16, 2026 at 8:19 PM Japin Li <[email protected]> wrote: >> >> On Mon, 16 Mar 2026 at 15:59, wenhui qiu <[email protected]> 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.
