On Tue, Dec 6, 2022 at 5:57 AM samay sharma <smilingsa...@gmail.com> wrote: > > Hi, > > On Mon, Oct 24, 2022 at 12:45 AM Peter Smith <smithpb2...@gmail.com> wrote: >> >> Hi hackers. >> >> There is a docs Logical Replication section "31.10 Configuration >> Settings" [1] which describes some logical replication GUCs, and >> details on how they interact with each other and how to take that into >> account when setting their values. >> >> There is another docs Server Configuration section "20.6 Replication" >> [2] which lists the replication-related GUC parameters, and what they >> are for. >> >> Currently AFAIK those two pages are unconnected, but I felt it might >> be helpful if some of the parameters in the list [2] had xref links to >> the additional logical replication configuration information [1]. PSA >> a patch to do that. > > > +1 on the patch. Some feedback on v5 below. >
Thanks for your detailed review comments! I have changed most things according to your suggestions. Please check patch v6. > > + <para> > > + For <firstterm>logical replication</firstterm> configuration settings > > refer > > + also to <xref linkend="logical-replication-config"/>. > > + </para> > > + > > I feel the top paragraph needs to explain terminology for logical replication > like it does for physical replication in addition to linking to the logical > replication config page. I'm recommending this as we use terms like > subscriber etc. in description of parameters without introducing them first. > > As an example, something like below might work. > > These settings control the behavior of the built-in streaming replication > feature (see Section 27.2.5) and logical replication (link). > > For physical replication, servers will be either a primary or a standby > server. Primaries can send data, while standbys are always receivers of > replicated data. When cascading replication (see Section 27.2.7) is used, > standby servers can also be senders, as well as receivers. Parameters are > mainly for sending and standby servers, though some parameters have meaning > only on the primary server. Settings may vary across the cluster without > problems if that is required. > > For logical replication, servers will either be publishers (also called > senders in the sections below) or subscribers. Publishers are ...., > Subscribers are... > OK. I split this blurb into 2 parts – streaming and logical replication. The streaming replication part is the same as before. The logical replication part is new. > > + <para> > > + See <xref linkend="logical-replication-config"/> for more details > > + about setting <varname>max_replication_slots</varname> for logical > > + replication. > > + </para> > > > The link doesn't add any new information regarding max_replication_slots > other than "to reserve some for table sync" and has a good amount of > unrelated info. I think it might be useful to just put a line here asking to > reserve some for table sync instead of linking to the entire logical > replication config section. > OK. I copied the tablesync note back to config.sgml definition of 'max_replication_slots' and removed the link as suggested. Frankly, I also thought it is a bit strange that the max_replication_slots in the “Sending Servers” section was describing this parameter for “Subscribers”. OTOH, I did not want to split the definition in half so instead, I’ve added another Subscriber <varlistentry> that just refers back to this place. It looks like an improvement to me. > > - Logical replication requires several configuration options to be set. > > + Logical replication requires several configuration parameters to be set. > > May not be needed? The docs have references to both options and parameters > but I don't feel strongly about it. Feel free to use what you prefer. OK. I removed this. > > I think we should add an additional line to the intro here saying that > parameters are mostly relevant only one of the subscriber or publisher. Maybe > a better written version of "While max_replication_slots means different > things on the publisher and subscriber, all other parameters are relevant > only on either the publisher or the subscriber." > OK. Done but with slightly different wording to that. > > + <sect2 id="logical-replication-config-notes"> > > + <title>Notes</title> > > I don't think we need this sub-section. If I understand correctly, these > parameters are effective only on the subscriber side. So, any reason to not > include them in that section? OK. I moved these notes into the "Subscribers" section as suggested, and removed "Notes". > > > + > > + <para> > > + Logical replication workers are also affected by > > + <link > > linkend="guc-wal-receiver-timeout"><varname>wal_receiver_timeout</varname></link>, > > + <link > > linkend="guc-wal-receiver-status-interval"><varname>wal_receiver_status_interval</varname></link> > > and > > + <link > > linkend="guc-wal-retrieve-retry-interval"><varname>wal_receiver_retry_interval</varname></link>. > > + </para> > > + > > I like moving this; it makes more sense here. Should we remove it from > config.sgml? It seems a bit out of place there as we generally talk only > about individual parameters there and this line is general logical > replication subscriber advise which is more suited to logical-replication.sgml OK. I agree, it looked repetitive since the link to the logical-replication page is nearby this information anyway, so I’ve removed it from the config.sgml as you suggested. > > > + <para> > > + Configuration parameter > > + <link > > linkend="guc-max-worker-processes"><varname>max_worker_processes</varname></link> > > + may need to be adjusted to accommodate for replication workers, at > > least ( > > + <link > > linkend="guc-max-logical-replication-workers"><varname>max_logical_replication_workers</varname></link> > > + + <literal>1</literal>). Some extensions and parallel queries also take > > + worker slots from <varname>max_worker_processes</varname>. > > + </para> > > + > > + </sect2> > > I think we should move this to the subscriber section as said above. It's > useful to know this and people might skip over the notes. OK. Done. ------ Kind Regards, Peter Smith. Fujitsu Australia
v6-0001-Logical-replication-GUCs-links-and-tidy.patch
Description: Binary data
