On Wed, Feb 8, 2023 at 7:57 AM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > I agree to the direction and thanks for the patch. > > At Tue, 7 Feb 2023 17:08:54 +0000, "Hayato Kuroda (Fujitsu)" > <kuroda.hay...@fujitsu.com> wrote in > > > I noticed that previous ones are rejected by cfbot, even if they passed > > > on my > > > environment... > > > PSA fixed version. > > > > While analyzing more, I found the further bug that forgets initialization. > > PSA new version that could be passed automated tests on my github > > repository. > > Sorry for noise. > > 0002: > > This patch doesn't seem to offer a means to change the default > walsender behavior. We need a subscription option named like > "walsender_exit_mode" to do that. >
I don't think at this stage we need a subscription-level option, we can extend it later if this is really useful for users. For now, we can set this new option when min_apply_delay > 0. > > +ConsumeWalsenderOptions(List *options, WalSndData *data) > > I wonder if it is the right design to put options for different things > into a single list. I rather choose to embed the walsender option in > the syntax than needing this function. > > K_START_REPLICATION opt_slot opt_physical RECPTR opt_timeline > opt_shutdown_mode > > K_START_REPLICATION K_SLOTIDENT K_LOGICAL RECPTR opt_shutdown_mode > plugin_options > > where opt_shutdown_mode would be like "SHUTDOWN_MODE immediate". > The other option could have been that we just add it as a plugin_option for logical replication but it doesn't seem to match with the other plugin options. I think it would be better to have it as a separate option something like opt_shutdown_immediate and extend the logical replication syntax for now. We can later extend physical replication syntax when we want to expose such an option via physical replication. -- With Regards, Amit Kapila.