Horton

RE: Logical replication timeout problem

Wei Wang (Fujitsu) <wangw.fnst@fujitsu.com>

View thread View on postgr.es
From: "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "kuroda.hayato@fujitsu.com" <kuroda.hayato@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, Fabrice Chapuis <fabrice636861@gmail.com>, Simon Riggs <simon.riggs@enterprisedb.com>, Petr Jelinek <petr.jelinek@enterprisedb.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Ajin Cherian <itsajin@gmail.com>
Date: 2022-03-09T02:26:14Z
Lists: pgsql-hackers 
On Tue, Mar 8, 2022 at 3:52 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> I've looked at the patch and have a question:
Thanks for your review and comments.

> +void
> +SendKeepaliveIfNecessary(LogicalDecodingContext *ctx, bool skipped) {
> +        static int skipped_changes_count = 0;
> +
> +        /*
> +         * skipped_changes_count is reset when processing changes that do not
> +         * need to be skipped.
> +         */
> +        if (!skipped)
> +        {
> +                skipped_changes_count = 0;
> +                return;
> +        }
> +
> +        /*
> +         * After continuously skipping SKIPPED_CHANGES_THRESHOLD
> changes, try to send a
> +         * keepalive message.
> +         */
> +        #define SKIPPED_CHANGES_THRESHOLD 10000
> +
> +        if (++skipped_changes_count >= SKIPPED_CHANGES_THRESHOLD)
> +        {
> +                /* Try to send a keepalive message. */
> +                OutputPluginUpdateProgress(ctx, true);
> +
> +                /* After trying to send a keepalive message, reset the flag. */
> +                skipped_changes_count = 0;
> +        }
> +}
> 
> Since we send a keepalive after continuously skipping 10000 changes, the
> originally reported issue can still occur if skipping 10000 changes took more than
> the timeout and the walsender didn't send any change while that, is that right?
Yes, theoretically so.
But after testing, I think this value should be conservative enough not to reproduce
this bug.
After the previous discussion[1], it is currently considered that it is better
to directly set a conservative threshold than to calculate the threshold based
on wal_sender_timeout.

[1] - https://www.postgresql.org/message-id/OS3PR01MB6275FEB9F83081F1C87539B99E019%40OS3PR01MB6275.jpnprd01.prod.outlook.com

Regards,
Wang wei

Commits

  1. Fix the logical replication timeout during large DDLs.

  2. Fix the logical replication timeout during large transactions.

  3. Rethink the delay-checkpoint-end mechanism in the back-branches.

  4. Revert "Logical decoding of sequences"

  5. Skip empty transactions for logical replication.

  6. Allow specifying column lists for logical replication

  7. Add decoding of sequences to built-in replication

  8. Fix ABI break introduced by commit 4daa140a2f.

  9. Lag tracking for logical replication