diff --git a/contrib/pg_visibility/pg_visibility.c b/contrib/pg_visibility/pg_visibility.c index 1a1a4ff7be7..773ba92e454 100644 --- a/contrib/pg_visibility/pg_visibility.c +++ b/contrib/pg_visibility/pg_visibility.c @@ -546,11 +546,21 @@ collect_visibility_data(Oid relid, bool include_pd) * * 1. Ignore processes xmin's, because they consider connection to other * databases that were ignored before. - * 2. Ignore KnownAssignedXids, because they are not database-aware. At the - * same time, the primary could compute its horizons database-aware. + * 2. Ignore KnownAssignedXids, as they are not database-aware. Although we + * now perform minimal checking on a standby by always using nextXid, this + * approach is better than nothing and will at least catch extremely broken + * cases where a xid is in the future. * 3. Ignore walsender xmin, because it could go backward if some replication * connections don't use replication slots. * + * While it might seem like we could use KnownAssignedXids for shared + * catalogs, since shared catalogs rely on a global horizon rather than a + * database-specific one - there are potential edge cases. For example, a + * transaction may crash on the primary without writing a commit/abort record. + * This would lead to a situation where it appears to still be running on the + * standby, even though it has already ended on the primary. For this reason, + * it's safer to ignore KnownAssignedXids, even for shared catalogs. + * * As a result, we're using only currently running xids to compute the horizon. * Surely these would significantly sacrifice accuracy. But we have to do so * to avoid reporting false errors. @@ -560,7 +570,17 @@ GetStrictOldestNonRemovableTransactionId(Relation rel) { RunningTransactions runningTransactions; - if (rel == NULL || rel->rd_rel->relisshared || RecoveryInProgress()) + if (RecoveryInProgress()) + { + TransactionId result; + + /* As we ignore KnownAssignedXids on standby, just pick nextXid */ + LWLockAcquire(XidGenLock, LW_SHARED); + result = XidFromFullTransactionId(TransamVariables->nextXid); + LWLockRelease(XidGenLock); + return result; + } + else if (rel == NULL || rel->rd_rel->relisshared) { /* Shared relation: take into account all running xids */ runningTransactions = GetRunningTransactionData(); diff --git a/contrib/pg_visibility/t/001_concurrent_transaction.pl b/contrib/pg_visibility/t/001_concurrent_transaction.pl index c31d041757d..498ce412d9a 100644 --- a/contrib/pg_visibility/t/001_concurrent_transaction.pl +++ b/contrib/pg_visibility/t/001_concurrent_transaction.pl @@ -10,11 +10,18 @@ use PostgreSQL::Test::Utils; use Test::More; +# Initialize the primary node my $node = PostgreSQL::Test::Cluster->new('main'); - -$node->init; +$node->init(allows_streaming => 1); $node->start; +# Initialize the streaming standby +my $backup_name = 'my_backup'; +$node->backup($backup_name); +my $standby = PostgreSQL::Test::Cluster->new('standby'); +$standby->init_from_backup($node, $backup_name, has_streaming => 1); +$standby->start; + # Setup another database $node->safe_psql("postgres", "CREATE DATABASE other_database;\n"); my $bsession = $node->background_psql('other_database'); @@ -39,9 +46,17 @@ my $result = $node->safe_psql("postgres", # There should be no false negatives ok($result eq "", "pg_check_visible() detects no errors"); +# Run pg_check_visible() on standby +$result = $standby->safe_psql("postgres", + "SELECT * FROM pg_check_visible('vacuum_test');"); + +# There should be no false negatives either +ok($result eq "", "pg_check_visible() detects no errors"); + # Shutdown $bsession->query_safe("COMMIT;"); $bsession->quit; $node->stop; +$standby->stop; done_testing();