Here are some very minor review comments for the patch v4-0001
======
Commit Message
1.
The other possibility was to apply the delay at the end of the parallel apply
transaction but that would cause issues related to resource bloat and
locks being
held for a long time.
~
The reply [1] for review comment #2 says that this was "slightly
reworded", but AFAICT nothing is changed here.
~~~
2.
Eariler versions were written by Euler Taveira, Takamichi Osumi, and
Kuroda Hayato
Typo: "Eariler"
======
doc/src/sgml/ref/create_subscription.sgml
3.
+ <para>
+ By default, the publisher sends changes as soon as possible. This
+ parameter allows the user to delay changes by given time period. If
+ the value is specified without units, it is taken as milliseconds.
+ The default is zero (no delay). See <xref
linkend="config-setting-names-values"/>
+ for details on the available valid time units.
+ </para>
"by given time period" --> "by the given time period"
======
src/backend/replication/pgoutput/pgoutput.c
4. parse_output_parameters
+ else if (strcmp(defel->defname, "min_send_delay") == 0)
+ {
+ unsigned long parsed;
+ char *endptr;
I think 'parsed' is a fairly meaningless variable name. How about
calling this variable something useful like 'delay_val' or
'min_send_delay_value', or something like those? Yes, I recognize that
you copied this from some existing code fragment, but IMO that doesn't
make it good.
======
src/backend/replication/walsender.c
5.
+ /* Sleep until we get reply from worker or we time out */
+ WalSndWait(WL_SOCKET_READABLE,
+ Min(timeout_sleeptime_ms, remaining_wait_time_ms),
+ WAIT_EVENT_WALSENDER_SEND_DELAY);
In my previous review [2] comment #14, I questioned if this comment
was correct. It looks like that was accidentally missed.
======
src/include/replication/logical.h
6.
+ /*
+ * The minimum delay, in milliseconds, by the publisher before sending
+ * COMMIT/PREPARE record
+ */
+ int32 min_send_delay;
The comment is missing a period.
------
[1] Kuroda-san replied to my review v3-0001.
https://www.postgresql.org/message-id/TYAPR01MB5866C6BCA4D9386D9C486033F5A59%40TYAPR01MB5866.jpnprd01.prod.outlook.com
[2] My previous review v3-0001.
https://www.postgresql.org/message-id/CAHut%2BPu6Y%2BBkYKg6MYGi2zGnx6imeK4QzxBVhpQoPMeDr1npnQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
