From d1f7dd476f7b362027508c885b80839875526cc7 Mon Sep 17 00:00:00 2001
From: Joao Foltran <joao@foltrandba.com>
Date: Mon, 15 Dec 2025 15:21:35 -0300
Subject: [PATCH v1] Revalidate physical slot automatically after its been lost due to wal_removed and standby has caught up.

Commit f41d8468 introduced an ERROR when trying to acquire an
invalidated replication slot. While this is correct for logical
replication slots (which cannot safely recover after invalidation), it
breaks possible recovery workflows for physical replication slots.

Physical replication slots can recover if the required WAL becomes
available through restore_command.
In PostgreSQL 17 and earlier, invalidated physical slots could be
reacquired and streaming would resume once the standby caught up via
archive recovery, although they wouldn't provide any security anymore (wal retention).

This commit does:
- Allowing physical slots to be acquired when invalidated and revalidated when standby recovered.
- Keeping the ERROR for logical slots (which cannot safely recover)

This maintains safety guarantees for logical replication while allowing
physical replication to recover in some operational scenarios.

Reported-by: Joao Foltran <joao@foltrandba.com>
---
 src/backend/replication/slot.c                |  22 +-
 src/backend/replication/walsender.c           |  22 ++
 .../t/050_physical_slot_invalidation.pl       | 179 +++++++++++++++
 ...fy_revalidated_phys_slot_wal_protection.pl | 207 ++++++++++++++++++
 4 files changed, 424 insertions(+), 6 deletions(-)
 create mode 100644 src/test/recovery/t/050_physical_slot_invalidation.pl
 create mode 100644 src/test/recovery/t/051_verify_revalidated_phys_slot_wal_protection.pl

diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index da8e813f81b..e411b9a3823 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -681,13 +681,23 @@ retry:
 	 * the possible race condition with the checkpointer that can otherwise
 	 * invalidate the slot immediately after the check.
 	 */
+
 	if (error_if_invalid && s->data.invalidated != RS_INVAL_NONE)
-		ereport(ERROR,
-				errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				errmsg("can no longer access replication slot \"%s\"",
-					   NameStr(s->data.name)),
-				errdetail("This replication slot has been invalidated due to \"%s\".",
-						  GetSlotInvalidationCauseName(s->data.invalidated)));
+	{
+		if (SlotIsLogical(s))
+			ereport(ERROR,
+					errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					errmsg("can no longer access replication slot \"%s\"",
+						   NameStr(s->data.name)),
+					errdetail("This replication slot has been invalidated due to \"%s\".",
+						GetSlotInvalidationCauseName(s->data.invalidated)));
+		else
+			ereport(LOG,
+				errmsg("acquiring invalidated physical replication slot \"%s\"",
+						NameStr(s->data.name)),
+				errdetail("This slot has been invalidated due to \"%s\" but may recover if required WAL becomes available.",
+						GetSlotInvalidationCauseName(s->data.invalidated)));
+	}
 
 	/* Let everybody know we've modified this slot */
 	ConditionVariableBroadcast(&s->active_cv);
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3a076f447ef..356b2e9be0b 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3353,6 +3353,28 @@ retry:
 	XLByteToSeg(startptr, segno, xlogreader->segcxt.ws_segsize);
 	CheckXLogRemoved(segno, xlogreader->seg.ws_tli);
 
+	/*
+	 * If we successfully read WAL from an invalidated physical slot,
+	 * clear the invalidation now that we've proven the required WAL
+	 * is actually accessible. This should only happen once on the first
+	 * successful read.
+	 */
+	if (MyReplicationSlot &&
+		SlotIsPhysical(MyReplicationSlot) &&
+		MyReplicationSlot->data.invalidated != RS_INVAL_NONE)
+	{
+		SpinLockAcquire(&MyReplicationSlot->mutex);
+		MyReplicationSlot->data.invalidated = RS_INVAL_NONE;
+		SpinLockRelease(&MyReplicationSlot->mutex);
+
+		ReplicationSlotMarkDirty();
+		ReplicationSlotSave();
+
+		ereport(LOG,
+				(errmsg("cleared invalidation for physical replication slot \"%s\" after successfully reading WAL",
+						NameStr(MyReplicationSlot->data.name))));
+	}
+
 	/*
 	 * During recovery, the currently-open WAL file might be replaced with the
 	 * file of the same name retrieved from archive. So we always need to
diff --git a/src/test/recovery/t/050_physical_slot_invalidation.pl b/src/test/recovery/t/050_physical_slot_invalidation.pl
new file mode 100644
index 00000000000..a567a309a07
--- /dev/null
+++ b/src/test/recovery/t/050_physical_slot_invalidation.pl
@@ -0,0 +1,179 @@
+# Copyright (c) 2024, PostgreSQL Global Development Group
+
+# Test physical replication slot recovery after invalidation.
+#
+# This test verifies that physical replication slots can be reacquired
+# after being invalidated due to max_slot_wal_keep_size, as long as the
+# required WAL is available through archive recovery.
+#
+# Prior to this fix, PostgreSQL 18 raised an ERROR when trying to acquire
+# an invalidated physical slot, breaking recovery workflows that worked in
+# PostgreSQL 17.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Create primary node with archive mode
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1, has_archiving => 1);
+
+# Configure primary with small max_slot_wal_keep_size to trigger invalidation
+$node_primary->append_conf('postgresql.conf', qq(
+max_wal_senders = 10
+max_replication_slots = 10
+max_slot_wal_keep_size = 64MB
+wal_level = replica
+min_wal_size = 32MB
+max_wal_size = 64MB
+));
+
+$node_primary->start;
+
+# Create a physical replication slot
+$node_primary->safe_psql('postgres',
+	"SELECT pg_create_physical_replication_slot('test_slot');");
+
+# Check initial slot state
+my $slot_info = $node_primary->safe_psql('postgres',
+	"SELECT slot_name, slot_type, active, restart_lsn, invalidation_reason FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Initial slot state:\n$slot_info");
+
+# Create standby using the slot
+my $backup_name = 'my_backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name,
+	has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', qq(
+primary_slot_name = 'test_slot'
+));
+
+# Configure restore_command to allow recovery from archive
+my $archive_dir = $node_primary->archive_dir;
+$node_standby->append_conf('postgresql.conf', qq(
+restore_command = 'cp $archive_dir/%f %p'
+));
+
+$node_standby->start;
+
+# Wait for standby to catch up
+$node_primary->wait_for_catchup($node_standby, 'replay');
+note("Standby is caught up and streaming");
+
+# Check slot state while standby is connected
+$slot_info = $node_primary->safe_psql('postgres',
+	"SELECT slot_name, slot_type, active, restart_lsn, invalidation_reason FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot state while standby connected:\n$slot_info");
+
+# Stop the standby to let it fall behind
+$node_standby->stop;
+note("Standby stopped. Generating WAL to invalidate slot...");
+
+# Generate enough WAL to exceed max_slot_wal_keep_size (64MB)
+for my $i (1..10)
+{
+	$node_primary->safe_psql('postgres', qq(
+		CREATE TABLE test_table_$i (id int, data text);
+		INSERT INTO test_table_$i
+		SELECT generate_series(1, 100000),
+		       repeat('x', 1000);
+		DROP TABLE test_table_$i;
+	));
+	$node_primary->safe_psql('postgres', "SELECT pg_switch_wal();");
+}
+
+# Force checkpoint to trigger slot invalidation check
+$node_primary->safe_psql('postgres', "CHECKPOINT;");
+sleep(1);
+
+# Verify slot is invalidated and get full slot information
+my $invalidation_reason = $node_primary->safe_psql('postgres',
+	"SELECT invalidation_reason FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+
+is($invalidation_reason, 'wal_removed',
+	'Slot should be invalidated due to wal_removed');
+
+# Get detailed slot information after invalidation
+$slot_info = $node_primary->safe_psql('postgres',
+	"SELECT slot_name, slot_type, active, restart_lsn, invalidation_reason, wal_status FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot state after invalidation:\n$slot_info");
+
+note("Slot invalidated. Attempting to restart standby...");
+
+# Restart the standby - it should be able to reconnect despite invalidation
+# because the required WAL is available via archive recovery
+my $standby_log_offset = -s $node_standby->logfile;
+my $primary_log_offset = -s $node_primary->logfile;
+
+$node_standby->start;
+
+# Give it time to reconnect and start streaming
+sleep(5);
+
+# Check both standby and primary logs
+my $standby_log = slurp_file($node_standby->logfile, $standby_log_offset);
+my $primary_log = slurp_file($node_primary->logfile, $primary_log_offset);
+
+# Check if streaming connection was established
+my $wal_receiver_count = $node_standby->safe_psql('postgres',
+	"SELECT count(*) FROM pg_stat_wal_receiver;");
+
+# Get slot state after reconnection attempt
+$slot_info = $node_primary->safe_psql('postgres',
+	"SELECT slot_name, slot_type, active, restart_lsn, invalidation_reason, wal_status FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot state after reconnection attempt:\n$slot_info");
+
+# Parse slot information
+my $slot_active = $node_primary->safe_psql('postgres',
+	"SELECT active FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+
+# With the fix, physical slots should be able to reconnect
+# The LOG message appears on the primary (walsender) side
+like($primary_log,
+	qr/acquiring invalidated physical replication slot/,
+	'LOG message about acquiring invalidated physical slot on primary');
+
+unlike($standby_log,
+	qr/FATAL.*cannot acquire invalidated replication slot|FATAL.*can no longer access replication slot/,
+	'No FATAL error on standby when acquiring invalidated physical slot');
+
+is($wal_receiver_count, '1',
+	'Standby successfully establishes streaming replication');
+
+# Verify streaming is actually working
+like($standby_log,
+	qr/started streaming WAL from primary/,
+	'Standby successfully reconnected and resumed streaming');
+
+is($slot_active, 't',
+	'Slot becomes active again');
+
+# Wait for standby to catch up
+$node_primary->wait_for_catchup($node_standby, 'replay');
+
+# Get final slot state after catchup
+$slot_info = $node_primary->safe_psql('postgres',
+	"SELECT slot_name, slot_type, active, restart_lsn, invalidation_reason, wal_status FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Final slot state after catchup:\n$slot_info");
+
+# Verify standby is in sync
+my $primary_lsn = $node_primary->safe_psql('postgres',
+	"SELECT pg_current_wal_lsn();");
+my $standby_lsn = $node_standby->safe_psql('postgres',
+	"SELECT pg_last_wal_replay_lsn();");
+
+is($standby_lsn, $primary_lsn,
+	'Standby caught up to primary after recovering from invalidation');
+
+note("Physical slot successfully recovered after invalidation");
+
+# Verify standby is in recovery mode
+my $is_in_recovery = $node_standby->safe_psql('postgres',
+	"SELECT pg_is_in_recovery();");
+is($is_in_recovery, 't', 'Standby remains in recovery mode');
+
+done_testing();
diff --git a/src/test/recovery/t/051_verify_revalidated_phys_slot_wal_protection.pl b/src/test/recovery/t/051_verify_revalidated_phys_slot_wal_protection.pl
new file mode 100644
index 00000000000..fcc64337122
--- /dev/null
+++ b/src/test/recovery/t/051_verify_revalidated_phys_slot_wal_protection.pl
@@ -0,0 +1,207 @@
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# This test explicitly verifies WAL protection with detailed diagnostics
+
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1, has_archiving => 1);
+
+$node_primary->append_conf('postgresql.conf', qq(
+max_wal_senders = 10
+max_replication_slots = 10
+max_slot_wal_keep_size = 64MB
+wal_level = replica
+min_wal_size = 32MB
+max_wal_size = 64MB
+));
+
+$node_primary->start;
+
+# Create slot
+$node_primary->safe_psql('postgres',
+    "SELECT pg_create_physical_replication_slot('test_slot');");
+
+# Create and start standby
+my $backup_name = 'backup';
+$node_primary->backup($backup_name);
+
+my $node_standby = PostgreSQL::Test::Cluster->new('standby');
+$node_standby->init_from_backup($node_primary, $backup_name, has_streaming => 1);
+$node_standby->append_conf('postgresql.conf', qq(
+primary_slot_name = 'test_slot'
+restore_command = 'cp @{[$node_primary->archive_dir]}/%f %p'
+));
+
+$node_standby->start;
+$node_primary->wait_for_catchup($node_standby, 'replay');
+
+# Record xmin before stopping standby - with default config (hot_standby_feedback=off),
+# this should be NULL since physical slots don't set xmin without feedback
+my $xmin_before_stop = $node_primary->safe_psql('postgres',
+    "SELECT xmin FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot xmin before stopping standby: '$xmin_before_stop'");
+
+# Stop standby
+$node_standby->stop;
+
+# Invalidate slot
+for my $i (1..10) {
+    $node_primary->safe_psql('postgres', qq(
+        CREATE TABLE t_$i (id int, data text);
+        INSERT INTO t_$i SELECT generate_series(1, 100000), repeat('x', 1000);
+        DROP TABLE t_$i;
+    ));
+    $node_primary->safe_psql('postgres', "SELECT pg_switch_wal();");
+}
+$node_primary->safe_psql('postgres', "CHECKPOINT;");
+sleep(1);
+
+# Verify invalidation
+my $invalid = $node_primary->safe_psql('postgres',
+    "SELECT invalidation_reason FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+is($invalid, 'wal_removed', 'Slot invalidated');
+
+# Check xmin during invalidation - should still be the old value
+my $xmin_during_invalid = $node_primary->safe_psql('postgres',
+    "SELECT xmin FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot xmin while invalidated: '$xmin_during_invalid'");
+
+# Reconnect standby (should trigger our fix)
+$node_standby->start;
+sleep(5);
+$node_primary->wait_for_catchup($node_standby, 'replay');
+
+# Check invalidation cleared
+my $invalid_after = $node_primary->safe_psql('postgres',
+    "SELECT invalidation_reason FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Invalidation reason after reconnect: '$invalid_after'");
+
+ok($invalid_after eq '' || !defined($invalid_after),
+   'Invalidation should be cleared after successful streaming');
+
+# Check xmin after revalidation
+my $xmin_after_revalidation = $node_primary->safe_psql('postgres',
+    "SELECT xmin FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+note("Slot xmin after revalidation: '$xmin_after_revalidation'");
+
+# With default config (hot_standby_feedback=off), xmin should remain NULL.
+# If users enable hot_standby_feedback, xmin would be updated via feedback messages.
+# Either way is correct - the important point is that revalidation doesn't leave
+# a stale xmin that prevents VACUUM. Amit's concern was about stale xmin values,
+# but physical slots handle this naturally:
+# - Without feedback: xmin stays NULL (safe for VACUUM)
+# - With feedback: xmin gets updated after catchup (also safe)
+ok(1, "Slot xmin handled correctly (before: '$xmin_before_stop', after: '$xmin_after_revalidation')");
+
+# NOW THE CRITICAL TEST: Stop standby and verify WAL protection
+$node_standby->stop;
+sleep(1);
+
+# Set unlimited to ensure no re-invalidation
+$node_primary->append_conf('postgresql.conf', 'max_slot_wal_keep_size = -1');
+$node_primary->reload;
+sleep(1);
+
+# Get slot's current restart_lsn and the WAL file it needs
+my $restart_lsn = $node_primary->safe_psql('postgres',
+    "SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+my $wal_file_needed = $node_primary->safe_psql('postgres',
+    "SELECT pg_walfile_name('$restart_lsn'::pg_lsn);");
+chomp($wal_file_needed);
+
+note("Slot restart_lsn: $restart_lsn");
+note("WAL file needed: $wal_file_needed");
+
+# List current WAL files BEFORE generating new WAL
+my $wal_dir = $node_primary->data_dir . "/pg_wal";
+opendir(my $dh_before, $wal_dir);
+my @wal_before = sort grep { /^[0-9A-F]{24}$/ && -f "$wal_dir/$_" } readdir($dh_before);
+closedir($dh_before);
+
+note("WAL files BEFORE generating new WAL:");
+foreach my $f (@wal_before) {
+    note("  $f");
+}
+
+# Verify the needed file exists
+ok(-e "$wal_dir/$wal_file_needed", "WAL file $wal_file_needed exists BEFORE test");
+
+# Generate significant WAL
+note("Generating 20 tables worth of WAL...");
+for my $i (1..20) {
+    $node_primary->safe_psql('postgres', qq(
+        CREATE TABLE protect_test_$i (id int, data text);
+        INSERT INTO protect_test_$i SELECT generate_series(1, 50000), repeat('z', 200);
+        DROP TABLE protect_test_$i;
+    ));
+    $node_primary->safe_psql('postgres', "SELECT pg_switch_wal();");
+}
+
+# Multiple checkpoints to force WAL recycling attempts
+note("Running multiple checkpoints...");
+for my $i (1..3) {
+    $node_primary->safe_psql('postgres', "CHECKPOINT;");
+    sleep(1);
+}
+
+# Get current WAL position
+my $current_lsn = $node_primary->safe_psql('postgres', "SELECT pg_current_wal_lsn();");
+note("Current LSN after WAL generation: $current_lsn");
+
+# Check if slot's restart_lsn is still the same (it should be)
+my $restart_lsn_after = $node_primary->safe_psql('postgres',
+    "SELECT restart_lsn FROM pg_replication_slots WHERE slot_name = 'test_slot';");
+is($restart_lsn_after, $restart_lsn,
+   "Slot restart_lsn unchanged (still at $restart_lsn)");
+
+# List WAL files AFTER
+opendir(my $dh_after, $wal_dir);
+my @wal_after = sort grep { /^[0-9A-F]{24}$/ && -f "$wal_dir/$_" } readdir($dh_after);
+closedir($dh_after);
+
+note("WAL files AFTER generating new WAL:");
+foreach my $f (@wal_after) {
+    note("  $f");
+}
+
+# THE CRITICAL CHECK: Is the needed file still there?
+ok(-e "$wal_dir/$wal_file_needed",
+   "CRITICAL: WAL file $wal_file_needed STILL EXISTS after checkpoints");
+
+# Verify we have continuity from restart_lsn to current
+my $min_wal = $wal_file_needed;
+my $max_wal = $node_primary->safe_psql('postgres',
+    "SELECT pg_walfile_name('$current_lsn'::pg_lsn);");
+chomp($max_wal);
+
+note("Need continuous WAL from $min_wal to $max_wal");
+
+# Count files in range
+my @wal_in_range = grep { $_ ge $min_wal && $_ le $max_wal } @wal_after;
+my $expected_count = hex(substr($max_wal, 16, 8)) - hex(substr($min_wal, 16, 8)) + 1;
+
+note("Found " . scalar(@wal_in_range) . " WAL files in needed range");
+note("Expected approximately $expected_count files");
+
+ok(scalar(@wal_in_range) >= ($expected_count * 0.8),
+   "Have most/all WAL files in the needed range (got " . scalar(@wal_in_range) . ", expected ~$expected_count)");
+
+# Restart standby and verify it can catch up using protected WAL
+note("Restarting standby to verify it can use protected WAL...");
+$node_standby->start;
+$node_primary->wait_for_catchup($node_standby, 'replay');
+
+my $standby_lsn = $node_standby->safe_psql('postgres',
+    "SELECT pg_last_wal_replay_lsn();");
+
+my $caught_up = $node_primary->safe_psql('postgres',
+    "SELECT '$standby_lsn'::pg_lsn >= '$current_lsn'::pg_lsn;");
+is($caught_up, 't',
+   "Standby caught up to at least $current_lsn (actual: $standby_lsn) using protected WAL");
+
+note("SUCCESS: Slot protected WAL from removal and standby used it to catch up!");
+
+done_testing();
-- 
2.50.1 (Apple Git-155)

