RE: Time delayed LR (WAS Re: logical replication restrictions) - Mailing list pgsql-hackers

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