Обсуждение: Shmem queue is not flushed if receiver is not yet attached
Hello,
While testing on the current PG master, I noticed a problem between backends communicating over a shared memory queue. I think `shm_mq_sendv()` fails to flush the queue, even if `force_flush` is set to true, if the receiver is not yet attached to the queue. This simple fix solves the problem for me.
On another note, `shm_mq.h` declares `shm_mq_flush()`, but I don't see it being implemented. Maybe just a leftover from the previous work? Though it seems useful to implement that API.
Thanks,
Вложения
On Thu, Mar 17, 2022 at 3:13 AM Pavan Deolasee <pavan.deolasee@gmail.com> wrote: > While testing on the current PG master, I noticed a problem between backends communicating over a shared memory queue.I think `shm_mq_sendv()` fails to flush the queue, even if `force_flush` is set to true, if the receiver is not yetattached to the queue. This simple fix solves the problem for me. > > On another note, `shm_mq.h` declares `shm_mq_flush()`, but I don't see it being implemented. Maybe just a leftover fromthe previous work? Though it seems useful to implement that API. I think that this patch is basically correct, except that it's not correct to set mqh_counterparty_attached when receiver is still NULL. Here's a v2 where I've attempted to correct that while preserving the essence of your proposed fix. I'm not sure that we need a shm_mq_flush(), but we definitely don't have one currently, so I've also adjusted your patch to remove the dead prototype. Please let me know your thoughts on the attached. Thanks, -- Robert Haas EDB: http://www.enterprisedb.com
Вложения
On Tue, 24 May 2022 at 23:05, Robert Haas <robertmhaas@gmail.com> wrote: > On Thu, Mar 17, 2022 at 3:13 AM Pavan Deolasee <pavan.deolasee@gmail.com> wrote: >> While testing on the current PG master, I noticed a problem between backends communicating over a shared memory queue.I think `shm_mq_sendv()` fails to flush the queue, even if `force_flush` is set to true, if the receiver is not yetattached to the queue. This simple fix solves the problem for me. >> >> On another note, `shm_mq.h` declares `shm_mq_flush()`, but I don't see it being implemented. Maybe just a leftover fromthe previous work? Though it seems useful to implement that API. > > I think that this patch is basically correct, except that it's not > correct to set mqh_counterparty_attached when receiver is still NULL. > Here's a v2 where I've attempted to correct that while preserving the > essence of your proposed fix. > > I'm not sure that we need a shm_mq_flush(), but we definitely don't > have one currently, so I've also adjusted your patch to remove the > dead prototype. > > Please let me know your thoughts on the attached. > > Thanks, Hi, I have a problem that is also related to shmem queue [1], however, I cannot reproduce it. How did you reproduce this problem? [1] https://www.postgresql.org/message-id/MEYP282MB1669C8D88F0997354C2313C1B6CA9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
On Tue, May 24, 2022 at 8:35 PM Robert Haas <robertmhaas@gmail.com> wrote:
I think that this patch is basically correct, except that it's not
correct to set mqh_counterparty_attached when receiver is still NULL.
Here's a v2 where I've attempted to correct that while preserving the
essence of your proposed fix.
This looks good to me,
I'm not sure that we need a shm_mq_flush(), but we definitely don't
have one currently, so I've also adjusted your patch to remove the
dead prototype.
Makes sense to me.
Thanks,
Pavan
Pavan Deolasee
EnterpriseDB: https://www.enterprisedb..com
On Wed, May 25, 2022 at 7:01 AM Japin Li <japinli@hotmail.com> wrote:
I have a problem that is also related to shmem queue [1], however, I cannot
reproduce it. How did you reproduce this problem?
I discovered this bug while working on an extension that makes use of the shared memory queue facility. Not sure how useful that is for your purpose.
Thanks,
Pavan
Pavan Deolasee
EnterpriseDB: https://www.enterprisedb..com
On Mon, May 30, 2022 at 3:06 AM Pavan Deolasee <pavan.deolasee@gmail.com> wrote: >> I think that this patch is basically correct, except that it's not >> correct to set mqh_counterparty_attached when receiver is still NULL. >> Here's a v2 where I've attempted to correct that while preserving the >> essence of your proposed fix. > > This looks good to me, Thanks for checking. Committed. -- Robert Haas EDB: http://www.enterprisedb.com