RE: Time delayed LR (WAS Re: logical replication restrictions)

Dear Peter,

Thank you for reviewing! PSA new version.

> 1.
> Nitpick. The new text is jagged-looking. It should wrap at ~80 chars.

Addressed.

>
> 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..."

Changed.

> ======
> src/backend/replication/walsender.c
>
> 3. WalSndDelay
>
> + /* die if timeout was reached */
> + WalSndCheckTimeOut();
>
> Other nearby comments start uppercase, so this should too.

I just picked from other part and they have lowercase, but fixed.

> ======
> 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)"

Added.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED