Dilip Kumar <dilipbalaut@gmail.com> writes:
>> IMHO we can just query the 'max_slot_wal_keep_size' after
>> start_postmaster() and check what is the final resultant value. So now
>> we will only throw an error if the final value is not -1. And we can
>> remove the hook from the server all together. Thoughts?
> I could come up with an attachment patch.
I don't love this patch. It's adding more cycles and more complexity
to pg_upgrade, when there is a simpler and more direct solution:
re-order the construction of the postmaster command line in
start_postmaster() so that our "-c max_slot_wal_keep_size" will
override anything the user supplies.
There's a bigger picture here, though. The fundamental thing that
I find wrong with the current code is that knowledge of and
responsibility for this max_slot_wal_keep_size hack is spread across
both pg_upgrade and the server. It would be better if it were on
just one side. Now, unless we want to change that Assert that
8bfb231b4 put into InvalidatePossiblyObsoleteSlot(), the server side
is going to be aware of this decision. So I'm inclined to think
that we should silently enforce max_slot_wal_keep_size = -1 in
binary-upgrade mode in the server's GUC check hook, and then remove
knowledge of it from pg_upgrade altogether. Maybe the same for
idle_replication_slot_timeout, which really has got the same issue
that we don't want users overriding that choice.
regards, tom lane
