diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c index dc9566fb51b..50bb1d8cfc5 100644 --- a/src/backend/access/transam/subtrans.c +++ b/src/backend/access/transam/subtrans.c @@ -311,7 +311,7 @@ StartupSUBTRANS(TransactionId oldestActiveXID) FullTransactionId nextXid; int64 startPage; int64 endPage; - LWLock *prevlock; + LWLock *prevlock = NULL; LWLock *lock; /* @@ -324,42 +324,27 @@ StartupSUBTRANS(TransactionId oldestActiveXID) nextXid = TransamVariables->nextXid; endPage = TransactionIdToPage(XidFromFullTransactionId(nextXid)); - prevlock = SimpleLruGetBankLock(SubTransCtl, startPage); - LWLockAcquire(prevlock, LW_EXCLUSIVE); - while (startPage != endPage) + for (;;) { lock = SimpleLruGetBankLock(SubTransCtl, startPage); - - /* - * Check if we need to acquire the lock on the new bank then release - * the lock on the old bank and acquire on the new bank. - */ if (prevlock != lock) { - LWLockRelease(prevlock); + if (prevlock) + LWLockRelease(prevlock); LWLockAcquire(lock, LW_EXCLUSIVE); prevlock = lock; } (void) ZeroSUBTRANSPage(startPage); + if (startPage == endPage) + break; + startPage++; /* must account for wraparound */ if (startPage > TransactionIdToPage(MaxTransactionId)) startPage = 0; } - lock = SimpleLruGetBankLock(SubTransCtl, startPage); - - /* - * Check if we need to acquire the lock on the new bank then release the - * lock on the old bank and acquire on the new bank. - */ - if (prevlock != lock) - { - LWLockRelease(prevlock); - LWLockAcquire(lock, LW_EXCLUSIVE); - } - (void) ZeroSUBTRANSPage(startPage); LWLockRelease(lock); } diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl index a3cdcf32408..701f9cc20f8 100644 --- a/src/test/recovery/t/009_twophase.pl +++ b/src/test/recovery/t/009_twophase.pl @@ -45,6 +45,10 @@ $node_london->backup('london_backup'); my $node_paris = PostgreSQL::Test::Cluster->new('paris'); $node_paris->init_from_backup($node_london, 'london_backup', has_streaming => 1); +$node_paris->append_conf( + 'postgresql.conf', qq( + subtransaction_buffers = 32 +)); $node_paris->start; # Switch to synchronous replication in both directions @@ -481,4 +485,42 @@ is( $psql_out, qq{27|issued to paris}, "Check expected t_009_tbl2 data on standby"); + +# Exercise the 2PC recovery code in StartupSUBTRANS, which is concerned with +# ensuring that enough pg_subtrans pages exist on disk to cover the range of +# prepared transactions at server start time. There's not much we can verify +# directly, but let's at least get the code to run. +$cur_standby->stop(); +configure_and_reload($cur_primary, "synchronous_standby_names = ''"); + +$cur_primary->safe_psql('postgres', "CHECKPOINT"); + +my $start_lsn = + $cur_primary->safe_psql('postgres', 'select pg_current_wal_insert_lsn()'); +$cur_primary->safe_psql('postgres', + "CREATE TABLE test(); BEGIN; CREATE TABLE test1(); PREPARE TRANSACTION 'foo';" +); +my $osubtrans = $cur_primary->safe_psql('postgres', + "select 'pg_subtrans/'||f, s.size from pg_ls_dir('pg_subtrans') f, pg_stat_file('pg_subtrans/'||f) s" +); +$cur_primary->pgbench( + '--no-vacuum --client=5 --transactions=1000', + 0, + [], + [], + 'pgbench run to cause pg_subtrans traffic', + { + '009_twophase.pgb' => 'insert into test default values' + }); +# StartupSUBTRANS is exercised with a wide range of visible XIDs in this +# stop/start sequence, because we left a prepared transaction open above. +# Also, setting subtransaction_buffers to 32 above causes to switch SLRU +# bank, for additional code coverage. +$cur_primary->stop; +$cur_primary->start; +my $nsubtrans = $cur_primary->safe_psql('postgres', + "select 'pg_subtrans/'||f, s.size from pg_ls_dir('pg_subtrans') f, pg_stat_file('pg_subtrans/'||f) s" +); +isnt($osubtrans, $nsubtrans, "contents of pg_subtrans/ have changed"); + done_testing();