qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alan Conway" <acon...@redhat.com>
Subject Re: Review Request: QPID-3346: message groups (multi-consumer per group)
Date Wed, 28 Sep 2011 12:48:39 GMT

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



/trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h
<https://reviews.apache.org/r/1980/#comment4916>

    How about QueueOrder? It defines the order that consumers see messages. Or perhaps something
like MessageChooser



/trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h
<https://reviews.apache.org/r/1980/#comment4917>

    Suggest calling this acquire() since that's what is used elsewhere to refer to assigning
ownership.



/trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h
<https://reviews.apache.org/r/1980/#comment4919>

    Don't put different logic in debug/release builds. Asserts are fine but this is calling
a different function. If there's a bug in the NDEBUG function then developer testing won't
uncover it. (even with asserts you need to be careful that they don't have side-effects, we've
been bitten by that before)



/trunk/qpid/cpp/src/qpid/broker/Queue.cpp
<https://reviews.apache.org/r/1980/#comment4918>

    Shouldn't MessageAllocator replace the Messages interface? They are both supposed to store
messages and determine the order in which they are acquired. 
    Why are we sometimes choosing the next message via the MessageAllocator and other times
via the Messages? Also aren't we doubling the storage?


- Alan


On 2011-09-27 20:26:22, Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1980/
> -----------------------------------------------------------
> 
> (Updated 2011-09-27 20:26:22)
> 
> 
> Review request for qpid, Alan Conway and Gordon Sim.
> 
> 
> Summary
> -------
> 
> This implements QPID-3346.  I'd like to get this onto trunk.
> 
> This patch does not include optimizations I have made - I'll be reviewing & submitting
these separately as they involve more intrusive changes to the Queue API. Ignore for now the
TODO comments in the MessageGroups source files.
> 
> Performance testing suggests that this patch has negligible effect on legacy traffic
performance (non-grouped traffic):
> 
> current trunk:
> [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 500000 --npubs=1
--nsubs=1 --qt=1 --summary
> 58096.5	56556.7	113124	110.473
> 62659.7	62195.4	124381	121.466
> 54278.2	54136.2	108265	105.728
> 55506.3	55501	110505	107.915
> 62176.8	58019.1	115500	112.793
> Averages: 
> 58543.5	57281.7	114355	111.675
> 
> + this patch:
> [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 500000 --npubs=1
--nsubs=1 --qt=1 --summary
> 58346.1	55008.2	110510	107.92
> 59309.8	58747.5	117482	114.729
> 61323.3	58222.1	116436	113.707
> 57603	57600.4	115193	112.493
> 56091.8	56086.4	111667	109.05
> Averages: 
> 58534.8	57132.9	114258	111.58
> 
> 
> This addresses bug qpid-3346.
>     https://issues.apache.org/jira/browse/qpid-3346
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/CMakeLists.txt 1172628 
>   /trunk/qpid/cpp/src/Makefile.am 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 
>   /trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 
>   /trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 
>   /trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 
>   /trunk/qpid/cpp/src/tests/Makefile.am 1172628 
>   /trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 
>   /trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 
>   /trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 
>   /trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION 
>   /trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION 
>   /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1172628 
>   /trunk/qpid/specs/management-schema.xml 1172628 
>   /trunk/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1172628 
> 
> Diff: https://reviews.apache.org/r/1980/diff
> 
> 
> Testing
> -------
> 
> new unit tests & tools added.   Built cpp & java on Windows & various linux
distros.
> 
> 
> Thanks,
> 
> Kenneth
> 
>


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