I/O worker and ConfigReload
Hi,
I've been rebasing the patch for online resizing of shared memory, and
noticed something strange about IoWorkerMain: although it sets the
handler SignalHandlerForConfigReload, it doesn't look like it acts upon
ConfigReloadPending. From what I see it happens because it only does
CHECK_FOR_INTERRUPTS in the main worker loop, which doesn't handle
ConfigReloadPending.
In the context of shared memory resizing patch it means I/O workers are
not receiving the new value of NBuffers and crash. Adding something like
pgaio_worker_process_interrupts to deal with ConfigReloadPending at the
beginning of the main worker loop seems to solve the issue. But I
haven't found any discussion about config reload in I/O workers, was
this omission intentional?
On Sun, May 25, 2025 at 1:56 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
I've been rebasing the patch for online resizing of shared memory, and
noticed something strange about IoWorkerMain: although it sets the
handler SignalHandlerForConfigReload, it doesn't look like it acts upon
ConfigReloadPending. From what I see it happens because it only does
CHECK_FOR_INTERRUPTS in the main worker loop, which doesn't handle
ConfigReloadPending.In the context of shared memory resizing patch it means I/O workers are
not receiving the new value of NBuffers and crash. Adding something like
pgaio_worker_process_interrupts to deal with ConfigReloadPending at the
beginning of the main worker loop seems to solve the issue. But I
haven't found any discussion about config reload in I/O workers, was
this omission intentional?
You're right, and I noticed the same thing and fixed it in
0004-aio-Adjust-IO-worker-pool-size-automatically.patch in
/messages/by-id/CA+hUKG+m4xV0LMoH2c=oRAdEXuCnh+tGBTWa7uFeFMGgTLAw+Q@mail.gmail.com.
(That patch also introduces a reason for the workers to care about
config reload.). Not sure how that happened, ie the history of
adding/removing reasons to worry about reload, I don't remember, this
code stewed for a long time, but it's at least inconsistent... We
could fix it as you suggested or as shown in that patch.
On Sun, May 25, 2025 at 02:15:12AM GMT, Thomas Munro wrote:
On Sun, May 25, 2025 at 1:56 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:I've been rebasing the patch for online resizing of shared memory, and
noticed something strange about IoWorkerMain: although it sets the
handler SignalHandlerForConfigReload, it doesn't look like it acts upon
ConfigReloadPending. From what I see it happens because it only does
CHECK_FOR_INTERRUPTS in the main worker loop, which doesn't handle
ConfigReloadPending.In the context of shared memory resizing patch it means I/O workers are
not receiving the new value of NBuffers and crash. Adding something like
pgaio_worker_process_interrupts to deal with ConfigReloadPending at the
beginning of the main worker loop seems to solve the issue. But I
haven't found any discussion about config reload in I/O workers, was
this omission intentional?You're right, and I noticed the same thing and fixed it in
0004-aio-Adjust-IO-worker-pool-size-automatically.patch in
/messages/by-id/CA+hUKG+m4xV0LMoH2c=oRAdEXuCnh+tGBTWa7uFeFMGgTLAw+Q@mail.gmail.com.
(That patch also introduces a reason for the workers to care about
config reload.). Not sure how that happened, ie the history of
adding/removing reasons to worry about reload, I don't remember, this
code stewed for a long time, but it's at least inconsistent... We
could fix it as you suggested or as shown in that patch.
I see thanks. Indeed, there isn't much difference between what I had in
mind and the relevant bits in 0004, so probably it's the way to go.