On Mon, Aug 29, 2022 at 3:40 PM Nathan Bossart <nathandboss...@gmail.com> wrote: > Agreed.
Attached is v2, which cleans up the structure of vacuum_set_xid_limits() a bit more. The overall idea was to improve the overall flow/readability of the function by moving the WARNINGs into their own "code stanza", just after the logic for establishing freeze cutoffs and just before the logic for deciding on aggressiveness. That is now the more logical approach (group the stanzas by functionality), since we can't sensibly group the code based on whether it deals with XIDs or with Multis anymore (not since the function was taught to deal with the question of whether caller's VACUUM will be aggressive). Going to push this in the next day or so. I also removed some local variables that seem to make the function a lot harder to follow in v2. Consider code like this: - freezemin = freeze_min_age; - if (freezemin < 0) - freezemin = vacuum_freeze_min_age; - freezemin = Min(freezemin, autovacuum_freeze_max_age / 2); - Assert(freezemin >= 0); + if (freeze_min_age < 0) + freeze_min_age = vacuum_freeze_min_age; + freeze_min_age = Min(freeze_min_age, autovacuum_freeze_max_age / 2); + Assert(freeze_min_age >= 0); Why have this freezemin temp variable? Why not just use the vacuum_freeze_min_age function parameter directly instead? That is a better representation of what's going on at the conceptual level. We now assign vacuum_freeze_min_age to the vacuum_freeze_min_age arg (not to the freezemin variable) when our VACUUM caller passes us a value of -1 for that arg. -1 effectively means "whatever the value of the vacuum_freeze_min_age GUC is'', which is clearer without the superfluous freezemin variable. -- Peter Geoghegan
v2-0001-Derive-freeze-cutoff-from-nextXID-not-OldestXmin.patch
Description: Binary data