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

On Tue, 27 Dec 2022 at 14:59, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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