From a03f366f5e14a4db7cf3f89f1adf8a311490651b Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 6 Mar 2024 08:44:29 +0000 Subject: [PATCH v8 1/4] Track invalidation_reason in pg_replication_slots Currently the reason for replication slot invalidation is not tracked in pg_replication_slots. A recent commit 007693f2a added conflict_reason to show the reasons for slot invalidation, but only for logical slots. This commit renames conflict_reason to invalidation_reason, and adds the support to show invalidation reasons for both physical and logical slots. --- doc/src/sgml/ref/pgupgrade.sgml | 2 +- doc/src/sgml/system-views.sgml | 11 ++-- src/backend/catalog/system_views.sql | 2 +- src/backend/replication/logical/slotsync.c | 2 +- src/backend/replication/slot.c | 6 +-- src/backend/replication/slotfuncs.c | 11 +--- src/bin/pg_upgrade/info.c | 4 +- src/include/catalog/pg_proc.dat | 2 +- src/include/replication/slot.h | 2 +- .../t/035_standby_logical_decoding.pl | 50 +++++++++---------- .../t/040_standby_failover_slots_sync.pl | 4 +- src/test/regress/expected/rules.out | 4 +- 12 files changed, 47 insertions(+), 53 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index 58c6c2df8b..50d13f3c1e 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -453,7 +453,7 @@ make prefix=/usr/local/pgsql.new install All slots on the old cluster must be usable, i.e., there are no slots whose - pg_replication_slots.conflict_reason + pg_replication_slots.invalidation_reason is not NULL. diff --git a/doc/src/sgml/system-views.sgml b/doc/src/sgml/system-views.sgml index be90edd0e2..c519b4a7f8 100644 --- a/doc/src/sgml/system-views.sgml +++ b/doc/src/sgml/system-views.sgml @@ -2525,13 +2525,14 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx - conflict_reason text + invalidation_reason text - The reason for the logical slot's conflict with recovery. It is always - NULL for physical slots, as well as for logical slots which are not - invalidated. The non-NULL values indicate that the slot is marked - as invalidated. Possible values are: + The reason for the slot's invalidation. NULL if the + slot is currently actively being used. The non-NULL values indicate that + the slot is marked as invalidated. In case of logical slots, it + represents the reason for the logical slot's conflict with recovery. + Possible values are: diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index 04227a72d1..1dbfcef9f1 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1023,7 +1023,7 @@ CREATE VIEW pg_replication_slots AS L.wal_status, L.safe_wal_size, L.two_phase, - L.conflict_reason, + L.invalidation_reason, L.failover, L.synced FROM pg_get_replication_slots() AS L diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c index ad0fc6a04b..80ffc24213 100644 --- a/src/backend/replication/logical/slotsync.c +++ b/src/backend/replication/logical/slotsync.c @@ -664,7 +664,7 @@ synchronize_slots(WalReceiverConn *wrconn) bool started_tx = false; const char *query = "SELECT slot_name, plugin, confirmed_flush_lsn," " restart_lsn, catalog_xmin, two_phase, failover," - " database, conflict_reason" + " database, invalidation_reason" " FROM pg_catalog.pg_replication_slots" " WHERE failover and NOT temporary"; diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c index 02ae27499b..b0f48229cb 100644 --- a/src/backend/replication/slot.c +++ b/src/backend/replication/slot.c @@ -2333,17 +2333,17 @@ RestoreSlotFromDisk(const char *name) * ReplicationSlotInvalidationCause. */ ReplicationSlotInvalidationCause -GetSlotInvalidationCause(const char *conflict_reason) +GetSlotInvalidationCause(const char *invalidation_reason) { ReplicationSlotInvalidationCause cause; ReplicationSlotInvalidationCause result = RS_INVAL_NONE; bool found PG_USED_FOR_ASSERTS_ONLY = false; - Assert(conflict_reason); + Assert(invalidation_reason); for (cause = RS_INVAL_NONE; cause <= RS_INVAL_MAX_CAUSES; cause++) { - if (strcmp(SlotInvalidationCauses[cause], conflict_reason) == 0) + if (strcmp(SlotInvalidationCauses[cause], invalidation_reason) == 0) { found = true; result = cause; diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 768a304723..758498d29d 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -409,17 +409,10 @@ pg_get_replication_slots(PG_FUNCTION_ARGS) values[i++] = BoolGetDatum(slot_contents.data.two_phase); - if (slot_contents.data.database == InvalidOid) + if (slot_contents.data.invalidated == RS_INVAL_NONE) nulls[i++] = true; else - { - ReplicationSlotInvalidationCause cause = slot_contents.data.invalidated; - - if (cause == RS_INVAL_NONE) - nulls[i++] = true; - else - values[i++] = CStringGetTextDatum(SlotInvalidationCauses[cause]); - } + values[i++] = CStringGetTextDatum(SlotInvalidationCauses[slot_contents.data.invalidated]); values[i++] = BoolGetDatum(slot_contents.data.failover); diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 183c2f84eb..9683c91d4a 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -667,13 +667,13 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check) * removed. */ res = executeQueryOrDie(conn, "SELECT slot_name, plugin, two_phase, failover, " - "%s as caught_up, conflict_reason IS NOT NULL as invalid " + "%s as caught_up, invalidation_reason IS NOT NULL as invalid " "FROM pg_catalog.pg_replication_slots " "WHERE slot_type = 'logical' AND " "database = current_database() AND " "temporary IS FALSE;", live_check ? "FALSE" : - "(CASE WHEN conflict_reason IS NOT NULL THEN FALSE " + "(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE " "ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) " "END)"); diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 291ed876fc..69140a0bf0 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -11122,7 +11122,7 @@ proargtypes => '', proallargtypes => '{name,name,text,oid,bool,bool,int4,xid,xid,pg_lsn,pg_lsn,text,int8,bool,text,bool,bool}', proargmodes => '{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}', - proargnames => '{slot_name,plugin,slot_type,datoid,temporary,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn,wal_status,safe_wal_size,two_phase,conflict_reason,failover,synced}', + proargnames => '{slot_name,plugin,slot_type,datoid,temporary,active,active_pid,xmin,catalog_xmin,restart_lsn,confirmed_flush_lsn,wal_status,safe_wal_size,two_phase,invalidation_reason,failover,synced}', prosrc => 'pg_get_replication_slots' }, { oid => '3786', descr => 'set up a logical replication slot', proname => 'pg_create_logical_replication_slot', provolatile => 'v', diff --git a/src/include/replication/slot.h b/src/include/replication/slot.h index acbf567150..02a96b0e19 100644 --- a/src/include/replication/slot.h +++ b/src/include/replication/slot.h @@ -272,6 +272,6 @@ extern void CheckPointReplicationSlots(bool is_shutdown); extern void CheckSlotRequirements(void); extern void CheckSlotPermissions(void); extern ReplicationSlotInvalidationCause - GetSlotInvalidationCause(const char *conflict_reason); + GetSlotInvalidationCause(const char *invalidation_reason); #endif /* SLOT_H */ diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl index 2659d4bb52..a02ae84991 100644 --- a/src/test/recovery/t/035_standby_logical_decoding.pl +++ b/src/test/recovery/t/035_standby_logical_decoding.pl @@ -168,8 +168,8 @@ sub change_hot_standby_feedback_and_wait_for_xmins } } -# Check conflict_reason in pg_replication_slots. -sub check_slots_conflict_reason +# Check invalidation_reason in pg_replication_slots. +sub check_slots_invalidation_reason { my ($slot_prefix, $reason) = @_; @@ -178,15 +178,15 @@ sub check_slots_conflict_reason $res = $node_standby->safe_psql( 'postgres', qq( - select conflict_reason from pg_replication_slots where slot_name = '$active_slot';)); + select invalidation_reason from pg_replication_slots where slot_name = '$active_slot';)); - is($res, "$reason", "$active_slot conflict_reason is $reason"); + is($res, "$reason", "$active_slot invalidation_reason is $reason"); $res = $node_standby->safe_psql( 'postgres', qq( - select conflict_reason from pg_replication_slots where slot_name = '$inactive_slot';)); + select invalidation_reason from pg_replication_slots where slot_name = '$inactive_slot';)); - is($res, "$reason", "$inactive_slot conflict_reason is $reason"); + is($res, "$reason", "$inactive_slot invalidation_reason is $reason"); } # Drop the slots, re-create them, change hot_standby_feedback, @@ -293,13 +293,13 @@ $node_primary->safe_psql('testdb', qq[SELECT * FROM pg_create_physical_replication_slot('$primary_slotname');] ); -# Check conflict_reason is NULL for physical slot +# Check invalidation_reason is NULL for physical slot $res = $node_primary->safe_psql( 'postgres', qq[ - SELECT conflict_reason is null FROM pg_replication_slots where slot_name = '$primary_slotname';] + SELECT invalidation_reason is null FROM pg_replication_slots where slot_name = '$primary_slotname';] ); -is($res, 't', "Physical slot reports conflict_reason as NULL"); +is($res, 't', "Physical slot reports invalidation_reason as NULL"); my $backup_name = 'b1'; $node_primary->backup($backup_name); @@ -512,8 +512,8 @@ $node_primary->wait_for_replay_catchup($node_standby); # Check invalidation in the logfile and in pg_stat_database_conflicts check_for_invalidation('vacuum_full_', 1, 'with vacuum FULL on pg_class'); -# Verify conflict_reason is 'rows_removed' in pg_replication_slots -check_slots_conflict_reason('vacuum_full_', 'rows_removed'); +# Verify invalidation_reason is 'rows_removed' in pg_replication_slots +check_slots_invalidation_reason('vacuum_full_', 'rows_removed'); $handle = make_slot_active($node_standby, 'vacuum_full_', 0, \$stdout, \$stderr); @@ -531,8 +531,8 @@ change_hot_standby_feedback_and_wait_for_xmins(1, 1); ################################################## $node_standby->restart; -# Verify conflict_reason is retained across a restart. -check_slots_conflict_reason('vacuum_full_', 'rows_removed'); +# Verify invalidation_reason is retained across a restart. +check_slots_invalidation_reason('vacuum_full_', 'rows_removed'); ################################################## # Verify that invalidated logical slots do not lead to retaining WAL. @@ -540,7 +540,7 @@ check_slots_conflict_reason('vacuum_full_', 'rows_removed'); # Get the restart_lsn from an invalidated slot my $restart_lsn = $node_standby->safe_psql('postgres', - "SELECT restart_lsn from pg_replication_slots WHERE slot_name = 'vacuum_full_activeslot' and conflict_reason is not null;" + "SELECT restart_lsn from pg_replication_slots WHERE slot_name = 'vacuum_full_activeslot' and invalidation_reason is not null;" ); chomp($restart_lsn); @@ -591,8 +591,8 @@ $node_primary->wait_for_replay_catchup($node_standby); # Check invalidation in the logfile and in pg_stat_database_conflicts check_for_invalidation('row_removal_', $logstart, 'with vacuum on pg_class'); -# Verify conflict_reason is 'rows_removed' in pg_replication_slots -check_slots_conflict_reason('row_removal_', 'rows_removed'); +# Verify invalidation_reason is 'rows_removed' in pg_replication_slots +check_slots_invalidation_reason('row_removal_', 'rows_removed'); $handle = make_slot_active($node_standby, 'row_removal_', 0, \$stdout, \$stderr); @@ -627,8 +627,8 @@ $node_primary->wait_for_replay_catchup($node_standby); check_for_invalidation('shared_row_removal_', $logstart, 'with vacuum on pg_authid'); -# Verify conflict_reason is 'rows_removed' in pg_replication_slots -check_slots_conflict_reason('shared_row_removal_', 'rows_removed'); +# Verify invalidation_reason is 'rows_removed' in pg_replication_slots +check_slots_invalidation_reason('shared_row_removal_', 'rows_removed'); $handle = make_slot_active($node_standby, 'shared_row_removal_', 0, \$stdout, \$stderr); @@ -680,7 +680,7 @@ ok( $node_standby->poll_query_until( is( $node_standby->safe_psql( 'postgres', q[select bool_or(conflicting) from - (select conflict_reason is not NULL as conflicting + (select invalidation_reason is not NULL as conflicting from pg_replication_slots WHERE slot_type = 'logical')]), 'f', 'Logical slots are reported as non conflicting'); @@ -719,8 +719,8 @@ $node_primary->wait_for_replay_catchup($node_standby); # Check invalidation in the logfile and in pg_stat_database_conflicts check_for_invalidation('pruning_', $logstart, 'with on-access pruning'); -# Verify conflict_reason is 'rows_removed' in pg_replication_slots -check_slots_conflict_reason('pruning_', 'rows_removed'); +# Verify invalidation_reason is 'rows_removed' in pg_replication_slots +check_slots_invalidation_reason('pruning_', 'rows_removed'); $handle = make_slot_active($node_standby, 'pruning_', 0, \$stdout, \$stderr); @@ -825,8 +825,8 @@ SKIP: $logstart), "activeslot slot invalidation is logged with injection point"); - # Verify conflict_reason is 'rows_removed' in pg_replication_slots. - check_slots_conflict_reason('injection_', 'rows_removed'); + # Verify invalidation_reason is 'rows_removed' in pg_replication_slots. + check_slots_invalidation_reason('injection_', 'rows_removed'); # Detach from the injection point $node_standby->safe_psql('testdb', @@ -875,8 +875,8 @@ $node_primary->wait_for_replay_catchup($node_standby); # Check invalidation in the logfile and in pg_stat_database_conflicts check_for_invalidation('wal_level_', $logstart, 'due to wal_level'); -# Verify conflict_reason is 'wal_level_insufficient' in pg_replication_slots -check_slots_conflict_reason('wal_level_', 'wal_level_insufficient'); +# Verify invalidation_reason is 'wal_level_insufficient' in pg_replication_slots + check_slots_invalidation_reason('wal_level_', 'wal_level_insufficient'); $handle = make_slot_active($node_standby, 'wal_level_', 0, \$stdout, \$stderr); diff --git a/src/test/recovery/t/040_standby_failover_slots_sync.pl b/src/test/recovery/t/040_standby_failover_slots_sync.pl index 021c58f621..2e1d01f750 100644 --- a/src/test/recovery/t/040_standby_failover_slots_sync.pl +++ b/src/test/recovery/t/040_standby_failover_slots_sync.pl @@ -228,7 +228,7 @@ $standby1->safe_psql('postgres', "CHECKPOINT"); # Check if the synced slot is invalidated is( $standby1->safe_psql( 'postgres', - q{SELECT conflict_reason = 'wal_removed' FROM pg_replication_slots WHERE slot_name = 'lsub1_slot';} + q{SELECT invalidation_reason = 'wal_removed' FROM pg_replication_slots WHERE slot_name = 'lsub1_slot';} ), "t", 'synchronized slot has been invalidated'); @@ -274,7 +274,7 @@ $standby1->wait_for_log(qr/dropped replication slot "lsub1_slot" of dbid [0-9]+/ # flagged as 'synced' is( $standby1->safe_psql( 'postgres', - q{SELECT conflict_reason IS NULL AND synced AND NOT temporary FROM pg_replication_slots WHERE slot_name = 'lsub1_slot';} + q{SELECT invalidation_reason IS NULL AND synced AND NOT temporary FROM pg_replication_slots WHERE slot_name = 'lsub1_slot';} ), "t", 'logical slot is re-synced'); diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 0cd2c64fca..08b0a34d55 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -1473,10 +1473,10 @@ pg_replication_slots| SELECT l.slot_name, l.wal_status, l.safe_wal_size, l.two_phase, - l.conflict_reason, + l.invalidation_reason, l.failover, l.synced - FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, temporary, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn, wal_status, safe_wal_size, two_phase, conflict_reason, failover, synced) + FROM (pg_get_replication_slots() l(slot_name, plugin, slot_type, datoid, temporary, active, active_pid, xmin, catalog_xmin, restart_lsn, confirmed_flush_lsn, wal_status, safe_wal_size, two_phase, invalidation_reason, failover, synced) LEFT JOIN pg_database d ON ((l.datoid = d.oid))); pg_roles| SELECT pg_authid.rolname, pg_authid.rolsuper, -- 2.34.1