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.kpltxxvjzislidks@alap3.anarazel.de
Backpatch-through: 11

diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c
index f2ce086732ac24b0741f06624a646cb112d52ff4..be85e90a70dbbefc183fc347c9346a80c0444c2c 100644 (file)
--- a/src/backend/replication/slotfuncs.c
+++ b/src/backend/replication/slotfuncs.c
@@ -532,6 +532,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 acc349dec6eea8a8f01afbef0b14a3bd92a34565..e787c82a6d2cc4a3b2973fec1b223a40a02bafbf 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');
@@ -335,14 +335,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);
@@ -359,3 +372,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");