Subject: Re: ERROR: invalid spinlock number: 0
To: Andres Freund <andres@anarazel.de>, Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@gmail.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <7ef8708c-5b6b-edd3-2cf2-7783f1c7c175@oss.nttdata.com>
 <YCUpTL8bZHa+9K+0@paquier.xyz>
 <79698f73-7dd7-d9e3-5322-4032b4428653@oss.nttdata.com>
 <YCowabyl1dwan+JP@paquier.xyz>
 <CA+hUKG+tZHpADD_BsPp=c3EiYeenTTx5AxsZVGkv1mA0J3O21w@mail.gmail.com>
 <YCpQwdyXErcTRhVX@paquier.xyz>
 <20210215212805.dfahbpqqpnl6lgwm@alap3.anarazel.de>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <918a47fc-e1fa-0c99-15e3-59b520455fe0@oss.nttdata.com>
Date: Tue, 16 Feb 2021 12:43:42 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <20210215212805.dfahbpqqpnl6lgwm@alap3.anarazel.de>
Content-Type: multipart/mixed;
 boundary="------------1FEDCCD420B6323727731E10"
Content-Language: en-US
Precedence: bulk

This is a multi-part message in MIME format.
--------------1FEDCCD420B6323727731E10
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit


On 2021/02/16 6:28, Andres Freund wrote:
> Hi,
> 
> On 2021-02-15 19:45:21 +0900, Michael Paquier wrote:
>> On Mon, Feb 15, 2021 at 10:47:05PM +1300, Thomas Munro wrote:
>>> Why not initialise it in WalRcvShmemInit()?
>>
>> I was thinking about doing that as well, but we have no real need to
>> initialize this stuff in most cases, say standalone deployments.  In
>> particular for the fallback implementation of atomics, we would
>> prepare a spinlock for nothing.
> 
> So what? It's just about free to initialize a spinlock, whether it's
> using the fallback implementation or not. Initializing upon walsender
> startup adds a lot of complications, because e.g. somebody could already
> hold the spinlock because the previous walsender just disconnected, and
> they were looking at the stats.

Even if we initialize "writtenUpto" in WalRcvShmemInit(), WalReceiverMain()
still needs to initialize (reset to 0) by using pg_atomic_write_u64().

Basically we should not acquire new spinlock while holding another spinlock,
to shorten the spinlock duration. Right? If yes, we need to move
pg_atomic_read_u64() of "writtenUpto" after the release of spinlock in
pg_stat_get_wal_receiver.

Attached is the updated version of the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

--------------1FEDCCD420B6323727731E10
Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0";
 name="bugfix_pg_stat_wal_receiver_v2.patch"
Content-Transfer-Encoding: base64
Content-Disposition: attachment;
 filename="bugfix_pg_stat_wal_receiver_v2.patch"

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHJlY2VpdmVyLmMgYi9z
cmMvYmFja2VuZC9yZXBsaWNhdGlvbi93YWxyZWNlaXZlci5jCmluZGV4IDcyM2Y1MTNkOGIu
LjMxNjY0MGMyNzUgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHJl
Y2VpdmVyLmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vd2FscmVjZWl2ZXIuYwpA
QCAtMjQ5LDcgKzI0OSw3IEBAIFdhbFJlY2VpdmVyTWFpbih2b2lkKQogCiAJU3BpbkxvY2tS
ZWxlYXNlKCZ3YWxyY3YtPm11dGV4KTsKIAotCXBnX2F0b21pY19pbml0X3U2NCgmV2FsUmN2
LT53cml0dGVuVXB0bywgMCk7CisJcGdfYXRvbWljX3dyaXRlX3U2NCgmV2FsUmN2LT53cml0
dGVuVXB0bywgMCk7CiAKIAkvKiBBcnJhbmdlIHRvIGNsZWFuIHVwIGF0IHdhbHJlY2VpdmVy
IGV4aXQgKi8KIAlvbl9zaG1lbV9leGl0KFdhbFJjdkRpZSwgMCk7CkBAIC0xMzI1LDcgKzEz
MjUsNiBAQCBwZ19zdGF0X2dldF93YWxfcmVjZWl2ZXIoUEdfRlVOQ1RJT05fQVJHUykKIAlz
dGF0ZSA9IFdhbFJjdi0+d2FsUmN2U3RhdGU7CiAJcmVjZWl2ZV9zdGFydF9sc24gPSBXYWxS
Y3YtPnJlY2VpdmVTdGFydDsKIAlyZWNlaXZlX3N0YXJ0X3RsaSA9IFdhbFJjdi0+cmVjZWl2
ZVN0YXJ0VExJOwotCXdyaXR0ZW5fbHNuID0gcGdfYXRvbWljX3JlYWRfdTY0KCZXYWxSY3Yt
PndyaXR0ZW5VcHRvKTsKIAlmbHVzaGVkX2xzbiA9IFdhbFJjdi0+Zmx1c2hlZFVwdG87CiAJ
cmVjZWl2ZWRfdGxpID0gV2FsUmN2LT5yZWNlaXZlZFRMSTsKIAlsYXN0X3NlbmRfdGltZSA9
IFdhbFJjdi0+bGFzdE1zZ1NlbmRUaW1lOwpAQCAtMTMzOCw2ICsxMzM3LDE0IEBAIHBnX3N0
YXRfZ2V0X3dhbF9yZWNlaXZlcihQR19GVU5DVElPTl9BUkdTKQogCXN0cmxjcHkoY29ubmlu
Zm8sIChjaGFyICopIFdhbFJjdi0+Y29ubmluZm8sIHNpemVvZihjb25uaW5mbykpOwogCVNw
aW5Mb2NrUmVsZWFzZSgmV2FsUmN2LT5tdXRleCk7CiAKKwkvKgorCSAqIFJlYWQgIndyaXR0
ZW5VcHRvIiB3aXRob3V0IGhvbGRpbmcgYSBzcGlubG9jay4gU28gaXQgbWF5IG5vdCBiZQor
CSAqIGNvbnNpc3RlbnQgd2l0aCBvdGhlciBXQUwgcmVjZWl2ZXIncyBzaGFyZWQgdmFyaWFi
bGVzIHByb3RlY3RlZCBieSBhCisJICogc3BpbmxvY2suIFRoaXMgaXMgT0sgYmVjYXVzZSB0
aGF0IHZhcmlhYmxlIGlzIHVzZWQgb25seSBmb3IKKwkgKiBpbmZvcm1hdGlvbmFsIHB1cnBv
c2UgYW5kIHNob3VsZCBub3QgYmUgdXNlZCBmb3IgZGF0YSBpbnRlZ3JpdHkgY2hlY2tzLgor
CSAqLworCXdyaXR0ZW5fbHNuID0gcGdfYXRvbWljX3JlYWRfdTY0KCZXYWxSY3YtPndyaXR0
ZW5VcHRvKTsKKwogCS8qCiAJICogTm8gV0FMIHJlY2VpdmVyIChvciBub3QgcmVhZHkgeWV0
KSwganVzdCByZXR1cm4gYSB0dXBsZSB3aXRoIE5VTEwKIAkgKiB2YWx1ZXMKZGlmZiAtLWdp
dCBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHJlY2VpdmVyZnVuY3MuYyBiL3NyYy9i
YWNrZW5kL3JlcGxpY2F0aW9uL3dhbHJlY2VpdmVyZnVuY3MuYwppbmRleCA2OWI5MWE3ZGFi
Li42M2U2MDQ3OGVhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi93YWxy
ZWNlaXZlcmZ1bmNzLmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vd2FscmVjZWl2
ZXJmdW5jcy5jCkBAIC02Myw2ICs2Myw3IEBAIFdhbFJjdlNobWVtSW5pdCh2b2lkKQogCQlN
ZW1TZXQoV2FsUmN2LCAwLCBXYWxSY3ZTaG1lbVNpemUoKSk7CiAJCVdhbFJjdi0+d2FsUmN2
U3RhdGUgPSBXQUxSQ1ZfU1RPUFBFRDsKIAkJU3BpbkxvY2tJbml0KCZXYWxSY3YtPm11dGV4
KTsKKwkJcGdfYXRvbWljX2luaXRfdTY0KCZXYWxSY3YtPndyaXR0ZW5VcHRvLCAwKTsKIAkJ
V2FsUmN2LT5sYXRjaCA9IE5VTEw7CiAJfQogfQpkaWZmIC0tZ2l0IGEvc3JjL3Rlc3QvcmVn
cmVzcy9leHBlY3RlZC9zeXN2aWV3cy5vdXQgYi9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVk
L3N5c3ZpZXdzLm91dAppbmRleCA4MWJkYWNmNTlkLi42ZDA0OGUzMDljIDEwMDY0NAotLS0g
YS9zcmMvdGVzdC9yZWdyZXNzL2V4cGVjdGVkL3N5c3ZpZXdzLm91dAorKysgYi9zcmMvdGVz
dC9yZWdyZXNzL2V4cGVjdGVkL3N5c3ZpZXdzLm91dApAQCAtODMsNiArODMsMTMgQEAgc2Vs
ZWN0IGNvdW50KCopID0gMSBhcyBvayBmcm9tIHBnX3N0YXRfd2FsOwogIHQKICgxIHJvdykK
IAorLS0gV2UgZXhwZWN0IG5vIHdhbHJlY2VpdmVyIHJ1bm5pbmcgaW4gdGhpcyB0ZXN0Citz
ZWxlY3QgY291bnQoKikgPSAwIGFzIG9rIGZyb20gcGdfc3RhdF93YWxfcmVjZWl2ZXI7Cisg
b2sgCistLS0tCisgdAorKDEgcm93KQorCiAtLSBUaGlzIGlzIHRvIHJlY29yZCB0aGUgcHJl
dmFpbGluZyBwbGFubmVyIGVuYWJsZV9mb28gc2V0dGluZ3MgZHVyaW5nCiAtLSBhIHJlZ3Jl
c3Npb24gdGVzdCBydW4uCiBzZWxlY3QgbmFtZSwgc2V0dGluZyBmcm9tIHBnX3NldHRpbmdz
IHdoZXJlIG5hbWUgbGlrZSAnZW5hYmxlJSc7CmRpZmYgLS1naXQgYS9zcmMvdGVzdC9yZWdy
ZXNzL3NxbC9zeXN2aWV3cy5zcWwgYi9zcmMvdGVzdC9yZWdyZXNzL3NxbC9zeXN2aWV3cy5z
cWwKaW5kZXggYjliODc1YmM2YS4uZGM4YzlhM2FjMiAxMDA2NDQKLS0tIGEvc3JjL3Rlc3Qv
cmVncmVzcy9zcWwvc3lzdmlld3Muc3FsCisrKyBiL3NyYy90ZXN0L3JlZ3Jlc3Mvc3FsL3N5
c3ZpZXdzLnNxbApAQCAtNDAsNiArNDAsOSBAQCBzZWxlY3QgY291bnQoKikgPj0gMCBhcyBv
ayBmcm9tIHBnX3ByZXBhcmVkX3hhY3RzOwogLS0gVGhlcmUgbXVzdCBiZSBvbmx5IG9uZSBy
ZWNvcmQKIHNlbGVjdCBjb3VudCgqKSA9IDEgYXMgb2sgZnJvbSBwZ19zdGF0X3dhbDsKIAor
LS0gV2UgZXhwZWN0IG5vIHdhbHJlY2VpdmVyIHJ1bm5pbmcgaW4gdGhpcyB0ZXN0CitzZWxl
Y3QgY291bnQoKikgPSAwIGFzIG9rIGZyb20gcGdfc3RhdF93YWxfcmVjZWl2ZXI7CisKIC0t
IFRoaXMgaXMgdG8gcmVjb3JkIHRoZSBwcmV2YWlsaW5nIHBsYW5uZXIgZW5hYmxlX2ZvbyBz
ZXR0aW5ncyBkdXJpbmcKIC0tIGEgcmVncmVzc2lvbiB0ZXN0IHJ1bi4KIHNlbGVjdCBuYW1l
LCBzZXR0aW5nIGZyb20gcGdfc2V0dGluZ3Mgd2hlcmUgbmFtZSBsaWtlICdlbmFibGUlJzsK

--------------1FEDCCD420B6323727731E10--