From e39a20edd589384f2a64913403cc39121726e238 Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Wed, 22 Jul 2026 12:35:04 +0500 Subject: [PATCH v7-AB-edited 3/6] fixup! Add archive_mode=shared for coordinated WAL archiving Assorted fixes to squash into v7-0001: * Register 055 and 056 TAP tests in src/test/recovery/meson.build; previously they only ran under autotools (prove glob) and were silently skipped in meson builds. * Add archive_status_report_interval to postgresql.conf.sample and mention "shared" in the archive_mode sample line (003_check_guc.pl fails otherwise). * Fix duplicate SGML id in protocol.sgml: the new message entry reused protocol-replication-primary-status-update, breaking the docs build. Rename to protocol-replication-archive-status-report. * Introduce XLogArchivingShared() macro (with the usual wal_level AssertMacro) and use it instead of direct comparisons with ARCHIVE_MODE_SHARED. * WalSndArchivalReport(): fetch pgstat archiver stats only on a primary; on a cascading standby the value comes from shared memory and the fetch was wasted work. Use strlcpy instead of memcpy. Document the skipped-report case when the most recently archived file is not a WAL segment (e.g. a timeline history file). * Walreceiver: pass the reported filename to ProcessArchivalReport() as an argument instead of re-reading shared memory without the spinlock; use strlcpy to avoid copying uninitialized stack bytes into shared memory; drop dead code before ereport(ERROR). * pgarch.c: repair accidentally dedented block in pgarch_ArchiverCopyLoop, drop redundant initialization after MemSet, fix trailing/extra whitespace. Add PGDLLIMPORT to the PgArch extern. * Test nits: copyright year in 056, stray tab-only line and comment wording in 055. --- doc/src/sgml/protocol.sgml | 2 +- src/backend/access/transam/xlogarchive.c | 2 +- src/backend/postmaster/pgarch.c | 14 ++---- src/backend/replication/walreceiver.c | 23 +++++---- src/backend/replication/walsender.c | 49 ++++++++++++------- src/backend/utils/misc/guc_parameters.dat | 2 +- src/backend/utils/misc/postgresql.conf.sample | 4 +- src/include/access/xlog.h | 3 ++ src/include/postmaster/pgarch.h | 4 +- src/test/recovery/meson.build | 2 + src/test/recovery/t/055_archive_shared.pl | 8 +-- .../t/056_archive_shared_checkpoint.pl | 3 +- 12 files changed, 64 insertions(+), 52 deletions(-) diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index bf97a411305..e21bd59cdd8 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -2840,7 +2840,7 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" - + WAL archival report message (B) diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c index 62360cc4864..3a492755ab8 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -582,7 +582,7 @@ XLogArchiveCheckDone(const char *xlog) * through to the .done/.ready check below so that checkpoint cannot * delete a segment whose .ready file has not yet become .done. */ - if (!XLogArchivingAlways() && !(XLogArchiveMode == ARCHIVE_MODE_SHARED) && + if (!XLogArchivingAlways() && !XLogArchivingShared() && GetRecoveryState() == RECOVERY_STATE_ARCHIVE) return true; diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 38900b3d004..0d54bb8ecb6 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -83,7 +83,6 @@ */ #define NUM_FILES_PER_DIRECTORY_SCAN 64 - char *XLogArchiveLibrary = ""; char *arch_module_check_errdetail_string; @@ -170,10 +169,7 @@ PgArchShmemInit(void *arg) MemSet(PgArch, 0, sizeof(PgArchData)); PgArch->pgprocno = INVALID_PROC_NUMBER; pg_atomic_init_u32(&PgArch->force_dir_scan, 0); - - PgArch->primary_last_archived[0] = '\0'; - - SpinLockInit(&PgArch->lock); + SpinLockInit(&PgArch->lock); } /* @@ -469,10 +465,10 @@ pgarch_ArchiverCopyLoop(void) continue; } - if (pgarch_archiveXlog(xlog)) - { - /* successful */ - pgarch_archiveDone(xlog); + if (pgarch_archiveXlog(xlog)) + { + /* successful */ + pgarch_archiveDone(xlog); /* * Tell the cumulative stats system about the WAL file that we diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index b115703f0da..03a045163a9 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -158,7 +158,7 @@ static void XLogWalRcvClose(XLogRecPtr recptr, TimeLineID tli); static void XLogWalRcvSendReply(bool force, bool requestReply, bool checkApply); static void XLogWalRcvSendHSFeedback(bool immed); static void ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime); -static void ProcessArchivalReport(void); +static void ProcessArchivalReport(const char *primary_last_archived); static void WalRcvComputeNextWakeup(WalRcvWakeupReason reason, TimestampTz now); @@ -918,7 +918,7 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) if (len >= sizeof(primary_last_archived)) ereport(ERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), - errmsg_internal("invalid archival report message with length %d, expected at most %ld", + errmsg_internal("invalid archival report message with length %d, expected at most %zu", (int) len, sizeof(primary_last_archived)))); memcpy(primary_last_archived, buf, len); @@ -926,18 +926,20 @@ XLogWalRcvProcessMsg(unsigned char type, char *buf, Size len, TimeLineID tli) /* Verify it contains only valid characters */ if (!IsXLogFileName(primary_last_archived)) - { - primary_last_archived[0] = '\0'; ereport(ERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg_internal("unexpected character in primary's last archived filename"))); - } + /* + * Publish the report for our own walsenders (cascading + * replication) and for pg_stat_wal_receiver. + */ SpinLockAcquire(&PgArch->lock); - memcpy(PgArch->primary_last_archived, primary_last_archived, sizeof(PgArch->primary_last_archived)); + strlcpy(PgArch->primary_last_archived, primary_last_archived, + sizeof(PgArch->primary_last_archived)); SpinLockRelease(&PgArch->lock); - ProcessArchivalReport(); + ProcessArchivalReport(primary_last_archived); break; } default: @@ -1154,7 +1156,7 @@ XLogWalRcvClose(XLogRecPtr recptr, TimeLineID tli) { XLogArchiveNotify(xlogfname); } - else if (XLogArchiveMode == ARCHIVE_MODE_SHARED) + else if (XLogArchivingShared()) { /* * In shared mode, check if this segment is already archived on primary. @@ -1378,16 +1380,13 @@ XLogWalRcvSendHSFeedback(bool immed) * timeline as .done if they're <= the reported segment. */ static void -ProcessArchivalReport(void) +ProcessArchivalReport(const char *primary_last_archived) { TimeLineID reported_tli; XLogSegNo reported_segno; char status_path[MAXPGPATH]; bool use_direct_check = false; XLogSegNo start_segno; - char *primary_last_archived; - - primary_last_archived = PgArch->primary_last_archived; elog(DEBUG2, "received archival report from primary: %s", primary_last_archived); diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 744777b745e..04c2aef4870 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2864,12 +2864,11 @@ ProcessStandbyHSFeedbackMessage(void) static void WalSndArchivalReport(void) { - PgStat_ArchiverStats *archiver_stats; TimestampTz now; char last_archived[MAX_XFN_CHARS + 1]; /* Only send reports when archive_mode=shared */ - if (XLogArchiveMode != ARCHIVE_MODE_SHARED) + if (!XLogArchivingShared()) return; /* Only send reports during physical streaming replication, not during backup */ @@ -2898,28 +2897,36 @@ WalSndArchivalReport(void) return; last_archival_report_timestamp = now; - /* - * Get archiver statistics. The pgstat snapshot is cached per-session and - * is only invalidated at transaction boundaries. The walsender runs - * without transaction boundaries, so we must clear the snapshot explicitly - * to avoid reading stale data (e.g. last_archived_wal stuck at its initial - * empty value even after the archiver has archived new segments). - */ - pgstat_clear_snapshot(); - archiver_stats = pgstat_fetch_stat_archiver(); - if (archiver_stats == NULL) - return; - if (RecoveryInProgress()) { + /* + * On a cascading standby, relay the report received from our own + * upstream by the walreceiver. + */ SpinLockAcquire(&PgArch->lock); - memcpy(last_archived, PgArch->primary_last_archived, sizeof(last_archived)); + strlcpy(last_archived, PgArch->primary_last_archived, + sizeof(last_archived)); SpinLockRelease(&PgArch->lock); } else - memcpy(last_archived, archiver_stats->last_archived_wal, sizeof(last_archived)); - + { + PgStat_ArchiverStats *archiver_stats; + + /* + * Get archiver statistics. The pgstat snapshot is cached per-session + * and is only invalidated at transaction boundaries. The walsender + * runs without transaction boundaries, so we must clear the snapshot + * explicitly to avoid reading stale data (e.g. last_archived_wal + * stuck at its initial empty value even after the archiver has + * archived new segments). + */ + pgstat_clear_snapshot(); + archiver_stats = pgstat_fetch_stat_archiver(); + strlcpy(last_archived, archiver_stats->last_archived_wal, + sizeof(last_archived)); + } + /* * Only send a report if the last archived WAL has changed. This is both * an optimization and ensures we don't send empty reports on startup. @@ -2927,7 +2934,13 @@ WalSndArchivalReport(void) if (strcmp(last_archived, last_archived_report_wal) == 0) return; - /* Only send reports for WAL segments, not backup history files or other archived files */ + /* + * Only send reports for WAL segments, not backup history files or other + * archived files. Note that if the most recently archived file is not a + * WAL segment (e.g. a timeline history file), the report is skipped + * entirely; the segments archived before it will be reported once the + * next regular WAL segment is archived. + */ if (!IsXLogFileName(last_archived)) return; diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index b36cab3126f..949f73d7a4d 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -94,7 +94,7 @@ boot_val => 'ARCHIVE_MODE_OFF', options => 'archive_mode_options', }, - + { name => 'archive_status_report_interval', type => 'int', context => 'PGC_SIGHUP', group => 'WAL_ARCHIVING', short_desc => 'Sets the amount of time between consecutive WAL archive status reports.', flags => 'GUC_UNIT_MS', diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 7958653077b..1ea754d6946 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -291,7 +291,7 @@ # - Archiving - -#archive_mode = off # enables archiving; off, on, or always +#archive_mode = off # enables archiving; off, on, shared, or always # (change requires restart) #archive_library = '' # library to use to archive a WAL file # (empty string indicates archive_command should @@ -302,6 +302,8 @@ # e.g. 'test ! -f "/mnt/server/archivedir/%f" && cp "%p" "/mnt/server/archivedir/%f"' #archive_timeout = 0 # force a WAL file switch after this # number of seconds; 0 disables +#archive_status_report_interval = 10s # time between archive status reports + # sent to standbys in archive_mode=shared # - Archive Recovery - diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 2aeb8ce14b5..e71606e60f0 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -106,6 +106,9 @@ extern PGDLLIMPORT bool XLogLogicalInfo; /* Is WAL archiving enabled always (even during recovery)? */ #define XLogArchivingAlways() \ (AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_REPLICA), XLogArchiveMode == ARCHIVE_MODE_ALWAYS) +/* Is WAL archiving coordinated between primary and standby? */ +#define XLogArchivingShared() \ + (AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_REPLICA), XLogArchiveMode == ARCHIVE_MODE_SHARED) /* * Is WAL-logging necessary for archival or log-shipping, or can we skip diff --git a/src/include/postmaster/pgarch.h b/src/include/postmaster/pgarch.h index 1d8571fc5a5..790e3ed4c70 100644 --- a/src/include/postmaster/pgarch.h +++ b/src/include/postmaster/pgarch.h @@ -51,8 +51,6 @@ typedef struct PgArchData pg_atomic_uint32 force_dir_scan; } PgArchData; - -extern PgArchData *PgArch; - +extern PGDLLIMPORT PgArchData *PgArch; #endif /* _PGARCH_H */ diff --git a/src/test/recovery/meson.build b/src/test/recovery/meson.build index ad0d85f4189..7e0c6ba67cc 100644 --- a/src/test/recovery/meson.build +++ b/src/test/recovery/meson.build @@ -63,6 +63,8 @@ tests += { 't/052_checkpoint_segment_missing.pl', 't/053_standby_login_event_trigger.pl', 't/054_unlogged_sequence_promotion.pl', + 't/055_archive_shared.pl', + 't/056_archive_shared_checkpoint.pl', ], }, } diff --git a/src/test/recovery/t/055_archive_shared.pl b/src/test/recovery/t/055_archive_shared.pl index f3f1e13c6de..f86594893c0 100644 --- a/src/test/recovery/t/055_archive_shared.pl +++ b/src/test/recovery/t/055_archive_shared.pl @@ -112,11 +112,11 @@ for (my $i = 0; $i < $PostgreSQL::Test::Utils::timeout_default; $i++) last if $done_count > 0; sleep(1); } - + ok($done_count > 0, "standby marked segments as .done after primary's archival report"); note("Standby has $done_count .done files"); -# The primary_last_archived done status for WAL file itself must also be present. +# The .done status file for the last-archived WAL segment must also be present. ok(-f "$standby_archive_status/$primary_last_archived.done", "WAL segment $primary_last_archived done file exists in standby pg_wal/archive_status"); @@ -191,9 +191,9 @@ ok( -f "$cascade_data/$walfile_done", # Before promotion, verify archiver is not running on standby (shared mode during recovery) # In shared mode, the standby's archiver should not be archiving during recovery -my $archived_before = $standby->safe_psql('postgres', +my $archived_before = $standby->safe_psql('postgres', "SELECT archived_count FROM pg_stat_archiver"); -is($archived_before, '0', +is($archived_before, '0', "archiver not active on standby before promotion (archived_count=0)"); # Verify standby is still in recovery before promoting diff --git a/src/test/recovery/t/056_archive_shared_checkpoint.pl b/src/test/recovery/t/056_archive_shared_checkpoint.pl index 0b840995452..8083ee99513 100644 --- a/src/test/recovery/t/056_archive_shared_checkpoint.pl +++ b/src/test/recovery/t/056_archive_shared_checkpoint.pl @@ -1,4 +1,4 @@ -# Copyright (c) 2025, PostgreSQL Global Development Group +# Copyright (c) 2026, PostgreSQL Global Development Group # Tests for archive_mode=shared correctness on standbys: # @@ -212,4 +212,3 @@ is(scalar(@still_missing), 0, "WAL segments were not lost while waiting for archival reports"); done_testing(); - -- 2.50.1 (Apple Git-155)