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