Horton

Thread

Commits

  1. shm_mq_sendv: Fix flushing bug when receiver not yet attached.

  1. Shmem queue is not flushed if receiver is not yet attached

    Pavan Deolasee <pavan.deolasee@gmail.com> — 2022-03-17T07:12:58Z

    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,
Pavan

-- 
Pavan Deolasee
EnterpriseDB: https://www.enterprisedb..com

  2. Re: Shmem queue is not flushed if receiver is not yet attached

    Robert Haas <robertmhaas@gmail.com> — 2022-05-24T15:05:23Z

    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 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.

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

  3. Re: Shmem queue is not flushed if receiver is not yet attached

    Japin Li <japinli@hotmail.com> — 2022-05-25T01:30:55Z

    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 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.
>
> 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.

  4. Re: Shmem queue is not flushed if receiver is not yet attached

    Pavan Deolasee <pavan.deolasee@gmail.com> — 2022-05-30T07:06:14Z

    Hi Robert,

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

  5. Re: Shmem queue is not flushed if receiver is not yet attached

    Pavan Deolasee <pavan.deolasee@gmail.com> — 2022-05-30T07:07:51Z

    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

  6. Re: Shmem queue is not flushed if receiver is not yet attached

    Robert Haas <robertmhaas@gmail.com> — 2022-05-31T13:02:55Z

    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