Fix second race with timeline selection during promotion

read_local_xlog_page_guts has the same race as logical_read_xlog_page:
RecoveryInProgress() can return true during promotion, impacting the
availability of the operations doing WAL page reads with this callback.

This problem is similar to eb4e7224a1c6 that has addressed the issue for
logical replication, impacting more areas of the code where this WAL
page callback can be used (same narrow window during promotion, same
availability issue):
- pg_walinspect.
- Slot advance (SQL function).
- Slot creation.

Repack workers (v19~) and 2PC files (since forever) can also use this
callback, but they are irrelevant as far as I know.  A test is added
with the SQL lookup functions.  This part relies on injection points,
and is backpatched down to v18, like the test added for eb4e7224a1c6.

This issue could probably be fixed as well in v14 and v15 for
pg_walinspect.  However, I also feel that there is a conservative
argument about consistency here due to the support of logical decoding
on standbys, so let's limit ourselves to v16 for now.  pg_walinspect is
used less in the field compared to the two other operations, making
addressing this problem less attractive in these two older branches.

Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/7daef094-abf3-4672-bc23-3df4763b16a3%40gmail.com
Backpatch-through: 16

diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c
index c389b27f77d47b3b21a0bc427a82d2b3d49c433a..db5a314edf86957d59f867c3be3e7a14debb2ae6 100644 (file)
--- a/src/backend/access/transam/xlogutils.c
+++ b/src/backend/access/transam/xlogutils.c
@@ -896,7 +896,19 @@ read_local_xlog_page_guts(XLogReaderState *state, XLogRecPtr targetPagePtr,
                if (!RecoveryInProgress())
                        read_upto = GetFlushRecPtr(&currTLI);
                else
+               {
+                       TimeLineID      insertTLI;
+
                        read_upto = GetXLogReplayRecPtr(&currTLI);
+
+                       /*
+                        * If the insertion timeline has already been set, use it. See
+                        * logical_read_xlog_page() for details.
+                        */
+                       insertTLI = GetWALInsertionTimeLineIfSet();
+                       if (insertTLI != 0)
+                               currTLI = insertTLI;
+               }
                tli = currTLI;
 
                /*

diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index d8acc8c173eac89613c745b8430a73b3f5d50dcb..bbc33afde3db6ebf38248428762ab0c1520372ca 100644 (file)
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -1065,6 +1065,8 @@ is($cascading_stdout, $expected,
 # Create a logical slot on the cascading standby for this test.
 $node_cascading_standby->create_logical_slot_on_standby($node_standby,
        'race_slot', 'testdb');
+$node_cascading_standby->create_logical_slot_on_standby($node_standby,
+       'race_slot_sql', 'testdb');
 
 $node_standby->safe_psql('testdb',
        qq[INSERT INTO decoding_test(x,y) SELECT s, s::text FROM generate_series(10,13) s;]
@@ -1081,6 +1083,10 @@ COMMIT};
 $node_standby->safe_psql('testdb', 'CREATE EXTENSION injection_points;');
 $node_standby->wait_for_replay_catchup($node_cascading_standby);
 
+# Open a background psql session BEFORE promotion for the SQL decoding
+# test.
+my $decode_session = $node_cascading_standby->background_psql('testdb');
+
 # Attach injection point to pause startup after WAL segment cleanup
 # but before RecoveryInProgress() flips to false.
 $node_cascading_standby->safe_psql('testdb',
@@ -1119,6 +1125,13 @@ is($stdout2, $expected,
        'got expected output from pg_recvlogical during promotion timeline switch'
 );
 
+# Verify SQL decoding.
+my $sql_out = $decode_session->query_safe(
+       "SELECT data FROM pg_logical_slot_peek_changes('race_slot_sql', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1')"
+);
+is($sql_out, $expected,
+       'pg_logical_slot_peek_changes works during promotion timeline switch');
+
 # Resume promotion.
 $node_cascading_standby->safe_psql('testdb',
        "SELECT injection_points_wakeup('promotion-after-wal-segment-cleanup');");