On Mon, Jul 15, 2024 at 4:10 PM Nathan Bossart <nathandboss...@gmail.com> wrote: > You don't, but the GUC check hook should always return true when > summarize_wal is processed first. We'd rely on the PostmasterMain() check > to fail in that case.
OK, I see. So at startup time, the check hook might or might not catch a configuration mismatch, but if it doesn't, then the PostmasterMain() check will fire. Later, we'd have an unsolvable problem if both wal_level and summarize_wal could change, but since wal_level can't change after startup time, we only need to check summarize_wal, and it can rely on the value of wal_level being correct. TBH, I don't want to do that. I think it's too fragile. It's the sort of thing that just barely works given the exact behavior of these particular GUCs, but it relies on a bunch of subtle assumptions which won't be evident to future readers of the code. People will very possibly copy this barely-working code into other contexts where it doesn't work at all, or they'll think the code implementing this is buggy even if it isn't. We don't really need that check for correctness here, and it arguably even prohibits more than necessary - e.g. suppose you crash without summarizing all the WAL and then restart with wal_level=minimal. It's perfectly fine to run the summarizer and have it catch up with all possible pre-crash summarization, but the proposed change would prohibit it. Granted, the current check would require you to start with summarize_wal=off and then enable it later to get that done, but we could remove that check. Or maybe we should leave it alone, but my point is: if we have a reasonable option to decouple the values of two GUCs so that the legal values of one don't depend on each other, I think that is always going to be better and simpler than trying to implement cross-checks on the values for which we don't have proper infrastructure. -- Robert Haas EDB: http://www.enterprisedb.com
