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 AM Sami Imseih <[email protected]> wrote: > 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’m hesitant to proceed with this idea 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
