On Wed, Jan 25, 2023 at 10:05 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jan 25, 2023 at 3:15 AM Peter Smith <smithpb2...@gmail.com> wrote: > > > > 1. > > @@ -210,7 +210,7 @@ int logical_decoding_work_mem; > > static const Size max_changes_in_memory = 4096; /* XXX for restore only */ > > > > /* GUC variable */ > > -int logical_decoding_mode = LOGICAL_DECODING_MODE_BUFFERED; > > +int logical_replication_mode = LOGICAL_REP_MODE_BUFFERED; > > > > > > I noticed that the comment /* GUC variable */ is currently only above > > the logical_replication_mode, but actually logical_decoding_work_mem > > is a GUC variable too. Maybe this should be rearranged somehow then > > change the comment "GUC variable" -> "GUC variables"? > > > > I think moving these variables together doesn't sound like a good idea > because logical_decoding_work_mem variable is defined with other > related variable. Also, if we are doing the last comment, I think that > will obviate the need for this. > > > ====== > > src/backend/utils/misc/guc_tables.c > > > > @@ -4908,13 +4908,13 @@ struct config_enum ConfigureNamesEnum[] = > > }, > > > > { > > - {"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, > > GUC_NOT_IN_SAMPLE > > }, > > - &logical_decoding_mode, > > - LOGICAL_DECODING_MODE_BUFFERED, logical_decoding_mode_options, > > + &logical_replication_mode, > > + LOGICAL_REP_MODE_BUFFERED, logical_replication_mode_options, > > NULL, NULL, NULL > > }, > > > > That gettext_noop string seems incorrect. I think Kuroda-san > > previously reported the same, but then you replied it has been fixed > > already [1] > > > > > 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. > > > > But this doesn't look fixed to me. (??) > > > > Okay, so, how about the following for the 0001 patch: > short desc: Controls when to replicate each change. > long desc: On the publisher, it allows streaming or serializing each > change in logical decoding. >
I have updated the patch accordingly and it looks good to me. I'll push this first patch early next week (Monday) unless there are more comments. -- With Regards, Amit Kapila.
v88-0001-Rename-GUC-logical_decoding_mode-to-logical_repl.patch
Description: Binary data