Here are some review comments for v9-0001, but these are only very trivial.
====== Commit Message 1. Nitpick. The new text is jagged-looking. It should wrap at ~80 chars. ~~~ 2. 2. Another reason is for that parallel streaming, the transaction will be opened immediately by the parallel apply worker. Therefore, if the walsender is delayed in sending the final record of the transaction, the parallel apply worker must wait to receive it with an open transaction. This would result in the locks acquired during the transaction not being released until the min_send_delay has elapsed. ~ The text already said there are "two reasons", and already this is numbered as reason 2. So it doesn't need to keep saying "Another reason" here. "Another reason is for that parallel streaming" --> "For parallel streaming..." ====== src/backend/replication/walsender.c 3. WalSndDelay + /* die if timeout was reached */ + WalSndCheckTimeOut(); Other nearby comments start uppercase, so this should too. ====== src/include/replication/walreceiver.h 4. WalRcvStreamOptions @@ -187,6 +187,7 @@ typedef struct * prepare time */ char *origin; /* Only publish data originating from the * specified origin */ + int32 min_send_delay; /* The minimum send delay */ } logical; } proto; } WalRcvStreamOptions; ~ Should that comment mention the units are "(ms)" ------ Kind Regards, Peter Smith. Fujitsu Australia
