Fix oldest xmin and LSN computation across repslots after advancing

Advancing a replication slot did not recompute the oldest xmin and LSN
values across replication slots, preventing resource removal like
segments not recycled at checkpoint time.  The original commit that
introduced the slot advancing in 9c7d06d never did the update of those
oldest values, and b0afdca removed this code.

This commit adds a TAP test to check segment recycling with advancing
for physical slots, enforcing an extra segment switch before advancing
to check if the segment gets correctly recycled after a checkpoint.

Reported-by: Andres Freund
Reviewed-by: Alexey Kondratov, Kyptaro Horiguchi
Discussion: https://postgr.es/m/20200609171904[email protected]
Backpatch-through: 11

diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index 947644b5dd6c8ade421f8828db7c006a7a985b04..9e69360370769357b676a5d83a3e10799545e81d 100644 (file)
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -581,6 +581,13 @@ pg_replication_slot_advance(PG_FUNCTION_ARGS)
    values[0] = NameGetDatum(&MyReplicationSlot->data.name);
    nulls[0] = false;
 
+   /*
+    * Recompute the minimum LSN and xmin across all slots to adjust with the
+    * advancing potentially done.
+    */
+   ReplicationSlotsComputeRequiredXmin(false);
+   ReplicationSlotsComputeRequiredLSN();
+
    ReplicationSlotRelease();
 
    /* Return the reached position. */

diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index d09ebe65a390616a8ac2c9d705a6fde4035fd6ef..be51fa0ffe31c4dd68a09b51761ea16a0b9dfab2 100644 (file)
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 34;
+use Test::More tests => 35;
 
 # Initialize master node
 my $node_master = get_new_node('master');
@@ -345,14 +345,27 @@ is($xmin, '', 'xmin of cascaded slot null with hs feedback reset');
 is($catalog_xmin, '',
    'catalog xmin of cascaded slot still null with hs_feedback reset');
 
+$node_standby_1->stop;
+
+# Drop any existing slots on the primary, for the follow-up tests.
+$node_master->safe_psql('postgres',
+   "SELECT pg_drop_replication_slot(slot_name) FROM pg_replication_slots;");
+
 # Test physical slot advancing and its durability.  Create a new slot on
 # the primary, not used by any of the standbys. This reserves WAL at creation.
 my $phys_slot = 'phys_slot';
 $node_master->safe_psql('postgres',
    "SELECT pg_create_physical_replication_slot('$phys_slot', true);");
+# Generate some WAL, and switch to a new segment, used to check that
+# the previous segment is correctly getting recycled as the slot advancing
+# would recompute the minimum LSN calculated across all slots.
+my $segment_removed = $node_master->safe_psql('postgres',
+   'SELECT pg_walfile_name(pg_current_wal_lsn())');
+chomp($segment_removed);
 $node_master->psql('postgres', "
    CREATE TABLE tab_phys_slot (a int);
-   INSERT INTO tab_phys_slot VALUES (generate_series(1,10));");
+   INSERT INTO tab_phys_slot VALUES (generate_series(1,10));
+   SELECT pg_switch_wal();");
 my $current_lsn = $node_master->safe_psql('postgres',
    "SELECT pg_current_wal_lsn();");
 chomp($current_lsn);
@@ -369,3 +382,9 @@ my $phys_restart_lsn_post = $node_master->safe_psql('postgres',
 chomp($phys_restart_lsn_post);
 ok(($phys_restart_lsn_pre cmp $phys_restart_lsn_post) == 0,
    "physical slot advance persists across restarts");
+
+# Check if the previous segment gets correctly recycled after the
+# server stopped cleanly, causing a shutdown checkpoint to be generated.
+my $master_data = $node_master->data_dir;
+ok(!-f "$master_data/pg_wal/$segment_removed",
+   "WAL segment $segment_removed recycled after physical slot advancing");