RE: Perform streaming logical transactions by background workers and parallel apply

On Monday, January 23, 2023 8:34 PM Kuroda, Hayato wrote:
>
> Followings are my comments.

Thanks for your comments.

>
> 1. guc_tables.c
>
> ```
> static const struct config_enum_entry logical_decoding_mode_options[] = {
> - {"buffered", LOGICAL_DECODING_MODE_BUFFERED, false},
> - {"immediate", LOGICAL_DECODING_MODE_IMMEDIATE, false},
> + {"buffered", LOGICAL_REP_MODE_BUFFERED, false},
> + {"immediate", LOGICAL_REP_MODE_IMMEDIATE, false},
> {NULL, 0, false}
> };
> ```
>
> This struct should be also modified.

Modified.

>
> 2. guc_tables.c
>
>
> ```
> - {"logical_decoding_mode", PGC_USERSET,
> DEVELOPER_OPTIONS,
> + {"logical_replication_mode", PGC_USERSET,
> + DEVELOPER_OPTIONS,
> gettext_noop("Allows streaming or serializing each
> change in logical decoding."),
> NULL,
> ```
>
> I felt the description seems not to be suitable for current behavior.
> A short description should be like "Sets a behavior of logical replication", and
> further descriptions can be added in lond description.

I adjusted the description here.

> 3. config.sgml
>
> ```
> <para>
> This parameter is intended to be used to test logical decoding and
> replication of large transactions for which otherwise we need to
> generate the changes till
> <varname>logical_decoding_work_mem</varname>
> is reached.
> </para>
> ```
>
> I understood that this part described the usage of the parameter. How about
> adding a statement like:
>
> " Moreover, this can be also used to test the message passing between the
> leader and parallel apply workers."

Added.

> 4. 015_stream.pl
>
> ```
> +# Ensure that the messages are serialized.
> ```
>
> In other parts "changes" are used instead of "messages". Can you change the
> word?

Changed.

Best Regards,
Hou zj