Re: Synchronizing slots from primary to standby

Here are some review comments for v70-0001.

======
doc/src/sgml/protocol.sgml

1.
Related to this, please also review my other patch to the same docs
page protocol.sgml [1].

======
src/backend/replication/logical/tablesync.c

2.
  walrcv_create_slot(LogRepWorkerWalRcvConn,
     slotname, false /* permanent */ , false /* two_phase */ ,
+    false,
     CRS_USE_SNAPSHOT, origin_startpos);

I know it was previously mentioned in this thread that inline
parameter comments are unnecessary, but here they are already in the
existing code so shouldn't we do the same?

======
src/backend/replication/repl_gram.y

3.
+/* ALTER_REPLICATION_SLOT slot options */
+alter_replication_slot:
+ K_ALTER_REPLICATION_SLOT IDENT '(' generic_option_list ')'
+ {
+ AlterReplicationSlotCmd *cmd;
+ cmd = makeNode(AlterReplicationSlotCmd);
+ cmd->slotname = $2;
+ cmd->options = $4;
+ $$ = (Node *) cmd;
+ }
+ ;
+

IMO write that comment with parentheses, so it matches the code.

SUGGESTION
ALTER_REPLICATION_SLOT slot ( options )

======
[1] https://www.postgresql.org/message-id/CAHut%2BPtDWSmW8uiRJF1LfGQJikmo7V2jdysLuRmtsanNZc7fNw%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia