On Tue, Dec 11, 2018 at 11:59:08AM +0300, Sergei Kornilov wrote:
>> but I think that this patch should document clearly that if
>> primary_conninfo or primary_slot_name are changed then the WAL receiver
>> is stopped immediately.
>
> Good idea, will change.
+ <para>
+ <application>walreceiver</application> will be restarted after
+ <varname>primary_conninfo</varname> was changed.
+ </para>
Hm. It may be cleaner to use "WAL receiver" here for clarity. Perhaps
that's a matter of state but that seems cleaner when referring to the
actual process. The docs share both grammar, depending on the context.
> If walreceiver will use primary_conninfo GUC at startup - i see no
> reason to leave *conninfo in RequestXLogStreaming. These parameters
> will be misleading, we request them, but not using for anything.
Oh, indeed. My apologies for being confused here, I can see now your
point. It seems to me that this is an extra cleanup caused by the
recovery parameters switched to be GUCs, and not something that we
should be changed as an effect to SIGHUP for primary_conninfo and
primary_slot_name. So let's do this cleanup first. For this purpose, I
am going to post a new thread with a proper patch, with in CC the folks
who moved the recovery parameters to be GUCs.
>> +$node_standby_2->reload; # should have effect without restart
>> This does not wait for the change to be effective, so I think that you
>> introduce a race condition for slow machines with this test, making it
>> unstable.
>
> No, here is no race condition because just after this line we wait
> this replication slot in upstream pg_catalog.pg_replication_slots
> (get_slot_xmins routine).
> Before reload we have no replication slot and should reconnect here
> with replication slot. I can add some comment here.
Good point, I have forgotten the call to get_slot_xmins() below.
--
Michael