From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Disallow altering invalidated replication slots |
Date: | 2024-09-10 05:23:07 |
Message-ID: | CAA4eK1LHpX4JUm1UGq6ixJRSdXpLNXfsxPcVrk=35ZHAvwj0cg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Sep 10, 2024 at 8:40 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi, here are some review comments for patch v1.
>
> ======
> Commit message
>
> 1.
> ALTER_REPLICATION_SLOT on invalidated replication slots is unnecessary
> as there is no way...
>
> suggestion:
> ALTER_REPLICATION_SLOT for invalid replication slots should not be
> allowed because there is no way...
>
> ======
> 2. Missing docs update
>
> Should this docs page [1] be updated to say ALTER_REPLICATION_SLOT is
> not allowed for invalid slots?
>
> ======
> src/backend/replication/slot.c
>
> 3.
> + if (MyReplicationSlot->data.invalidated != RS_INVAL_NONE)
> + ereport(ERROR,
> + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot alter replication slot \"%s\"", name),
> + errdetail("This replication slot was invalidated due to \"%s\".",
> + SlotInvalidationCauses[MyReplicationSlot->data.invalidated]));
> +
>
> I thought including the reason "invalid" (e.g. "cannot alter invalid
> replication slot \"%s\"") in the message might be better,
>
Agreed, I could see a similar case with a message ("cannot alter
invalid database \"%s\"") in the code. Additionally, we should also
include Shveta's suggestion to change the detailed message to other
similar messages in logical.c
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-09-10 05:29:57 | Re: [PATCH] pg_stat_activity: make slow/hanging authentication more visible |
Previous Message | Masahiko Sawada | 2024-09-10 05:18:49 | Re: pg_trgm comparison bug on cross-architecture replication due to different char implementation |