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