| From: | Vaibhav Dalvi <vaibhav(dot)dalvi(at)enterprisedb(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, 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-07 04:44:37 |
| Message-ID: | CA+vB=AF6Xe5MZ=B6N=NdnpfbyEVZMgftE7+VfPtbJLAja1s23g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Fujii-san,
Thanks for the review.
Attached v3. Only the test changed.
> How about configuring sync replication at startup instead?
Done. synchronous_standby_names = '*' and synchronous_commit = local
are now set in the primary's initial configuration, so there is no reload
and
no window where the standby appears synchronous before the backends are
aware of it. Only the test session uses remote_apply.
> Also, do we really need to pause replay?
No, it is not needed. Thank you for pointing that out. The test now runs the
INSERT with query_safe() and then checks that the row is visible on the
standby.
Without the fix the INSERT remains blocked, and the test fails on timeout.
I ran it a few times with the fix and it passes. With the walreceiver.c
change
reverted, it fails on a timeout.
Thanks again for your time and for the review.
Regards,
Vaibhav
On Tue, Oct 6, 2026 at 9:11 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Tue, Oct 6, 2026 at 9:31 PM Vaibhav Dalvi
> <vaibhav(dot)dalvi(at)enterprisedb(dot)com> wrote:
> > The walreceiver.c change is the same as in v1. Only the test changed.
> > The v2 patch is attached.
>
> Thanks for the patch! The walreceiver change looks correct to me. I have a
> couple of comments on the test.
>
> +$primary->safe_psql('postgres',
> + "ALTER SYSTEM SET synchronous_standby_names = '*'");
> +$primary->reload;
> +$primary->poll_query_until(
> + 'postgres',
> + "SELECT count(*) = 1 FROM pg_stat_replication
> + WHERE state = 'streaming' AND sync_state = 'sync'
> + AND flush_lsn IS NOT NULL"
>
> There seems to be another race in the test. After the configuration reload,
> the checkpointer needs to read the new synchronous_standby_names setting
> and update the shared SYNC_STANDBY_DEFINED flag before backends can wait
> for sync replication. But, walsender could read the new setting first,
> causing sync_state to show sync before the checkpointer updates that flag.
> If the INSERT commits during this window, it could skip the SyncRep wait
> and cause the test to fail.
>
> How about configuring sync replication at startup instead?
>
> $primary->append_conf(
> 'postgresql.conf', qq(
> wal_sender_timeout = 0
> synchronous_standby_names = '*'
> synchronous_commit = local
> ));
>
> Using local allows setup to proceed before the standby is available. Only
> the test session needs to switch to remote_apply:
>
> $bg->query_safe('SET synchronous_commit = remote_apply');
>
>
> Also, do we really need to pause replay? Isn't it sufficient to wait
> directly for the INSERT to complete using query_safe() in a background psql
> session with a timeout, and then check that the row is visible on the
> standby?
> Without the fix, the INSERT would remain blocked and the test would fail on
> timeout. With the fix, it would complete and the test would pass. This
> would
> simplify the test.
>
> Regards,
>
> --
> Fujii Masao
>
| Attachment | Content-Type | Size |
|---|---|---|
| v3-walreceiver-send-apply-reply-even-when-wal_receiver_status_interval-disabled.patch | application/octet-stream | 6.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kirill Reshke | 2026-10-07 04:53:38 | Re: pg_dump/restore failure (dependency?) on BF serinus |
| Previous Message | shveta malik | 2026-10-07 04:03:24 | Re: Persist slot invalidations before publishing them |