MIME-Version: 1.0
References: <CANtu0oh0si=jG5z_fLeFtmYcETssQ08kLEa8b6TQqDm_cinroA@mail.gmail.com>
 <20230814153634.GB1395427@nathanxps13> <CANtu0ohnGXW6wfM7eXts=m23A1g9D7s1X0ms7bV3-x5oQugX5g@mail.gmail.com>
 <20230815152224.GA2296544@nathanxps13>
In-Reply-To: <20230815152224.GA2296544@nathanxps13>
From: Michail Nikolaev <michail.nikolaev@gmail.com>
Date: Wed, 16 Aug 2023 17:30:59 +0200
Message-ID: <CANtu0ogdo86z-+oQVT7LENPPe_W4N5Q4HsisM_Xpf-H=v5vaow@mail.gmail.com>
Subject: Re: Replace known_assigned_xids_lck by memory barrier
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>
Content-Type: multipart/mixed; boundary="000000000000411a0f06030bfc68"
Archived-At: <https://www.postgresql.org/message-id/CANtu0ogdo86z-%2BoQVT7LENPPe_W4N5Q4HsisM_Xpf-H%3Dv5vaow%40mail.gmail.com>
Precedence: bulk

--000000000000411a0f06030bfc68
Content-Type: text/plain; charset="UTF-8"

Hello!

Updated version (with read barriers is attached).

> One remaining question I have is whether it is okay if we see an updated value
> for one of the head/tail variables but not the other.  It looks like the
> tail variable is only updated with ProcArrayLock held exclusively, which
> IIUC wouldn't prevent such mismatches even today, since we use a separate
> spinlock for reading them in some cases.

1) "The convention is that backends must hold shared ProcArrayLock to
examine the array", it is applied to pointers as well
2) Also, "only the startup process modifies the head/tail pointers."

So, the "tail" variable is updated by the startup process with
ProcArrayLock held in exclusive-only mode - so, no issues here.

Regarding "head" variable -  updates by the startup processes are
possible in next cases:
* ProcArrayLock in exclusive mode (like KnownAssignedXidsCompress or
KnownAssignedXidsSearch(remove=true)), no issues here
* ProcArrayLock not taken at all (like
KnownAssignedXidsAdd(exclusive_lock=false)) in such case we rely on
memory barrier machinery

Both head and tail variables are changed only with exclusive lock held.

I'll think more, but can't find something wrong here so far.

--000000000000411a0f06030bfc68
Content-Type: text/x-patch; charset="US-ASCII"; 
	name="v3-0001-Removes-known_assigned_xids_lck-using-the-write-m.patch"
Content-Disposition: attachment; 
	filename="v3-0001-Removes-known_assigned_xids_lck-using-the-write-m.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lldvl1ir0>
X-Attachment-Id: f_lldvl1ir0

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3N0b3JhZ2UvaXBjL3Byb2NhcnJheS5jIGIvc3JjL2Jh
Y2tlbmQvc3RvcmFnZS9pcGMvcHJvY2FycmF5LmMKaW5kZXggMmEzZGE0OWI4Zi4uZTQwOTNlZDA2
ZCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9pcGMvcHJvY2FycmF5LmMKKysrIGIv
c3JjL2JhY2tlbmQvc3RvcmFnZS9pcGMvcHJvY2FycmF5LmMKQEAgLTYxLDcgKzYxLDYgQEAKICNp
bmNsdWRlICJwb3J0L3BnX2xmaW5kLmgiCiAjaW5jbHVkZSAic3RvcmFnZS9wcm9jLmgiCiAjaW5j
bHVkZSAic3RvcmFnZS9wcm9jYXJyYXkuaCIKLSNpbmNsdWRlICJzdG9yYWdlL3NwaW4uaCIKICNp
bmNsdWRlICJ1dGlscy9hY2wuaCIKICNpbmNsdWRlICJ1dGlscy9idWlsdGlucy5oIgogI2luY2x1
ZGUgInV0aWxzL3JlbC5oIgpAQCAtODIsNyArODEsNiBAQCB0eXBlZGVmIHN0cnVjdCBQcm9jQXJy
YXlTdHJ1Y3QKIAlpbnQJCQludW1Lbm93bkFzc2lnbmVkWGlkczsJLyogY3VycmVudCAjIG9mIHZh
bGlkIGVudHJpZXMgKi8KIAlpbnQJCQl0YWlsS25vd25Bc3NpZ25lZFhpZHM7CS8qIGluZGV4IG9m
IG9sZGVzdCB2YWxpZCBlbGVtZW50ICovCiAJaW50CQkJaGVhZEtub3duQXNzaWduZWRYaWRzOwkv
KiBpbmRleCBvZiBuZXdlc3QgZWxlbWVudCwgKyAxICovCi0Jc2xvY2tfdAkJa25vd25fYXNzaWdu
ZWRfeGlkc19sY2s7CS8qIHByb3RlY3RzIGhlYWQvdGFpbCBwb2ludGVycyAqLwogCiAJLyoKIAkg
KiBIaWdoZXN0IHN1YnhpZCB0aGF0IGhhcyBiZWVuIHJlbW92ZWQgZnJvbSBLbm93bkFzc2lnbmVk
WGlkcyBhcnJheSB0bwpAQCAtNDQxLDcgKzQzOSw2IEBAIENyZWF0ZVNoYXJlZFByb2NBcnJheSh2
b2lkKQogCQlwcm9jQXJyYXktPm51bUtub3duQXNzaWduZWRYaWRzID0gMDsKIAkJcHJvY0FycmF5
LT50YWlsS25vd25Bc3NpZ25lZFhpZHMgPSAwOwogCQlwcm9jQXJyYXktPmhlYWRLbm93bkFzc2ln
bmVkWGlkcyA9IDA7Ci0JCVNwaW5Mb2NrSW5pdCgmcHJvY0FycmF5LT5rbm93bl9hc3NpZ25lZF94
aWRzX2xjayk7CiAJCXByb2NBcnJheS0+bGFzdE92ZXJmbG93ZWRYaWQgPSBJbnZhbGlkVHJhbnNh
Y3Rpb25JZDsKIAkJcHJvY0FycmF5LT5yZXBsaWNhdGlvbl9zbG90X3htaW4gPSBJbnZhbGlkVHJh
bnNhY3Rpb25JZDsKIAkJcHJvY0FycmF5LT5yZXBsaWNhdGlvbl9zbG90X2NhdGFsb2dfeG1pbiA9
IEludmFsaWRUcmFuc2FjdGlvbklkOwpAQCAtNDU2NCwxMSArNDU2MSwxMSBAQCBLbm93bkFzc2ln
bmVkVHJhbnNhY3Rpb25JZHNJZGxlTWFpbnRlbmFuY2Uodm9pZCkKICAqIHBvaW50ZXIuICBUaGlz
IHdvdWxkbid0IHJlcXVpcmUgYW55IGxvY2sgYXQgYWxsLCBleGNlcHQgdGhhdCBvbiBtYWNoaW5l
cwogICogd2l0aCB3ZWFrIG1lbW9yeSBvcmRlcmluZyB3ZSBuZWVkIHRvIGJlIGNhcmVmdWwgdGhh
dCBvdGhlciBwcm9jZXNzb3JzCiAgKiBzZWUgdGhlIGFycmF5IGVsZW1lbnQgY2hhbmdlcyBiZWZv
cmUgdGhleSBzZWUgdGhlIGhlYWQgcG9pbnRlciBjaGFuZ2UuCi0gKiBXZSBoYW5kbGUgdGhpcyBi
eSB1c2luZyBhIHNwaW5sb2NrIHRvIHByb3RlY3QgcmVhZHMgYW5kIHdyaXRlcyBvZiB0aGUKLSAq
IGhlYWQvdGFpbCBwb2ludGVycy4gIChXZSBjb3VsZCBkaXNwZW5zZSB3aXRoIHRoZSBzcGlubG9j
ayBpZiB3ZSB3ZXJlIHRvCi0gKiBjcmVhdGUgc3VpdGFibGUgbWVtb3J5IGFjY2VzcyBiYXJyaWVy
IHByaW1pdGl2ZXMgYW5kIHVzZSB0aG9zZSBpbnN0ZWFkLikKLSAqIFRoZSBzcGlubG9jayBtdXN0
IGJlIHRha2VuIHRvIHJlYWQgb3Igd3JpdGUgdGhlIGhlYWQvdGFpbCBwb2ludGVycyB1bmxlc3MK
LSAqIHRoZSBjYWxsZXIgaG9sZHMgUHJvY0FycmF5TG9jayBleGNsdXNpdmVseS4KKyAqIFdlIGhh
bmRsZSB0aGlzIGJ5IHVzaW5nIGEgbWVtb3J5IGJhcnJpZXIgdG8gcHJvdGVjdCB3cml0ZXMgb2Yg
dGhlCisgKiBoZWFkIHBvaW50ZXIuCisgKiBUaGUgbWVtb3J5IGJhcnJpZXIgaXMgdGFrZW4gYmVm
b3JlIHdyaXRlIHRoZSBoZWFkIHBvaW50ZXIgdW5sZXNzCisgKiB0aGUgY2FsbGVyIGhvbGRzIFBy
b2NBcnJheUxvY2sgZXhjbHVzaXZlbHkuIEFwcHJvcHJpYXRlIHJlYWQgbWVtb3J5IGJhcnJpZXIK
KyAqIGlzIHRha2VuIGFjY29yZGluZ2x5LgogICoKICAqIEFsZ29yaXRobWljIGFuYWx5c2lzOgog
ICoKQEAgLTQ3NTUsNyArNDc1Miw3IEBAIEtub3duQXNzaWduZWRYaWRzQWRkKFRyYW5zYWN0aW9u
SWQgZnJvbV94aWQsIFRyYW5zYWN0aW9uSWQgdG9feGlkLAogCiAJLyoKIAkgKiBTaW5jZSBvbmx5
IHRoZSBzdGFydHVwIHByb2Nlc3MgbW9kaWZpZXMgdGhlIGhlYWQvdGFpbCBwb2ludGVycywgd2UK
LQkgKiBkb24ndCBuZWVkIGEgbG9jayB0byByZWFkIHRoZW0gaGVyZS4KKwkgKiBhcmUgc2FmZSBy
ZWFkIHRoZW0gaGVyZS4KIAkgKi8KIAloZWFkID0gcEFycmF5LT5oZWFkS25vd25Bc3NpZ25lZFhp
ZHM7CiAJdGFpbCA9IHBBcnJheS0+dGFpbEtub3duQXNzaWduZWRYaWRzOwpAQCAtNDgwNiwyMiAr
NDgwMywxNyBAQCBLbm93bkFzc2lnbmVkWGlkc0FkZChUcmFuc2FjdGlvbklkIGZyb21feGlkLCBU
cmFuc2FjdGlvbklkIHRvX3hpZCwKIAlwQXJyYXktPm51bUtub3duQXNzaWduZWRYaWRzICs9IG54
aWRzOwogCiAJLyoKLQkgKiBOb3cgdXBkYXRlIHRoZSBoZWFkIHBvaW50ZXIuICBXZSB1c2UgYSBz
cGlubG9jayB0byBwcm90ZWN0IHRoaXMKKwkgKiBOb3cgdXBkYXRlIHRoZSBoZWFkIHBvaW50ZXIu
ICBXZSB1c2UgYSBtZW1vcnkgYmFycmllciB0byBwcm90ZWN0IHRoaXMKIAkgKiBwb2ludGVyLCBu
b3QgYmVjYXVzZSB0aGUgdXBkYXRlIGlzIGxpa2VseSB0byBiZSBub24tYXRvbWljLCBidXQgdG8K
IAkgKiBlbnN1cmUgdGhhdCBvdGhlciBwcm9jZXNzb3JzIHNlZSB0aGUgYWJvdmUgYXJyYXkgdXBk
YXRlcyBiZWZvcmUgdGhleQogCSAqIHNlZSB0aGUgaGVhZCBwb2ludGVyIGNoYW5nZS4KIAkgKgog
CSAqIElmIHdlJ3JlIGhvbGRpbmcgUHJvY0FycmF5TG9jayBleGNsdXNpdmVseSwgdGhlcmUncyBu
byBuZWVkIHRvIHRha2UgdGhlCi0JICogc3BpbmxvY2suCisJICogYmFycmllci4KIAkgKi8KLQlp
ZiAoZXhjbHVzaXZlX2xvY2spCi0JCXBBcnJheS0+aGVhZEtub3duQXNzaWduZWRYaWRzID0gaGVh
ZDsKLQllbHNlCi0JewotCQlTcGluTG9ja0FjcXVpcmUoJnBBcnJheS0+a25vd25fYXNzaWduZWRf
eGlkc19sY2spOwotCQlwQXJyYXktPmhlYWRLbm93bkFzc2lnbmVkWGlkcyA9IGhlYWQ7Ci0JCVNw
aW5Mb2NrUmVsZWFzZSgmcEFycmF5LT5rbm93bl9hc3NpZ25lZF94aWRzX2xjayk7Ci0JfQorCWlm
ICghZXhjbHVzaXZlX2xvY2spCisJCXBnX3dyaXRlX2JhcnJpZXIoKTsKKwlwQXJyYXktPmhlYWRL
bm93bkFzc2lnbmVkWGlkcyA9IGhlYWQ7CiB9CiAKIC8qCkBAIC00ODQzLDIwICs0ODM1LDEyIEBA
IEtub3duQXNzaWduZWRYaWRzU2VhcmNoKFRyYW5zYWN0aW9uSWQgeGlkLCBib29sIHJlbW92ZSkK
IAlpbnQJCQl0YWlsOwogCWludAkJCXJlc3VsdF9pbmRleCA9IC0xOwogCi0JaWYgKHJlbW92ZSkK
LQl7Ci0JCS8qIHdlIGhvbGQgUHJvY0FycmF5TG9jayBleGNsdXNpdmVseSwgc28gbm8gbmVlZCBm
b3Igc3BpbmxvY2sgKi8KLQkJdGFpbCA9IHBBcnJheS0+dGFpbEtub3duQXNzaWduZWRYaWRzOwot
CQloZWFkID0gcEFycmF5LT5oZWFkS25vd25Bc3NpZ25lZFhpZHM7Ci0JfQotCWVsc2UKLQl7Ci0J
CS8qIHRha2Ugc3BpbmxvY2sgdG8gZW5zdXJlIHdlIHNlZSB1cC10by1kYXRlIGFycmF5IGNvbnRl
bnRzICovCi0JCVNwaW5Mb2NrQWNxdWlyZSgmcEFycmF5LT5rbm93bl9hc3NpZ25lZF94aWRzX2xj
ayk7Ci0JCXRhaWwgPSBwQXJyYXktPnRhaWxLbm93bkFzc2lnbmVkWGlkczsKLQkJaGVhZCA9IHBB
cnJheS0+aGVhZEtub3duQXNzaWduZWRYaWRzOwotCQlTcGluTG9ja1JlbGVhc2UoJnBBcnJheS0+
a25vd25fYXNzaWduZWRfeGlkc19sY2spOwotCX0KKwl0YWlsID0gcEFycmF5LT50YWlsS25vd25B
c3NpZ25lZFhpZHM7CisJaGVhZCA9IHBBcnJheS0+aGVhZEtub3duQXNzaWduZWRYaWRzOworCisJ
LyogaW4gY2FzZSBvZiByZW1vdmUgd2UgaG9sZCBQcm9jQXJyYXlMb2NrIGV4Y2x1c2l2ZWx5LCBz
byBubyBuZWVkIGZvciBiYXJyaWVyICovCisJaWYgKCFyZW1vdmUpCisJCXBnX3JlYWRfYmFycmll
cigpOwogCiAJLyoKIAkgKiBTdGFuZGFyZCBiaW5hcnkgc2VhcmNoLiAgTm90ZSB3ZSBjYW4gaWdu
b3JlIHRoZSBLbm93bkFzc2lnbmVkWGlkc1ZhbGlkCkBAIC01MDk1LDEyICs1MDc5LDExIEBAIEtu
b3duQXNzaWduZWRYaWRzR2V0QW5kU2V0WG1pbihUcmFuc2FjdGlvbklkICp4YXJyYXksIFRyYW5z
YWN0aW9uSWQgKnhtaW4sCiAJICogbWlnaHQgbWlzcyBuZXdseS1hZGRlZCB4aWRzLCBidXQgdGhl
eSBzaG91bGQgYmUgPj0geG1heCBzbyBpcnJlbGV2YW50CiAJICogYW55d2F5LgogCSAqCi0JICog
TXVzdCB0YWtlIHNwaW5sb2NrIHRvIGVuc3VyZSB3ZSBzZWUgdXAtdG8tZGF0ZSBhcnJheSBjb250
ZW50cy4KKwkgKiBNdXN0IHRha2UgcmVhZCBtZW1vcnkgYmFycmllciB0byBlbnN1cmUgd2Ugc2Vl
IHVwLXRvLWRhdGUgYXJyYXkgY29udGVudHMuCiAJICovCi0JU3BpbkxvY2tBY3F1aXJlKCZwcm9j
QXJyYXktPmtub3duX2Fzc2lnbmVkX3hpZHNfbGNrKTsKIAl0YWlsID0gcHJvY0FycmF5LT50YWls
S25vd25Bc3NpZ25lZFhpZHM7CiAJaGVhZCA9IHByb2NBcnJheS0+aGVhZEtub3duQXNzaWduZWRY
aWRzOwotCVNwaW5Mb2NrUmVsZWFzZSgmcHJvY0FycmF5LT5rbm93bl9hc3NpZ25lZF94aWRzX2xj
ayk7CisJcGdfcmVhZF9iYXJyaWVyKCk7CiAKIAlmb3IgKGkgPSB0YWlsOyBpIDwgaGVhZDsgaSsr
KQogCXsKQEAgLTUxNDcsMTAgKzUxMzAsOSBAQCBLbm93bkFzc2lnbmVkWGlkc0dldE9sZGVzdFht
aW4odm9pZCkKIAkvKgogCSAqIEZldGNoIGhlYWQganVzdCBvbmNlLCBzaW5jZSBpdCBtYXkgY2hh
bmdlIHdoaWxlIHdlIGxvb3AuCiAJICovCi0JU3BpbkxvY2tBY3F1aXJlKCZwcm9jQXJyYXktPmtu
b3duX2Fzc2lnbmVkX3hpZHNfbGNrKTsKIAl0YWlsID0gcHJvY0FycmF5LT50YWlsS25vd25Bc3Np
Z25lZFhpZHM7CiAJaGVhZCA9IHByb2NBcnJheS0+aGVhZEtub3duQXNzaWduZWRYaWRzOwotCVNw
aW5Mb2NrUmVsZWFzZSgmcHJvY0FycmF5LT5rbm93bl9hc3NpZ25lZF94aWRzX2xjayk7CisJcGdf
cmVhZF9iYXJyaWVyKCk7CiAKIAlmb3IgKGkgPSB0YWlsOyBpIDwgaGVhZDsgaSsrKQogCXsK
--000000000000411a0f06030bfc68--