You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2013/08/16 19:09:20 UTC
Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/
-----------------------------------------------------------
Review request for qpid and Andrew Stitcher.
Bugs: QPID-5078
https://issues.apache.org/jira/browse/QPID-5078
Repository: qpid
Description
-------
This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
Diffs
-----
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
Diff: https://reviews.apache.org/r/13619/diff/
Testing
-------
make test passes
Thanks,
Gordon Sim
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/#review25255
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Stitcher
On Aug. 16, 2013, 7:48 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 7:48 p.m.)
>
>
> Review request for qpid and Andrew Stitcher.
>
>
> Bugs: QPID-5078
> https://issues.apache.org/jira/browse/QPID-5078
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
>
> Diff: https://reviews.apache.org/r/13619/diff/
>
>
> Testing
> -------
>
> make test passes
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Gordon Sim <gs...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/
-----------------------------------------------------------
(Updated Aug. 16, 2013, 7:48 p.m.)
Review request for qpid and Andrew Stitcher.
Changes
-------
Remove unnecessary notifyRequired flag, as suggested by Andrew (I think!).
Bugs: QPID-5078
https://issues.apache.org/jira/browse/QPID-5078
Repository: qpid
Description
-------
This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
Diffs (updated)
-----
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
Diff: https://reviews.apache.org/r/13619/diff/
Testing
-------
make test passes
Thanks,
Gordon Sim
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Gordon Sim <gs...@redhat.com>.
> On Aug. 16, 2013, 6:13 p.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 447
> > <https://reviews.apache.org/r/13619/diff/1/?file=342087#file342087line447>
> >
> > Do you need both set and notifyRequired? It looks like set is only non-empty if notifyRequired (but I may have missed a case).
>
> Gordon Sim wrote:
> There is no way to test the set however. Now an alternative fix would of course be to add such a method and use that. This approach seemed reasonable to me.
It occurs to me that I may have misunderstood your point. Are you saying that the conditional notify() is not needed, since it is a no-op if the set is not populated? That would in fact be the case and would make the change simpler.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/#review25238
-----------------------------------------------------------
On Aug. 16, 2013, 5:09 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 5:09 p.m.)
>
>
> Review request for qpid and Andrew Stitcher.
>
>
> Bugs: QPID-5078
> https://issues.apache.org/jira/browse/QPID-5078
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
>
> Diff: https://reviews.apache.org/r/13619/diff/
>
>
> Testing
> -------
>
> make test passes
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Gordon Sim <gs...@redhat.com>.
> On Aug. 16, 2013, 6:13 p.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 447
> > <https://reviews.apache.org/r/13619/diff/1/?file=342087#file342087line447>
> >
> > Do you need both set and notifyRequired? It looks like set is only non-empty if notifyRequired (but I may have missed a case).
There is no way to test the set however. Now an alternative fix would of course be to add such a method and use that. This approach seemed reasonable to me.
- Gordon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/#review25238
-----------------------------------------------------------
On Aug. 16, 2013, 5:09 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 5:09 p.m.)
>
>
> Review request for qpid and Andrew Stitcher.
>
>
> Bugs: QPID-5078
> https://issues.apache.org/jira/browse/QPID-5078
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
>
> Diff: https://reviews.apache.org/r/13619/diff/
>
>
> Testing
> -------
>
> make test passes
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Andrew Stitcher <as...@apache.org>.
> On Aug. 16, 2013, 6:13 p.m., Andrew Stitcher wrote:
> > /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 447
> > <https://reviews.apache.org/r/13619/diff/1/?file=342087#file342087line447>
> >
> > Do you need both set and notifyRequired? It looks like set is only non-empty if notifyRequired (but I may have missed a case).
>
> Gordon Sim wrote:
> There is no way to test the set however. Now an alternative fix would of course be to add such a method and use that. This approach seemed reasonable to me.
>
> Gordon Sim wrote:
> It occurs to me that I may have misunderstood your point. Are you saying that the conditional notify() is not needed, since it is a no-op if the set is not populated? That would in fact be the case and would make the change simpler.
Yes that was one of my thoughts, sorry for being obtuse.
- Andrew
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/#review25238
-----------------------------------------------------------
On Aug. 16, 2013, 7:48 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 7:48 p.m.)
>
>
> Review request for qpid and Andrew Stitcher.
>
>
> Bugs: QPID-5078
> https://issues.apache.org/jira/browse/QPID-5078
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
>
> Diff: https://reviews.apache.org/r/13619/diff/
>
>
> Testing
> -------
>
> make test passes
>
>
> Thanks,
>
> Gordon Sim
>
>
Re: Review Request 13619: ensure notifications to queue listeners works when
using selectors/filters
Posted by Andrew Stitcher <as...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13619/#review25238
-----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/13619/#comment49533>
Do you need both set and notifyRequired? It looks like set is only non-empty if notifyRequired (but I may have missed a case).
- Andrew Stitcher
On Aug. 16, 2013, 5:09 p.m., Gordon Sim wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13619/
> -----------------------------------------------------------
>
> (Updated Aug. 16, 2013, 5:09 p.m.)
>
>
> Review request for qpid and Andrew Stitcher.
>
>
> Bugs: QPID-5078
> https://issues.apache.org/jira/browse/QPID-5078
>
>
> Repository: qpid
>
>
> Description
> -------
>
> This change ensures that if a consumer with a filter/selector skips over a message, then other listeners or consumers will still get notified of the availability of that skipped message.
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1514329
>
> Diff: https://reviews.apache.org/r/13619/diff/
>
>
> Testing
> -------
>
> make test passes
>
>
> Thanks,
>
> Gordon Sim
>
>