| From: | Kirill Reshke <reshkekirill(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
| Cc: | Grigory Smolkin <smallkeen(at)gmail(dot)com>, Jaroslav Novikov <njrslv(at)yandex-team(dot)ru>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Venkata Balaji N <nag1010(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Borodin Vladimir <root(at)simply(dot)name>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, nkak(at)vmware(dot)com, Roman Khapov <rkhapov(at)yandex-team(dot)ru>, ShirishaRao(at)vmware(dot)com |
| Subject: | Re: Streaming replication and WAL archive interactions |
| Date: | 2026-07-30 14:44:13 |
| Message-ID: | CALdSSPiVbHM9+XettYcOHDJ6KTLzjNEhdURwXwfzyMV-dtL1Gg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hii
On Wed, 22 Jul 2026 at 16:19, Andrey Borodin <x4mmm(at)yandex-team(dot)ru> wrote:
> 1. The unsolicited 'a' message breaks other physical protocol
> clients. WalSndArchivalReport() sends the report to any physical
> walsender in CATCHUP or STREAMING state, excluding only temporary
> slots. But pg_receivewal speaks the same protocol. I tried it against
> a shared-mode primary (with a permanent slot): receivelog.c does not
> know the message and drops the connection with
>
> pg_receivewal: error: unrecognized streaming header: "a"
>
> With --no-loop it just exits. With the default retry loop it
> reconnects every 5 seconds, receives the next report right away (the
> new walsender's report timer starts at zero), errors out again, and
> makes no progress at all - I watched it restart from the same LSN
> three times in a row. Existing tests do not catch this because
> nothing runs pg_receivewal against a shared-mode primary.
> pg_basebackup -S <slot> --wal-method=stream should hit the same path.
>
> I think the primary recommendation here is that our own tools should
> digest such messages gracefully instead of dropping the connection:
> teach receivelog.c to skip messages it does not understand (or at
> least this one). The physical protocol will keep growing new
> informational messages, and erroring out on every unknown byte turns
> each addition into a compatibility hazard.
>
> That fixes in-core tools, but third-party tools speaking the physical
> protocol are still affected. Note that the PrimaryStatusUpdate ('s')
> message added in PG 18 is only ever sent in response to an explicit
> request ('p') from the standby, I suppose for exactly this reason.
> So making the reports opt-in - a new option to START_REPLICATION, or
> a request message similar to PrimaryStatusRequest - may still be
> worth considering on top of that, rather than the walsender guessing
> by slot persistence.
> 7. An operational concern from running this in production. In shared
> mode a standby retains WAL without any bound while the archive
> storage is down or the primary is not archiving. archive_mode is
> PGC_POSTMASTER, so there is no runtime escape hatch: the standby
> fills its disk and fails over, then the next standby repeats the
> exercise. Downstream we ended up adding a SIGHUP GUC that disables
> the retention on a standby, trading a gap in the archive for cluster
> availability. For upstream, maybe something in the spirit of
> max_slot_wal_keep_size would fit: a cap after which .ready segments
> are recycled with a WARNING. Or, at the very least, a documentation
> paragraph describing this failure mode. WDYT?
>
For this two:
> 2. The new TAP tests are not added to src/test/recovery/meson.build.
> Under autotools prove picks up t/*.pl by glob, so they run, but in
> meson builds (which is what most of CI runs) they are silently
> skipped.
>
> 3. archive_status_report_interval is missing from
> postgresql.conf.sample, and the GUC is not marked GUC_NOT_IN_SAMPLE,
> so src/test/modules/test_misc/t/003_check_guc.pl should fail. There
> is also a stray whitespace-only line added to guc_parameters.dat.
fixed
> While at it: do we want 0 to mean "disabled"? Not proposing it as a
> must.
> 4. protocol.sgml: the new varlistentry reuses the existing id
> "protocol-replication-primary-status-update", so the id is now
> duplicated and the docs build fails. It needs its own id, something
> like protocol-replication-archive-status-report.
fixed
> 5. v7-0002 leaves make check red: pg_stat_wal_receiver gained a
> column, but src/test/regress/expected/rules.out is not updated. The
> commit message says "Bumps catversion", but catversion.h is not
> touched. The new column is also not described in monitoring.sgml.
fixed
> The assertions 0002 adds to the 055 test are also racy, and a 32-bit
> CI run of mine hit it. The test captures
> pg_walfile_name(pg_current_wal_lsn()), switches WAL and then
> immediately expects the standby's primary_last_archived to equal the
> captured name. Nothing guarantees the standby has received the
> report covering that segment yet (they are sent asynchronously every
> archive_status_report_interval), so on a slow machine the view still
> holds the previous report. The archived_count poll on the primary
> side is racy too: the counter can be bumped by archiving an older
> pending segment. Both need to poll for the expected state - primary
> for last_archived_wal >= the captured name, standby likewise for the
> view - before checking the .done file.
> 6. The 055 test fails on Windows. It sets archive_command to a Unix
> cp invocation with a raw tempdir path interpolated into
> postgresql.conf. On Windows the path contains backslashes, and the
> GUC parser processes backslash escapes inside quoted configuration
> values, so the path gets mangled (\t becomes a tab and so on) and
> archiving never succeeds; the test dies waiting for
> archived_count > 0. The archive command needs the same treatment as
> PostgreSQL::Test::Cluster::enable_archiving: copy on Windows, with
> backslashes doubled. We cannot just use enable_archiving because all
> nodes must share one archive directory - that is the point of the
> feature. 056 has the same latent problem in its $good_command (copy,
> but backslashes not doubled).
>
should be fixed now with injection-point based wait
> Best regards, Andrey Borodin.
I have added a third patch to this series, which changes how this
feature works. If this behavioural change is ok i'm planning to maybe
squash the whole thing into 1 or two patches.
So, our major concern and what's Andrey point 7 is about possible
configuration hazards. There are several setups in which standby will
not
receive archive status reports from master, leading to disk space
exhaustion (because we will never delete WAL segments from standby).
The simplest case is when master's archive mode is 'off' and
standby's archive mode is 'shared'. In this case, the standby will
never get any archival report from the master. v8-0003 makes
additional replication-level GUC,
which is used by standby to request archival reports from primary. if
primary cannot ever serve this, connection is rejected.
Another case is with archive mode on->on->shared replication setup. In
this case, in-between standby cannot relay any archival reports from
master,because it wasn't configured to. So, to counter this, I do
actually activate the archiver on standby (in contrast to what v7-0001
does). On standby, we try to avoid re-archiving WAL's which master
has already archived, so we wait for 3 *
archival_status_report_interval ms before starting any archiving.
Note that WAL archiving in this setup will be from both primary and
cascade standby
Here is also patch series description:
== Patch decomposition ==
The series is split into N patches:
0001 - Add archive_mode=shared for coordinated WAL archiving
what it does: new enum mode, protocol parameter,
walsender/walreceiver exchange, basic archiver logic
0002 - Add primary_last_archived column to pg_stat_wal_receiver
purpose: observability - to see how far the primary has archived
0003 - Guard against configuration hazards in archive_mode=shared
FATAL when requesting archive_status_report_interval if archiving
is not active on the upstream; fallback archiving on standby; TAP
tests 055/056/057
Note that I rebased this many times, so v8-0001 or v8-0001 + v8-0002
only may not actually work.
--
Best regards,
Kirill Reshke
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0003-Guard-against-configuration-hazards-in-archive_mo.patch | application/octet-stream | 30.9 KB |
| v8-0002-Add-primary_last_archived-column-to-pg_stat_wal_r.patch | application/octet-stream | 7.8 KB |
| v8-0001-Add-archive_mode-shared-for-coordinated-WAL-archi.patch | application/octet-stream | 65.4 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Marcos Pegoraro | 2026-07-30 14:37:00 | Correct some doc items due to the REPACK |