On Fri, Nov 03, 2023 at 01:33:26PM +1100, Peter Smith wrote: > On Fri, Nov 3, 2023 at 1:11 PM Amit Kapila <amit.kapil...@gmail.com> wrote: >> Now, that Michael also committed another similar change in commit >> 7021d3b176, it is better to be consistent in both cases. So, either we > > I agree. Both patches are setting a special GUC value at the command > line, and both of them don't want the user to somehow override that. > Since the requirements are the same, I felt the implementations > (regardless if they use a guc hook or something else) should also be > done the same way. Yesterday I posted a review comment on the other > thread [1] (#2c) trying to express the same point about consistency.
Yeah, I certainly agree about consistency in the implementation for both sides of the coin. Nevertheless, I'm still +-0 on the GUC hook addition as I am wondering if there could be a case where one would be interested in enforcing the state of the GUCs anyway, and we'd prevent entirely that. Another thing that we can do for max_logical_replication_workers, rather than a GUC hook, is to add a check on IsBinaryUpgrade in ApplyLauncherRegister(). At least that would be consistent with what we do for autovacuum as the apply worker is just a bgworker. -- Michael
signature.asc
Description: PGP signature
