Re: Conflict detection and logging in logical replication
Peter Smith <smithpb2250@gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
shveta malik <shveta.malik@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
Nisha Moond <nisha.moond412@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>,
Dilip Kumar <dilipbalaut@gmail.com>, Jan Wieck <jan@wi3ck.info>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2024-08-21T06:44:42Z
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
Commits
-
Rename the conflict types for the origin differ cases.
- 640178c92e3f 18.0 landed
-
Doc: explain the log format of logical replication conflicts.
- edcb71258504 18.0 landed
-
Log the conflicts while applying changes in logical replication.
- 9758174e2e5c 18.0 landed
-
Avoid duplicate table scans for cross-partition updates during logical replication.
- a67da49e1d98 18.0 cited