Fix "invalid spinlock number: 0" error in pg_stat_wal_receiver.

Commit 2c8dd05d6c added the atomic variable writtenUpto into
walreceiver's shared memory information. It's initialized only
when walreceiver started up but could be read via pg_stat_wal_receiver
view anytime, i.e., even before it's initialized. In the server built
with --disable-atomics and --disable-spinlocks, this uninitialized
atomic variable read could cause "invalid spinlock number: 0" error.

This commit changed writtenUpto so that it's initialized at
the postmaster startup, to avoid the uninitialized variable read
via pg_stat_wal_receiver and fix the error.

Also this commit moved the read of writtenUpto after the release
of spinlock protecting walreceiver's shared variables. This is
necessary to prevent new spinlock from being taken by atomic
variable read while holding another spinlock, and to shorten
the spinlock duration. This change leads writtenUpto not to be
consistent with the other walreceiver's shared variables protected
by a spinlock. But this is OK because writtenUpto should not be
used for data integrity checks.

Back-patch to v13 where commit 2c8dd05d6c introduced the bug.

Author: Fujii Masao
Reviewed-by: Michael Paquier, Thomas Munro, Andres Freund
Discussion: https://postgr.es/m/7ef8708c-5b6b-edd3-2cf2-7783f1c7c175@oss.nttdata.com

diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 723f513d8bcb240b073ee7eb2339fc19045a8a6a..9ec71238c48923839fee4fdcc131ad5c4d49f4e7 100644 (file)
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -249,7 +249,7 @@ WalReceiverMain(void)
 
    SpinLockRelease(&walrcv->mutex);
 
-   pg_atomic_init_u64(&WalRcv->writtenUpto, 0);
+   pg_atomic_write_u64(&WalRcv->writtenUpto, 0);
 
    /* Arrange to clean up at walreceiver exit */
    on_shmem_exit(WalRcvDie, 0);
@@ -1325,7 +1325,6 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
    state = WalRcv->walRcvState;
    receive_start_lsn = WalRcv->receiveStart;
    receive_start_tli = WalRcv->receiveStartTLI;
-   written_lsn = pg_atomic_read_u64(&WalRcv->writtenUpto);
    flushed_lsn = WalRcv->flushedUpto;
    received_tli = WalRcv->receivedTLI;
    last_send_time = WalRcv->lastMsgSendTime;
@@ -1345,6 +1344,14 @@ pg_stat_get_wal_receiver(PG_FUNCTION_ARGS)
    if (pid == 0 || !ready_to_display)
        PG_RETURN_NULL();
 
+   /*
+    * Read "writtenUpto" without holding a spinlock.  Note that it may not be
+    * consistent with the other shared variables of the WAL receiver
+    * protected by a spinlock, but this should not be used for data integrity
+    * checks.
+    */
+   written_lsn = pg_atomic_read_u64(&WalRcv->writtenUpto);
+
    /* determine result type */
    if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
        elog(ERROR, "return type must be a row type");

diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
index 69b91a7dab8f54653b41161fe4dd4bb8b90cf17d..63e60478ea6e8f4b7041ac28808c039958dfe3a4 100644 (file)
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -63,6 +63,7 @@ WalRcvShmemInit(void)
        MemSet(WalRcv, 0, WalRcvShmemSize());
        WalRcv->walRcvState = WALRCV_STOPPED;
        SpinLockInit(&WalRcv->mutex);
+       pg_atomic_init_u64(&WalRcv->writtenUpto, 0);
        WalRcv->latch = NULL;
    }
 }

diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out
index 81bdacf59daa5a9718dc93a04f1af913a0b3871f..6d048e309cb093efb1d05ecc5e60d441891e411a 100644 (file)
--- a/src/test/regress/expected/sysviews.out
+++ b/src/test/regress/expected/sysviews.out
@@ -83,6 +83,13 @@ select count(*) = 1 as ok from pg_stat_wal;
  t
 (1 row)
 
+-- We expect no walreceiver running in this test
+select count(*) = 0 as ok from pg_stat_wal_receiver;
+ ok 
+----
+ t
+(1 row)
+
 -- This is to record the prevailing planner enable_foo settings during
 -- a regression test run.
 select name, setting from pg_settings where name like 'enable%';

diff --git a/src/test/regress/sql/sysviews.sql b/src/test/regress/sql/sysviews.sql
index b9b875bc6abc2528a79d24e5d0f10f3227f7ca6b..dc8c9a3ac23f86b663364d4e2af68cb6247eab0a 100644 (file)
--- a/src/test/regress/sql/sysviews.sql
+++ b/src/test/regress/sql/sysviews.sql
@@ -40,6 +40,9 @@ select count(*) >= 0 as ok from pg_prepared_xacts;
 -- There must be only one record
 select count(*) = 1 as ok from pg_stat_wal;
 
+-- We expect no walreceiver running in this test
+select count(*) = 0 as ok from pg_stat_wal_receiver;
+
 -- This is to record the prevailing planner enable_foo settings during
 -- a regression test run.
 select name, setting from pg_settings where name like 'enable%';