On Mon, Jul 28, 2025 at 5:34 AM Hayato Kuroda (Fujitsu)
<kuroda.hay...@fujitsu.com> wrote:
>
> Dear Sawada-san,
>
> > Thank you for testing the patch!
> >
> > I've reworked the locking part in the patch. The attached v4 patch
> > should address all review comments including your previous
> > comments[1].
>
> Thanks for making the patch! I resumed to spending time for the project.
Thank you for reviewing the patch!
> Here are my comments.
>
> 1.
> Just in case - can you modify xlogdesc.c based on your fix?
Will fix.
>
> 2.
> Currently pg_upgrade has below checking:
> ```
> if (nslots_on_old > 0 && strcmp(wal_level, "logical") != 0)
> pg_fatal("\"wal_level\" must be \"logical\" but is set to
> \"%s\"",
> wal_level);
> ```
>
> But this can be relaxed because wal_level can be adjusted appropriately. IIUC
> it
> is enough to be higher than "minimal". Is it right?
Right, will fix.
>
> 3.
> Currently pg_createsubscriber has below checking:
> ```
> if (strcmp(wal_level, "logical") != 0)
> {
> pg_log_error("publisher requires \"wal_level\" >=
> \"logical\"");
> failed = true;
> }
> ```
>
> I feel the checking is completely not needed, because pg_createsubscriber
> needs
> a streaming standby and wal_level = minimal cannot be set with this node
> placement.
> Thought?
Yes, we can get rid of this check.
>
> 4.
> We should update PG_CONTROL_VERSION and pg_controldata as well.
Right, I'll update pg_controldata. For PG_CONTROL_VERSION, I'm going
to update before the push.
>
> 5.
> I'm wondering how pg_resetwal handles. Since all the replication slot cannot
> be
> used after the command, logicalDecodingEnabled can be set to false, right?
I think that logical decoding remains enabled as long as logical slots
are present. For example, it remains enabled even if the sole logical
slot is invalidated.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
