MIME-Version: 1.0
References: 
 <CALj2ACWd5apddj6Cd885WwJ6LquYu_G81C4GoR4xSoDV1x-FEA@mail.gmail.com>
 <b5eab65a-e11b-6435-b6aa-dac206e17d38@gmail.com>
In-Reply-To: <b5eab65a-e11b-6435-b6aa-dac206e17d38@gmail.com>
From: Fujii Masao <masao.fujii@gmail.com>
Date: Mon, 30 Mar 2026 09:30:08 +0900
Message-ID: 
 <CAHGQGwFBnBQZbt4e9qjZoiM0GW2eXCvS=Ny5wt+s1-+iJpK15A@mail.gmail.com>
Subject: Re: Avoid multiple SetLatch() calls in procsignal_sigusr1_handler()
To: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: multipart/mixed; boundary="000000000000bcddd7064e32f460"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHGQGwFBnBQZbt4e9qjZoiM0GW2eXCvS%3DNy5wt%2Bs1-%2BiJpK15A%40mail.gmail.com>
Precedence: bulk

--000000000000bcddd7064e32f460
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Mar 9, 2023 at 9:24=E2=80=AFPM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On 2/28/23 4:30 PM, Bharath Rupireddy wrote:
> > Hi,
> >
> > Most of the multiplexed SIGUSR1 handlers are setting latch explicitly
> > when the procsignal_sigusr1_handler() can do that for them at the end.
>
> Right.
>
> > These multiplexed handlers are currently being used as SIGUSR1
> > handlers, not as independent handlers, so no problem if SetLatch() is
> > removed from them.
>
> Agree, they are only used in procsignal_sigusr1_handler().
>
> > A few others do it right by saying /* latch will be
> > set by procsignal_sigusr1_handler */.
>
> Yeap, so do HandleProcSignalBarrierInterrupt() and HandleLogMemoryContext=
Interrupt().
>
> > Although, calling SetLatch() in
> > quick succession does no harm (it just returns if the latch was
> > previously set), it seems unnecessary.
> >
>
> Agree.

While reviewing the patch in [1], I noticed this issue and ended up here.
I agree with the approach and have attached a revised version of the patch.


> > I'm attaching a patch that avoids multiple SetLatch() calls.
> >
> > Thoughts?
> >
>
> I agree with the idea behind the patch. The thing
> that worry me a bit is that the changed functions are defined
> as external and so may produce an impact outside of core pg and I'm
> not sure that's worth it.

I understand the concern. There's no guarantee that PostgreSQL functions
behave identically across major versions, so removing redundant SetLatch()
calls is generally fine. However, as you are concerned, extensions might ca=
ll
these functions and implicitly rely on the extra SetLatch(). Since the patc=
h
doesn't change the API, such behavioral changes may be hard for extension
authors to notice. Also they will be not in release notes. In practice,
they would probably catch this during testing against a new major version,
though.

I guess similar situations have come up in past major upgrades, so perhaps
I'm overthinking this??

Regards,

[1] https://postgr.es/m/CABdArM6pmn5yFqiU33KTYBXYM=3DVny2ULnJY_gqFbsMEdt+1d=
PA@mail.gmail.com

--=20
Fujii Masao

--000000000000bcddd7064e32f460
Content-Type: application/octet-stream;
	name="v2-0001-Remove-redundant-SetLatch-calls-in-interrupt-hand.patch"
Content-Disposition: attachment;
	filename="v2-0001-Remove-redundant-SetLatch-calls-in-interrupt-hand.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mncgbfpu0>
X-Attachment-Id: f_mncgbfpu0

RnJvbSAyMWQ4ZTM2ZjUxZjFjOTU1OTY4NDU5NzljNDQ3NDBkMzlkZjc0ZWVlIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBGdWppaSBNYXNhbyA8ZnVqaWlAcG9zdGdyZXNxbC5vcmc+CkRh
dGU6IE1vbiwgMzAgTWFyIDIwMjYgMDg6NDg6MDUgKzA5MDAKU3ViamVjdDogW1BBVENIIHYyXSBS
ZW1vdmUgcmVkdW5kYW50IFNldExhdGNoKCkgY2FsbHMgaW4gaW50ZXJydXB0IGhhbmRsaW5nCiBm
dW5jdGlvbnMKCkludGVycnVwdCBoYW5kbGluZyBmdW5jdGlvbnMgKGUuZy4sIEhhbmRsZUNhdGNo
dXBJbnRlcnJ1cHQoKSwKSGFuZGxlUGFyYWxsZWxBcHBseU1lc3NhZ2VJbnRlcnJ1cHQoKSkgYXJl
IGNhbGxlZCBvbmx5IGJ5CnByb2NzaWduYWxfc2lndXNyMV9oYW5kbGVyKCksIHdoaWNoIGFscmVh
ZHkgY2FsbHMgU2V0TGF0Y2goKQpmb3IgdGhlIGN1cnJlbnQgcHJvY2VzcyBhdCB0aGUgZW5kIG9m
IGl0cyBwcm9jZXNzaW5nLgpUaGVyZWZvcmUsIHRoZXNlIGludGVycnVwdCBoYW5kbGluZyBmdW5j
dGlvbnMgZG8gbm90IG5lZWQgdG8KY2FsbCBTZXRMYXRjaCgpIHRoZW1zZWx2ZXMuCgpIb3dldmVy
LCBwcmV2aW91c2x5LCBzb21lIG9mIHRoZXNlIGZ1bmN0aW9ucyByZWR1bmRhbnRseQpjYWxsZWQg
U2V0TGF0Y2goKS4gVGhpcyBjb21taXQgcmVtb3ZlcyB0aG9zZSB1bm5lY2Vzc2FyeQpjYWxscy4K
CldoaWxlIGR1cGxpY2F0ZSBTZXRMYXRjaCgpIGNhbGxzIGFyZSByZWR1bmRhbnQsIHRoZXkgYXJl
Cmhhcm1sZXNzLCBzbyB0aGlzIGNoYW5nZSBpcyBub3QgYmFja3BhdGNoZWQuCgpBdXRob3I6IEJo
YXJhdGggUnVwaXJlZGR5IDxiaGFyYXRoLnJ1cGlyZWRkeWZvcnBvc3RncmVzQGdtYWlsLmNvbT4K
UmV2aWV3ZWQtYnk6IEJlcnRyYW5kIERyb3V2b3QgPGJlcnRyYW5kZHJvdXZvdC5wZ0BnbWFpbC5j
b20+ClJldmlld2VkLWJ5OiBGdWppaSBNYXNhbyA8bWFzYW8uZnVqaWlAZ21haWwuY29tPgpEaXNj
dXNzaW9uOiBodHRwczovL3Bvc3Rnci5lcy9tL0NBTGoyQUNXZDVhcGRkajZDZDg4NVd3SjZMcXVZ
dV9HODFDNEdvUjR4U29EVjF4LUZFQUBtYWlsLmdtYWlsLmNvbQotLS0KIHNyYy9iYWNrZW5kL2Fj
Y2Vzcy90cmFuc2FtL3BhcmFsbGVsLmMgICAgICAgICAgICAgICAgIHwgMiArLQogc3JjL2JhY2tl
bmQvY29tbWFuZHMvYXN5bmMuYyAgICAgICAgICAgICAgICAgICAgICAgICAgfCAzICstLQogc3Jj
L2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9hcHBseXBhcmFsbGVsd29ya2VyLmMgfCAyICst
CiBzcmMvYmFja2VuZC9yZXBsaWNhdGlvbi93YWxzZW5kZXIuYyAgICAgICAgICAgICAgICAgICB8
IDIgKysKIHNyYy9iYWNrZW5kL3N0b3JhZ2UvaXBjL3NpbnZhbC5jICAgICAgICAgICAgICAgICAg
ICAgIHwgMyArLS0KIDUgZmlsZXMgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspLCA2IGRlbGV0aW9u
cygtKQoKZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2FtL3BhcmFsbGVsLmMg
Yi9zcmMvYmFja2VuZC9hY2Nlc3MvdHJhbnNhbS9wYXJhbGxlbC5jCmluZGV4IGFiMWRmYjMwZTcz
Li44OWU5ZDIyNGVlYyAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0vcGFy
YWxsZWwuYworKysgYi9zcmMvYmFja2VuZC9hY2Nlc3MvdHJhbnNhbS9wYXJhbGxlbC5jCkBAIC0x
MDQ3LDcgKzEwNDcsNyBAQCBIYW5kbGVQYXJhbGxlbE1lc3NhZ2VJbnRlcnJ1cHQodm9pZCkKIHsK
IAlJbnRlcnJ1cHRQZW5kaW5nID0gdHJ1ZTsKIAlQYXJhbGxlbE1lc3NhZ2VQZW5kaW5nID0gdHJ1
ZTsKLQlTZXRMYXRjaChNeUxhdGNoKTsKKwkvKiBsYXRjaCB3aWxsIGJlIHNldCBieSBwcm9jc2ln
bmFsX3NpZ3VzcjFfaGFuZGxlciAqLwogfQogCiAvKgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQv
Y29tbWFuZHMvYXN5bmMuYyBiL3NyYy9iYWNrZW5kL2NvbW1hbmRzL2FzeW5jLmMKaW5kZXggNWM5
YTU2YzNkNDAuLmU5MWE2MmZmNDJhIDEwMDY0NAotLS0gYS9zcmMvYmFja2VuZC9jb21tYW5kcy9h
c3luYy5jCisrKyBiL3NyYy9iYWNrZW5kL2NvbW1hbmRzL2FzeW5jLmMKQEAgLTI1NjAsOCArMjU2
MCw3IEBAIEhhbmRsZU5vdGlmeUludGVycnVwdCh2b2lkKQogCS8qIHNpZ25hbCB0aGF0IHdvcmsg
bmVlZHMgdG8gYmUgZG9uZSAqLwogCW5vdGlmeUludGVycnVwdFBlbmRpbmcgPSB0cnVlOwogCi0J
LyogbWFrZSBzdXJlIHRoZSBldmVudCBpcyBwcm9jZXNzZWQgaW4gZHVlIGNvdXJzZSAqLwotCVNl
dExhdGNoKE15TGF0Y2gpOworCS8qIGxhdGNoIHdpbGwgYmUgc2V0IGJ5IHByb2NzaWduYWxfc2ln
dXNyMV9oYW5kbGVyICovCiB9CiAKIC8qCmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9yZXBsaWNh
dGlvbi9sb2dpY2FsL2FwcGx5cGFyYWxsZWx3b3JrZXIuYyBiL3NyYy9iYWNrZW5kL3JlcGxpY2F0
aW9uL2xvZ2ljYWwvYXBwbHlwYXJhbGxlbHdvcmtlci5jCmluZGV4IGQ3ODY5M2ZmYThlLi43OThl
OGQ4NWIzZSAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9hcHBs
eXBhcmFsbGVsd29ya2VyLmMKKysrIGIvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9h
cHBseXBhcmFsbGVsd29ya2VyLmMKQEAgLTEwMDAsNyArMTAwMCw3IEBAIEhhbmRsZVBhcmFsbGVs
QXBwbHlNZXNzYWdlSW50ZXJydXB0KHZvaWQpCiB7CiAJSW50ZXJydXB0UGVuZGluZyA9IHRydWU7
CiAJUGFyYWxsZWxBcHBseU1lc3NhZ2VQZW5kaW5nID0gdHJ1ZTsKLQlTZXRMYXRjaChNeUxhdGNo
KTsKKwkvKiBsYXRjaCB3aWxsIGJlIHNldCBieSBwcm9jc2lnbmFsX3NpZ3VzcjFfaGFuZGxlciAq
LwogfQogCiAvKgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vd2Fsc2VuZGVy
LmMgYi9zcmMvYmFja2VuZC9yZXBsaWNhdGlvbi93YWxzZW5kZXIuYwppbmRleCA2NjUwN2U5YzJk
ZC4uMmJiM2YzNGRjNmQgMTAwNjQ0Ci0tLSBhL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHNl
bmRlci5jCisrKyBiL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL3dhbHNlbmRlci5jCkBAIC0zNzMw
LDYgKzM3MzAsOCBAQCBIYW5kbGVXYWxTbmRJbml0U3RvcHBpbmcodm9pZCkKIAkJa2lsbChNeVBy
b2NQaWQsIFNJR1RFUk0pOwogCWVsc2UKIAkJZ290X1NUT1BQSU5HID0gdHJ1ZTsKKworCS8qIGxh
dGNoIHdpbGwgYmUgc2V0IGJ5IHByb2NzaWduYWxfc2lndXNyMV9oYW5kbGVyICovCiB9CiAKIC8q
CmRpZmYgLS1naXQgYS9zcmMvYmFja2VuZC9zdG9yYWdlL2lwYy9zaW52YWwuYyBiL3NyYy9iYWNr
ZW5kL3N0b3JhZ2UvaXBjL3NpbnZhbC5jCmluZGV4IDU1NTlmN2MxY2ZhLi4xNTQwYzdlMDY5NiAx
MDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvc3RvcmFnZS9pcGMvc2ludmFsLmMKKysrIGIvc3JjL2Jh
Y2tlbmQvc3RvcmFnZS9pcGMvc2ludmFsLmMKQEAgLTE2MCw4ICsxNjAsNyBAQCBIYW5kbGVDYXRj
aHVwSW50ZXJydXB0KHZvaWQpCiAKIAljYXRjaHVwSW50ZXJydXB0UGVuZGluZyA9IHRydWU7CiAK
LQkvKiBtYWtlIHN1cmUgdGhlIGV2ZW50IGlzIHByb2Nlc3NlZCBpbiBkdWUgY291cnNlICovCi0J
U2V0TGF0Y2goTXlMYXRjaCk7CisJLyogbGF0Y2ggd2lsbCBiZSBzZXQgYnkgcHJvY3NpZ25hbF9z
aWd1c3IxX2hhbmRsZXIgKi8KIH0KIAogLyoKLS0gCjIuNTEuMgoK
--000000000000bcddd7064e32f460--