| From: | Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com> |
|---|---|
| To: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org, masao(dot)fujii(at)gmail(dot)com, li(dot)evan(dot)chao(at)gmail(dot)com, shinya11(dot)kato(at)gmail(dot)com |
| Subject: | Re: remote_apply commit hangs when wal_receiver_status_interval = 0 |
| Date: | 2026-10-06 12:30:38 |
| Message-ID: | CA+vB=AHrOvP4vRQPkW_n2UsUmu8medC4Pv4sV9xw8xji+5Wu+w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
Thanks for the review. You're right, the test had a race.
If the INSERT hadn't started yet, the backend could show up as idle
and the check would pass without the commit ever waiting.
I've changed the test as you suggested. It now does the following:
1. Pauses replay on the standby with pg_wal_replay_pause().
2. Starts the remote_apply INSERT in the background session.
3. Waits until the backend shows wait_event = 'SyncRep'. This is a new
assertion.
4. Resumes replay with pg_wal_replay_resume().
5. Waits until the backend is idle, which is the original check.
The walreceiver.c change is the same as in v1. Only the test changed.
The v2 patch is attached.
Regards,
Vaibhav
On Wed, Sep 30, 2026 at 5:38 PM Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>
wrote:
> Hi,
>
> On Wed, Sep 30, 2026 at 1:52 PM Vaibhav Dalvi
> <vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
> >
> > Hi hackers,
> >
> > On master, a commit with synchronous_commit = remote_apply does not
> > return when the standby has wal_receiver_status_interval = 0. The
> > standby applies the commit, but it never sends the apply reply to the
> > primary, so the backend keeps waiting in SyncRep. With default
> > timeouts it waits about 30 seconds, until the walsender keepalive
> > forces a reply. With wal_sender_timeout = 0 and wal_receiver_timeout
> > = 0 it waits forever.
> >
> > The docs for wal_receiver_status_interval say that updates are sent
> > while ignoring this parameter "when synchronous_commit is set to
> > remote_apply", so I think this is a regression.
> >
>
> This indeed looks like a regression, the fix looks correct but I have
> one small comment to share for the following change:
>
> +my $bg = $primary->background_psql('postgres', on_error_stop => 0);
> +my $pid = $bg->query_safe('SELECT pg_backend_pid()');
> +$bg->query_safe('SET synchronous_commit = remote_apply');
> +$bg->query_until(qr/start/, "\\echo start\nINSERT INTO t VALUES (1);\n");
> +
> +ok( $primary->poll_query_until(
> + 'postgres',
> + "SELECT state = 'idle' FROM pg_stat_activity WHERE pid = $pid"),
> + 'remote_apply commit completes with wal_receiver_status_interval = 0');
> +is($standby->safe_psql('postgres', 'SELECT count(*) FROM t'),
> + '1', 'remote_apply commit is visible on standby');
>
> There is one small race here - it is quite possible that before the
> INSERT starts asynchronously, the pg_stat_activity says the backend is
> in the idle state, making the test case false positive.
>
> So the right thing would be to maybe pause the replay on the standby,
> start the INSERT, confirm that the backend enters the SyncRep wait,
> resume replay, and then verify that the backend becomes idle.
>
> $standby->safe_psql('postgres', 'SELECT pg_wal_replay_pause()');
>
> $bg->query_until(qr/start/, "\\echo start\nINSERT INTO t VALUES (1);\n");
>
> ok($primary->poll_query_until(
> 'postgres',
> "SELECT wait_event = 'SyncRep'
> FROM pg_stat_activity WHERE pid = $pid"),
> 'remote_apply commit waits for standby replay');
>
> $standby->safe_psql('postgres', 'SELECT pg_wal_replay_resume()');
>
> ok($primary->poll_query_until(
> 'postgres',
> "SELECT state = 'idle'
> FROM pg_stat_activity WHERE pid = $pid"),
> 'remote_apply commit completes with wal_receiver_status_interval = 0');
>
> --
> With Regards,
> Ashutosh Sharma.
>
| Attachment | Content-Type | Size |
|---|---|---|
| v2-walreceiver-send-apply-reply-even-when-wal_receiver_status_interval-disabled.patch | application/octet-stream | 6.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andrey Borodin | 2026-10-06 13:09:38 | Re: Compression of bigger WAL records |
| Previous Message | Andrey Borodin | 2026-10-06 12:28:02 | Re: amcheck: detect corruption from the recent snapshot-export bug |