I presented this patch at PostgreSQL Developer Meeting [1], an offline event in Japan, and Fujii-san gave me the following comments.
1. If the blocker is looked up right after the cutoff is computed, one extra scan is enough. 2. Information that needs a second lookup, such as the active versus idle state and the prepared transaction GID, is out of scope for this patch and should come as a follow-up. 3. Do not widen the scope of the patch. v10 addresses all three. 1. My previous message proposed scanning twice, right after the cutoff and again when the log line is emitted, to keep the line accurate. As Fujii-san says, the scan right after the cutoff is enough. The process named in the line may already be gone by the time the line is written, but that is true of the other values in the VACUUM log as well. So v10 scans the ProcArray and the replication slots once, right after the cutoffs are computed and only for an instrumented vacuum, keeps the blocker, and prints it when the log line is emitted: ``` tuples: 0 removed, 20000 remain, 20000 are dead but not yet removable removable cutoff: 695, which was 2 XIDs old when operation ended removable cutoff was held back by: prepared transaction ``` 2. Fixed as suggested. The line v10 prints is a simple one, and I plan to report more useful detail in follow-up patches, including Scott's comments [2]. 3. Bharath's WARNING line and pg_stat_progress_vacuum column [3] may well be useful, but I would like to keep them out of scope for now. Other changes from v9: - The log line now says "removable cutoff was held back by". "oldest xmin" is not a term the user-facing output uses, and VACUUM already calls this value the removable cutoff on the line above. - The replication slot name is now copied while ReplicationSlotControlLock is held, which closes the race Scott reported [2]. - Slots are reported by name whatever their type. v9 reported a slot that is not tied to a database and is held by some process as hot standby feedback, so a subscriber with retain_dead_tuples and no standby at all was told hot standby feedback and given the pid of the logical replication launcher holding pg_conflict_detection. - Comment cleanup. - Removed the serializable test, which duplicated another test case. The patch is attached. Thoughts? [1] https://speakerdeck.com/shinyakato_/postgresql-improve-vacuum-log-en [2] https://www.postgresql.org/message-id/o8gz3Yoq3LmnoO9oeF_3pPSlndgQmosyYKAFkiVZ60HyiA_g8cDQVs8AMkSJBRrz0-nclaPVJ2c0ilGyi3TIuAqNqjjOYzLIaJa5V65tZDg%3D%40scottray.io [3] https://www.postgresql.org/message-id/CALj2ACWaCpesxSupvN1ZpdQ1oRDdu8YvXg5-zeLFqY6zqofVGA%40mail.gmail.com -- Shinya Kato NTT OSS Center
v10-0001-Report-what-held-back-the-removable-cutoff-in-VA.patch
Description: Binary data
