Some review comments for v23-0003 (docs). ====== doc/src/sgml/logical-replication.sgml
1. GENERAL. Mentioning those specific enum values of coflict_log_dest every time seemed overkill. In most cases you do really need to mention 'all' or 'log' ot 'table'. IMO the user should use the link to the 'conflict_log_destination' parameter to see that level of detail. Some examples what I am referring to: (29.2 Subscription) BEFORE The CREATE SUBSCRIPTION command provides the conflict_log_destination option to record detailed conflict information in a structured, queryable format. When this parameter is set to table or all, the system automatically manages a dedicated conflict log table, which is created and dropped along with the subscription. SUGGESTION The CREATE SUBSCRIPTION conflict_log_destination parameter can be set to create a dedicated conflict log table that is automatically managed with the subscription. ~ (29.8) BEFORE When the conflict_log_destination parameter is set to table or all, the system automatically creates a new table with a predefined schema to log conflict details. SUGGESTION The conflict_log_destination parameter can automatically create a dedicated conflict log table. ~ (29.8 Conflicts) BEFORE If conflict_log_destination is left at the default setting or explicitly configured as log or all, logical replication conflicts are logged in the following format: SUGGESTION When conflict_log_destination is set to log conflicts to the server log, the following format is used: ~ (29.9 Restrictions) BEFORE: The internal table automatically created when conflict_log_destination is set to table or all is excluded from logical replication. It will not be published, even if a publication on the subscriber is defined using FOR ALL TABLES. SUGGESTION Conflict log tables (see conflict_log_destination parameter) are never published, even when using FOR ALL TABLES in a publication. ~~~ (29.8 conflicts) 2. <para> - The log format for logical replication conflicts is as follows: + When the <link linkend="sql-createsubscription-params-with-conflict-log-destination"><literal>conflict_log_destination</literal></link> + parameter is set to <literal>table</literal> or <literal>all</literal>, the system + automatically creates a new table with a predefined schema to log conflict + details. This table is created in the dedicated + <literal>pg_conflict</literal> namespace. The name of the conflict log table + is <literal>pg_conflict_<subid></literal>. The schema of this + table is detailed in + <xref linkend="logical-replication-conflict-log-schema"/>. + </para> Maybe instead of mentioned "schema" twice you can remove that part that says "with a predefined schema", and then also change: /The schema of this table is detailed in.../The predefined schema of this table is detailed in.../ ~~~ (29.8 Conflicts) 3. + <para> + The conflicting row data, including the incoming remote row and the associated + local conflict details, is stored in <type>JSON</type> formats (<literal>remote_tuple</literal> + and <literal>local_conflicts</literal>) for flexible querying and analysis. + </para> Would this be better rearranged slightly to name those columns where they are mentioned? SUGGESTION The conflicting row data, including the incoming remote row (remote_tuple) and the associated local conflict details (local_conflicts), is stored in JSON format, for flexible querying and analysis. ====== doc/src/sgml/ref/create_subscription.sgml (value: table) 4. + <caution> + <para> + The internal conflict log table is strictly tied to the lifecycle of the + subscription or the <literal>conflict_log_destination</literal> setting. If + the subscription is dropped, or if the destination is changed to + <literal>log</literal>, the table and all its recorded conflict data are + <emphasis>permanently deleted</emphasis>. To perform a post-mortem analysis + after removing a subscription, users must manually back up the conflict log + table before the deletion occurs. + </para> + </caution> 4a. That last sentence seems a bit awkward. You already explained that the CLT is removed when a subscription is dropped. Here is one simpler alternative: SUGGESTION If post-mortem analysis may be needed, back up the conflict log table before removing the subscription. ~ 4b. I also thought this is more readable when that last caution sentence was in a new <para>, but maybe just my opinion. ~~~ (value: all) 5. + <listitem> + <para> + <literal>all</literal>: Records to both the server log and the table. + </para> + </listitem> I felt that the description for 'all' should refer to those other destinations explicitly. SUGGESTION <literal>all</literal>: Records conflict details to both destinations <literal>log</literal> and <literal>table</literal>. ====== Kind Regards, Peter Smith. Fujitsu Australia
