qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kenneth Giusti" <kgiu...@apache.org>
Subject Re: Review Request: QPID-3346: message groups (multi-consumer per group)
Date Wed, 21 Sep 2011 21:27:39 GMT

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



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

    Yes it should - but at this point all messages currently on the queue are acquireable
(we remove them as they are acquired).    I'll add a comment here.



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

    :) - I hope these changes move us closer to that convergence.



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

    Thanks - I will fix this.



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

    Good suggestion - I'll add that.



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

    Now that you mention it - not really.  Actually, the original code already splits the
lock (Queue::dequeue() re-takes it).



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

    This needs to be done.
    
    One thing I'll do for these private methods that require locking is to pass the lock as
an (unused) argument.  Makes the need for the lock mandatory (though perhaps a little ugly).
    



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

    Good suggestion - I'll try to think of something...
    



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

    I'll try removing it.
    



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

    That's a good point - I was trying to get away from having an 0.10-based terminology,
but those terms are so well-understood I should use them here.


- Kenneth


On 2011-09-20 15:14:26, Kenneth Giusti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1980/
> -----------------------------------------------------------
> 
> (Updated 2011-09-20 15:14:26)
> 
> 
> 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/MessageAllocator.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp 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