postgres.git / summary / log / commit / refs

commit    d4b667e9353a70c4ee2847603e5ad2e14e20f82e
Author:   Fujii Masao <fujii@postgresql.org>
Date:     Thu Feb 18 14:28:58 2021 +0000
First released in: 14.0, 13.3

    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


src/backend/replication/walreceiver.c | 11 +++++++++-- src/backend/replication/walreceiverfuncs.c | 1 + src/test/regress/expected/sysviews.out | 7 +++++++ src/test/regress/sql/sysviews.sql | 3 +++ 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index a6c0351c815..d4f294f4aa6 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -261,7 +261,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); @@ -1376,7 +1376,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; @@ -1396,6 +1395,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 c3e317df9ff..3984228a113 100644 --- 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 06c4c3e4763..c3b988597c6 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -67,6 +67,13 @@ select count(*) >= 0 as ok from pg_prepared_xacts; 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 28e412b7353..5eb111d3fda 100644 --- a/src/test/regress/sql/sysviews.sql +++ b/src/test/regress/sql/sysviews.sql @@ -32,6 +32,9 @@ select count(*) = 0 as ok from pg_prepared_statements; -- See also prepared_xacts.sql select count(*) >= 0 as ok from pg_prepared_xacts; +-- 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%'; [parent: daf2e708edfb]