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

On Tue, 27 Dec 2022 at 14:59, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> Note that more than half of the modifications are done by Osumi-san.

1) This global variable can be removed as it is used only in
send_feedback which is called from maybe_delay_apply so we could pass
it as a function argument:
+ * delay, avoid having positions of the flushed and apply LSN overwritten by
+ * the latest LSN.
+ */
+static bool in_delaying_apply = false;
+static XLogRecPtr last_received = InvalidXLogRecPtr;
+

2) -1 gets converted to -1000

+int64
+interval2ms(const Interval *interval)
+{
+ int64 days;
+ int64 ms;
+ int64 result;
+
+ days = interval->month * INT64CONST(30);
+ days += interval->day;
+
+ /* Detect whether the value of interval can cause an overflow. */
+ if (pg_mul_s64_overflow(days, MSECS_PER_DAY, &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));
+
+ /* Adds portion time (in ms) to the previous result. */
+ ms = interval->time / INT64CONST(1000);
+ if (pg_add_s64_overflow(result, ms, &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("bigint out of range")));

create subscription sub7 connection 'dbname=regression host=localhost
port=5432' publication pub1 with (min_apply_delay = '-1');
ERROR: -1000 ms is outside the valid range for parameter "min_apply_delay"

3) This can be slightly reworded:
+ <para>
+ The length of time (ms) to delay the application of changes.
+ </para></entry>
to:
Delay applying the changes by a specified amount of time(ms).

4) maybe_delay_apply can be moved from apply_handle_stream_prepare to
apply_spooled_messages so that it is consistent with
maybe_start_skipping_changes:
@@ -1120,6 +1240,19 @@ apply_handle_stream_prepare(StringInfo s)

elog(DEBUG1, "received prepare for streamed transaction %u",
prepare_data.xid);

+ /*
+ * Should we delay the current prepared transaction?
+ *
+ * Although the delay is applied in BEGIN PREPARE messages, streamed
+ * prepared transactions apply the delay in a STREAM PREPARE message.
+ * That's ok because no changes have been applied yet
+ * (apply_spooled_messages() will do it). The STREAM START message does
+ * not contain a prepare time (it will be available when the in-progress
+ * prepared transaction finishes), hence, it was not possible to apply a
+ * delay at that time.
+ */
+ maybe_delay_apply(prepare_data.prepare_time);

That way the call from apply_handle_stream_commit can also be removed.

5) typo transfering should be transferring
+ publisher and the current time on the subscriber. Time
spent in logical
+ decoding and in transfering the transaction may reduce the
actual wait
+ time. If the system clocks on publisher and subscriber are not

6) feedbacks can be changed to feedback messages
+ * it's necessary to keep sending feedbacks during the delay from the worker
+ * process. Meanwhile, the feature delays the apply before starting the

7)
+ /*
+ * Suppress overwrites of flushed and writtten positions by the lastest
+ * LSN in send_feedback().
+ */

7a) typo writtten should be written

7b) lastest should latest

Regards,
Vignesh