Hi Hou-San, here are a few trivial comments remaining for patch v6-0001.
======
General.
1.
There are multiple comments in this patch mentioning 'wal' which
probably should say 'WAL' (uppercase).
~~~
2.
There are multiple comments in this patch missing periods (.)
======
doc/src/sgml/protocol.sgml
3.
+ <term>Primary status update (B)</term>
+ <listitem>
+ <variablelist>
+ <varlistentry>
+ <term>Byte1('s')</term>
Currently, there are identifiers 's' for the "Primary status update"
message, and 'S' for the "Primary status request" message.
As mentioned in the previous review ([1] #5b) I preferred it to be the
other way around:
'S' = status from primary
's' = request status from primary
Of course, it doesn't make any difference, but "S" seems more
important than "s", so therefore "S" being the main msg and coming
from the *primary* seemed more natural to me.
~~~
4.
+ <varlistentry id="protocol-replication-standby-wal-status-request">
+ <term>Primary status request (F)</term>
Is it better to call this slightly differently to emphasise this is
only the request?
/Primary status request/Request primary status update/
======
src/backend/replication/logical/worker.c
5.
+ * Retaining the dead tuples for this period is sufficient for ensuring
+ * eventual consistency using last-update-wins strategy, as dead tuples are
+ * useful for detecting conflicts only during the application of concurrent
As mentioned in review [1] #9, this is still called "last-update-wins
strategy" here in the comment, but was called the "last update win
strategy" strategy in the commit message. Those terms should be the
same -- e.g. the 'last-update-wins' strategy.
======
[1]
https://www.postgresql.org/message-id/CAHut%2BPs3sgXh2%3DrHDaqjU%3Dp28CK5rCgCLJZgPByc6Tr7_P2imw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
