| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
| Date: | 2025-10-27 19:20:18 |
| Message-ID: | CAD21AoChak=5J_ovSHz0uLgtvOfd=B46TCf8W9M9k95e3BZqng@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Oct 26, 2025 at 5:54 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Sawada.
>
> A couple of comments for v22-0001.
>
> ======
>
> 1.
> + /*
> + * We don't need this warning message when wal_level >= 'replica' since
> + * logical decoding is automatically enabled up on a logical slot
> + * creation.
> + */
> + if (wal_level == WAL_LEVEL_MINIMAL)
> ereport(WARNING,
> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> - errmsg("\"wal_level\" is insufficient to publish logical changes"),
> - errhint("Set \"wal_level\" to \"logical\" before creating subscriptions.")));
> + errmsg("logical decoding must be enabled to publish logical changes"),
> + errhint("Before creating subscriptions, set \"wal_level\" >=
> \"logical\" or create a logical replication slot when \"wal_level\" =
> \"replica\".")));
>
> 1a.
> Sorry, for repeating the same question about the HINT message, but
> AFAICT I did not yet notice any reply to directly reject or address my
> question.
>
> Basically, I felt the errhint part that says "...or create a logical
> replication slot when wal_level = replica." is overkill, because
> creating the subscription will do that anyway.
> In other words, we don't need to tell the user this is needed "Before
> creating subscriptions". So, I thought the only requirement to be able
> to publish a subscription is for wal_level >= replica.
>
> SUGGESTION
> errhint("Before creating subscriptions, set \"wal_level\" >= \"replica\".")
>
Agreed with the suggested change.
> ~~
>
> 1b.
> Since wal_level needs to be >= replica, the condition for this WARNING
> might be better to be written as:
> if (wal_level < WAL_LEVEL_REPLICA)
Changed.
>
> ~~
>
> 1c.
> Now that the condition for this warning was slightly changed, there
> seems to be no test case anymore for this WARNING. Isn't it better to
> still have a test case for this?
There are other tests covering this path. For instance, we check the
WARNING message in 001_rep_changes.pl.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2025-10-27 19:23:13 | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |
| Previous Message | Luis Felippe | 2025-10-27 18:33:59 | [PATCH] Fix ICU strength not being honored in collation rules |