Re: Conflict detection and logging in logical replication

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Jan Wieck <jan(at)wi3ck(dot)info>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Subject: Re: Conflict detection and logging in logical replication
Date: 2024-08-21 06:44:42
Message-ID: CAHut+PsJnOKqnyUwEhVqa+JAjdVGc1c1nc-qQGvSy=sh0uFizQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Here are some review comments for the v19-0001 docs patch.

The content seemed reasonable, but IMO it should be presented quite differently.

~~~~

1. Use sub-sections

I expect this logical replication "Conflicts" section is going to
evolve into something much bigger. Surely, it's not going to be one
humongous page of details, so it will be a section with lots of
subsections like all the other in Chapter 29.

IMO, you should be writing the docs in that kind of structure from the
beginning.

For example, I'm thinking something like below (this is just an
example - surely lots more subsections will be needed for this topic):

29.6 Conflicts
29.6.1. Conflict types
29.6.2. Logging format
29.6.3. Examples

Specifically, this v19-0001 patch information should be put into a
subsection like the 29.6.2 shown above.

~~~

2. Markup

+<screen>
+LOG: conflict detected on relation "schemaname.tablename":
conflict=<literal>conflict_type</literal>
+DETAIL: <literal>detailed explaination</literal>.
+<literal>Key</literal> (column_name, ...)=(column_name, ...);
<literal>existing local tuple</literal> (column_name,
...)=(column_name, ...); <literal>remote tuple</literal> (column_name,
...)=(column_name, ...); <literal>replica identity</literal>
(column_name, ...)=(column_name, ...).
+</screen>

IMO this should be using markup more like the SQL syntax references.
- e.g. I suggest <synopsis> instead of <screen>
- e.g. I suggest all the substitution parameters (e.g. detailed
explanation, conflict_type, column_name, ...) in the log should use
<replaceable class="parameter"> and use those markups again later in
these docs instead of <literal>

~

nit - typo /explaination/explanation/

~

nit - The amount of scrolling needed makes this LOG format too hard to
see. Try to wrap it better so it can fit without being so wide.

~~~

3. Restructure the list

+ <itemizedlist>
+ <listitem>

I suggest restructuring all this to use a nested list like:

LOG
- conflict_type
DETAIL
- detailed_explanation
- key
- existing_local_tuple
- remote_tuple
- replica_identity

Doing this means you can remove a great deal of the unnecessary junk
words like "of the first sentence in the DETAIL", and "sentence of the
DETAIL line" etc. The result will be much less text but much simpler
text too.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message zfmohz 2024-08-21 06:48:28 The json_table function returns an incorrect column type
Previous Message Heikki Linnakangas 2024-08-21 06:29:39 Re: configure failures on chipmunk