qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request 17641: QPID-5534: [C++ broker] Headers exchange can route a message to one queue multiple times
Date Mon, 03 Feb 2014 09:52:42 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17641/#review33417
-----------------------------------------------------------



/trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp
<https://reviews.apache.org/r/17641/#comment62864>

    Rather than putting each item in a map, then generating the list, you could use the map
(or even just a set) to test whether the queue has already been added to the list, and if
not add the binding here.
    
    Whether using a map/set is better than simply checking for the queue in the vector will
probably vary quite a bit. Sometimes the extra overhead of adding to the map will outweigh
the time to search the list. However I think on balance its probably better/nicer to do this
anyway.


- Gordon Sim


On Feb. 2, 2014, 10:56 a.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17641/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2014, 10:56 a.m.)
> 
> 
> Review request for qpid, Chug Rolke and Gordon Sim.
> 
> 
> Bugs: QPID-5534
>     https://issues.apache.org/jira/browse/QPID-5534
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Root cause of the bug: HeadersExchange::route method generates BindingList that may contain
multiple bindings to the same queue. That causes a message to be enqueued to the same queue
more than once.
> 
> The patch sorts+makes unique the BindingList by using std::map < Queue , Binding >
(boost pointers in fact). Also an automated python test added.
> 
> The patch is trivial, but:
> - I am not sure if there isn't easier / more elegant way of sort&uniq of BindingList
> - one quite irrelevant test "qpid_tests.broker_0_10.management.ManagementTest.test_connection_stats"
fails in 1/2 of runs; Debugging it found that the QMF related connection was created almost
minute _after_ session.attach("stats-session") invoked. That seems like an issue unrelated
to this patch.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/HeadersExchange.cpp 1563578 
>   /trunk/qpid/tests/src/py/qpid_tests/broker_0_10/exchange.py 1563578 
> 
> Diff: https://reviews.apache.org/r/17641/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message