Re: Why ALTER SUBSCRIPTION ... SET (slot_name='none') requires subscription disabled?

On Wed, Jul 7, 2021 at 7:25 PM Japin Li <japinli(at)hotmail(dot)com> wrote:
>
> Hi, hackers
>
> The documentation [1] says:
>
> When dropping a subscription that is associated with a replication slot on the
> remote host (the normal state), DROP SUBSCRIPTION will connect to the remote
> host and try to drop the replication slot as part of its operation. This is
> necessary so that the resources allocated for the subscription on the remote
> host are released. If this fails, either because the remote host is not
> reachable or because the remote replication slot cannot be dropped or does not
> exist or never existed, the DROP SUBSCRIPTION command will fail. To proceed in
> this situation, disassociate the subscription from the replication slot by
> executing ALTER SUBSCRIPTION ... SET (slot_name = NONE).
>
> However, when I try this, it complains the subscription is enabled, this command
> requires the subscription disabled. Why we need this limitation?
>

If we don't have this limitation then even after you have set the slot
name to none, the background apply worker corresponding to that
subscription will continue to stream changes via the previous slot.

> In src/backend/commands/subscriptioncmds.c:
>
> if (IsSet(opts.specified_opts, SUBOPT_SLOT_NAME))
> {
> if (sub->enabled && !opts.slot_name)
> ereport(ERROR,
> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> errmsg("cannot set %s for enabled subscription",
> "slot_name = NONE")));
>
> if (opts.slot_name)
> values[Anum_pg_subscription_subslotname - 1] =
> DirectFunctionCall1(namein, CStringGetDatum(opts.slot_name));
> else
> nulls[Anum_pg_subscription_subslotname - 1] = true;
> replaces[Anum_pg_subscription_subslotname - 1] = true;
> }
>
>
> OTOH, when I execute ALTER SUBSCRIPTION ... SET (slot_name=''), it doesn't complain. However,
> SELECT select pg_create_logical_replication_slot('', 'pgoutput') complains slot name is too
> short. Although, the slot will be created at publisher, and validate the slot name, IMO, we
> can also validate the slot_name in parse_subscription_options() to get the error early.
> Attached fixes it. Any thoughts?
>

Oh, I think this should be fixed. Can anyone else think this to be
valid behavior?

With Regards,
Amit Kapila.