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

On Tuesday, January 3, 2023 8:22 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> Please find a few minor comments.
Thanks for your review !

> 1.
> + diffms = TimestampDifferenceMilliseconds(GetCurrentTimestamp(),
> +
>
> TimestampTzPlusMilliseconds(ts, MySubscription->minapplydelay)); on
> unix, above code looks unaligned (copied from unix)
>
> 2. same with:
> + interval = DatumGetIntervalP(DirectFunctionCall3(interval_in,
> +
>
> CStringGetDatum(val),
> +
>
> ObjectIdGetDatum(InvalidOid),
> +
>
> Int32GetDatum(-1)));
> perhaps due to tabs?
Those patches indentation look OK. I checked them
by pgindent and less command described in [1]. So, I didn't change those.

> 2. comment not clear:
> + * During the time delayed replication, avoid reporting the suspended
> + * latest LSN are already flushed and written, to the publisher.
You are right. I fixed this part to make it clearer.
Could you please check ?

> 3.
> + * Call send_feedback() to prevent the publisher from exiting by
> + * timeout during the delay, when wal_receiver_status_interval is
> + * available. The WALs for this delayed transaction is neither
> + * written nor flushed yet, Thus, we don't make the latest LSN
> + * overwrite those positions of the update message for this delay.
>
> yet, Thus, we --> yet, thus, we/ yet. Thus, we
Yeah, you are right. But, I have removed the last sentence, because the last one
explains some internals of send_feedback(). I judged that it would be awkward
to describe it in maybe_delay_apply(). Now this part has become concise.

> 4.
> + /* Adds portion time (in ms) to the previous result. */
> + ms = interval->time / INT64CONST(1000);
> Is interval->time always in micro-seconds here?
Yeah, it seems so. Some internal codes indicate it. Kindly have a look at functions
such as make_interval() and interval2itm().

Please have a look at the latest patch v12 in [2].

[1] - https://www.postgresql.org/docs/current/source-format.html
[2] - https://www.postgresql.org/message-id/TYCPR01MB837340F78F4A16F542589195EDFF9%40TYCPR01MB8373.jpnprd01.prod.outlook.com

Best Regards,
Takamichi Osumi