Re: pgsql: walreceiver uses a temporary replication slot by default

On Wed, Jan 22, 2020 at 06:58:46PM +0300, Sergei Kornilov wrote:
> I would like to cross-post here a patch with such changes that I posted in "allow online change primary_conninfo" thread.
> This thread is more appropriate for discussion about wal_receiver_create_temp_slot.
>
> PS: I posted this patch in both threads mostly to make cfbot happy.

Thanks for posting this patch, Sergei. Here is a review to make
things move on.

- * Create temporary replication slot if no slot name is configured or
- * the slot from the previous run was temporary, unless
- * wal_receiver_create_temp_slot is disabled. We also need to handle
- * the case where the previous run used a temporary slot but
- * wal_receiver_create_temp_slot was changed in the meantime. In that
- * case, we delete the old slot name in shared memory. (This would
+ * Create temporary replication slot if requested. In that
+ * case, we update slot name in shared memory. (This would

The set of comments you are removing from walreceiver.c to decide if a
temporary slot needs to be created or not should be moved to
walreceiverfuncs.c as you move the logic from the WAL receiver startup
phase to the moment the WAL receiver spawn is requested.

I agree with the simplifications in WalReceiverMain() as you have
switched wal_receiver_create_temp_slot to be PGC_POSTMASTER, so
modifications are no longer a state that matter.

It would be more consistent with primary_conn_info and
primary_slot_name if wal_receiver_create_temp_slot is passed down as
an argument of RequestXLogStreaming().

As per the discussion done on this thread, let's also switch the
parameter default to be disabled. Peter, as the committer of 3297308,
it would be good if you could chime in.
--
Michael