[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13134102#comment-13134102 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-10-22 23:36:00, Andrew Stitcher wrote: bq. /trunk/qpid/cpp/src/qpid/sys/unordered_map.h, line 24 bq. https://reviews.apache.org/r/2466/diff/1/?file=51487#file51487line24 bq. bq. This is almost certainly the wrong test. You should be testing the compiler being used not the platform being compiled for. bq. (_WIN32 will be true for the mingw compile, but it uses the gcc compiler) Thanks Andrew - I didn't think of that. I'll replace the _WIN32 check with a check for the definition of _MSC_VER instead: as per http://msdn.microsoft.com/en-us/library/b0084kay.aspx - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2466/#review2782 --- On 2011-10-20 13:26:52, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/2466/ bq. --- bq. bq. (Updated 2011-10-20 13:26:52) bq. bq. bq. Review request for qpid, Andrew Stitcher, Gordon Sim, and Chug Rolke. bq. bq. bq. Summary bq. --- bq. bq. Two performance-related tweaks to message group code: bq. 1) cache the last group lookup bq. 2) use tr1::unordered_map rather than std::map for group identifier lookup - no need to keep the group id's in order. bq. bq. Probably the most visible change would be for #2: the include path for unordered_map differs between gnu and windows - please see the included sys/unordered_map.h header for my clever work-around. bq. bq. bq. Simple benchmark before patch: bq. bq. [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 bq. send-tprecv-tp l-min l-max l-avg bq. 52863 52752 0.4762.65 18.52 bq. 52437 52440 0.5379.35 19.10 bq. 52042 52070 0.3687.69 19.26 bq. bq. after patch: bq. bq. [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 bq. send-tprecv-tp l-min l-max l-avg bq. 55888 55842 0.6884.53 17.37 bq. 55810 55784 0.4685.09 17.23 bq. 54776 54750 0.3677.25 17.46 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/Makefile.am 1186491 bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1186491 bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1186491 bq./trunk/qpid/cpp/src/qpid/sys/unordered_map.h PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/2466/diff bq. bq. bq. Testing bq. --- bq. bq. unit benchmarking. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13133518#comment-13133518 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2466/#review2782 --- /trunk/qpid/cpp/src/qpid/sys/unordered_map.h https://reviews.apache.org/r/2466/#comment6243 This is almost certainly the wrong test. You should be testing the compiler being used not the platform being compiled for. (_WIN32 will be true for the mingw compile, but it uses the gcc compiler) - Andrew On 2011-10-20 13:26:52, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/2466/ bq. --- bq. bq. (Updated 2011-10-20 13:26:52) bq. bq. bq. Review request for qpid, Andrew Stitcher, Gordon Sim, and Chug Rolke. bq. bq. bq. Summary bq. --- bq. bq. Two performance-related tweaks to message group code: bq. 1) cache the last group lookup bq. 2) use tr1::unordered_map rather than std::map for group identifier lookup - no need to keep the group id's in order. bq. bq. Probably the most visible change would be for #2: the include path for unordered_map differs between gnu and windows - please see the included sys/unordered_map.h header for my clever work-around. bq. bq. bq. Simple benchmark before patch: bq. bq. [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 bq. send-tprecv-tp l-min l-max l-avg bq. 52863 52752 0.4762.65 18.52 bq. 52437 52440 0.5379.35 19.10 bq. 52042 52070 0.3687.69 19.26 bq. bq. after patch: bq. bq. [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 bq. send-tprecv-tp l-min l-max l-avg bq. 55888 55842 0.6884.53 17.37 bq. 55810 55784 0.4685.09 17.23 bq. 54776 54750 0.3677.25 17.46 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/Makefile.am 1186491 bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1186491 bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1186491 bq./trunk/qpid/cpp/src/qpid/sys/unordered_map.h PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/2466/diff bq. bq. bq. Testing bq. --- bq. bq. unit benchmarking. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13131623#comment-13131623 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2466/ --- Review request for qpid, Andrew Stitcher, Gordon Sim, and Chug Rolke. Summary --- Two performance-related tweaks to message group code: 1) cache the last group lookup 2) use tr1::unordered_map rather than std::map for group identifier lookup - no need to keep the group id's in order. Probably the most visible change would be for #2: the include path for unordered_map differs between gnu and windows - please see the included sys/unordered_map.h header for my clever work-around. Simple benchmark before patch: [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 send-tp recv-tp l-min l-max l-avg 52863 52752 0.4762.65 18.52 52437 52440 0.5379.35 19.10 52042 52070 0.3687.69 19.26 after patch: [root@mrg44 kgiusti]# qpid-cpp-benchmark -m 100 --content-size=8 --repeat=3 --summarize --send-arg=--group-key=GROUP-ID --send-arg=--group-size=11 --receive-option node:{x-declare:{arguments:{qpid.group_header_key:GROUP-ID,qpid.shared_msg_group:1}}} --senders=2 --receivers=2 --send-arg=--group-interleave=3 send-tp recv-tp l-min l-max l-avg 55888 55842 0.6884.53 17.37 55810 55784 0.4685.09 17.23 54776 54750 0.3677.25 17.46 This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs - /trunk/qpid/cpp/src/Makefile.am 1186491 /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1186491 /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1186491 /trunk/qpid/cpp/src/qpid/sys/unordered_map.h PRE-CREATION Diff: https://reviews.apache.org/r/2466/diff Testing --- unit benchmarking. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13118035#comment-13118035 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2217 --- /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h https://reviews.apache.org/r/1980/#comment5147 +1 - Alan On 2011-09-27 20:26:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq./trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 bq./trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 bq./trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq./trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION bq. /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1172628 bq./trunk/qpid/specs/management-schema.xml 1172628 bq./trunk/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1172628 bq. bq. Diff: https://reviews.apache.org/r/1980/diff bq. bq. bq. Testing bq. --- bq. bq. new unit tests tools added. Built cpp java on Windows various linux distros. bq. bq. bq. Thanks, bq. bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13117332#comment-13117332 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h, line 37 bq. https://reviews.apache.org/r/1980/diff/3/?file=46024#file46024line37 bq. bq. How about QueueOrder? It defines the order that consumers see messages. Or perhaps something like MessageChooser bq. bq. Kenneth Giusti wrote: bq. A Rose by Any Other Name :) bq. bq. I had originally thought of calling it MessageSelector... - see below for my thoughts on the purpose of this class vs Messages class, and why I've separated the two. bq. bq. Gordon Sim wrote: bq. I didn't like MessageSelector because I felt it was likely to be confused with a selector i.e. filter as specified by a consumer. It is the component that determines which subscribers are 'allocated' particular messages, hence the current name. bq. bq. How about MessageDistributor? Aside from the outstanding naming issue - is this patch something we want/good enough for inclusion on trunk for 0.14? We're getting near the close-down date for including large changes on trunk, and I'd consider this patch a large change. As far as the MessageAllocator re-naming: I'm a bit too used to the existing names to make a good judgment call here. So I did a lookup of Allocate in my thesaurus and Distribute came up in the top ten. I'll go with MessageDistributor unless I hear otherwise. And to muddy the waters further, here are a few more matches from the thesaurus for your consideration admeasure, allot, apportion, appropriate, budget, cut, designate, dish out, divvy, earmark, give, grant, mete, set aside, share, slice, appoint, assign, budget, cut, cut the pie, distribute, divvy, dole, shell out, split up :) - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2127 --- On 2011-09-27 20:26:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13117351#comment-13117351 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h, line 37 bq. https://reviews.apache.org/r/1980/diff/3/?file=46024#file46024line37 bq. bq. How about QueueOrder? It defines the order that consumers see messages. Or perhaps something like MessageChooser bq. bq. Kenneth Giusti wrote: bq. A Rose by Any Other Name :) bq. bq. I had originally thought of calling it MessageSelector... - see below for my thoughts on the purpose of this class vs Messages class, and why I've separated the two. bq. bq. Gordon Sim wrote: bq. I didn't like MessageSelector because I felt it was likely to be confused with a selector i.e. filter as specified by a consumer. It is the component that determines which subscribers are 'allocated' particular messages, hence the current name. bq. bq. How about MessageDistributor? bq. bq. Kenneth Giusti wrote: bq. Aside from the outstanding naming issue - is this patch something we want/good enough for inclusion on trunk for 0.14? We're getting near the close-down date for including large changes on trunk, and I'd consider this patch a large change. bq. bq. As far as the MessageAllocator re-naming: I'm a bit too used to the existing names to make a good judgment call here. So I did a lookup of Allocate in my thesaurus and Distribute came up in the top ten. I'll go with MessageDistributor unless I hear otherwise. And to muddy the waters further, here are a few more matches from the thesaurus for your consideration bq. bq. bq. admeasure, allot, apportion, appropriate, budget, cut, designate, dish out, divvy, earmark, give, grant, mete, set aside, share, slice, appoint, assign, budget, cut, cut the pie, distribute, divvy, dole, shell out, split up bq. bq. :) bq. Yes, I think this is a good addition. - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2127 --- On 2011-09-27 20:26:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13116384#comment-13116384 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- 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: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13116497#comment-13116497 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h, line 91 bq. https://reviews.apache.org/r/1980/diff/3/?file=46025#file46025line91 bq. bq. 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) Ouch - good catch, thought I removed those NDEBUG paths - I'll pull it. bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h, line 59 bq. https://reviews.apache.org/r/1980/diff/3/?file=46024#file46024line59 bq. bq. Suggest calling this acquire() since that's what is used elsewhere to refer to assigning ownership. I used that name at first, but the Queue code also has acquire methods, and I thought it was confusing to see the allocator acquire the message, and the Queue also acquire the message. See below for more thoughts on the relationships of these objects... bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h, line 37 bq. https://reviews.apache.org/r/1980/diff/3/?file=46024#file46024line37 bq. bq. How about QueueOrder? It defines the order that consumers see messages. Or perhaps something like MessageChooser A Rose by Any Other Name :) I had originally thought of calling it MessageSelector... - see below for my thoughts on the purpose of this class vs Messages class, and why I've separated the two. bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 406 bq. https://reviews.apache.org/r/1980/diff/3/?file=46029#file46029line406 bq. bq. Shouldn't MessageAllocator replace the Messages interface? They are both supposed to store messages and determine the order in which they are acquired. bq. Why are we sometimes choosing the next message via the MessageAllocator and other times via the Messages? Also aren't we doubling the storage? Let me take a step back and explain my thoughts regarding these different classes: Currently, the Messages abstraction is used as the interface for the broker::Queue's in-memory storage for queued messages. It provides two services to the queue: 1) storage of the queued messages, and 2) Ordering of those stored messages (IMPORTANT). Different implementations of Queues (Priority, FIFO, LVQ, etc) can define their method of ordering which is hidden from the Queue's implementation. Personally, I'd love to rename Messages to MessageContainer, as I think that's more descriptive of its purpose. But I digress There are places in the code where the Queue merely needs to get the head available (non-acquired) message with out dispatching it to a client. For these cases, choosing a message via Messages is optimal, and there is no need for MessageAllocator. However, with the addition of message groups, the -ordering- of available messages is not enough to determine which of the available messages can be dispatched to a consumer. Now, we have to also consider the particular consumer that is requesting a message in order to select the correct message from those that are available. I don't want to overload the Messages class with yet more state (like tracking which Consumer has acquired what message) - I think it is best to keep Messages simple. That's where MessageAllocator (aka MessageSelector, MessageChooser, MessageFinagler) comes in: its purpose is to determine which message - from the sequence of available messages as determined by the Messages object - is the proper message to dispatch to a Consumer. Unlike Messages, it does not store the message, nor does it know (or care) about message ordering (again, the job of Messages). ** So these two objects allow us to separate the ordering and storage of a queued message from the intelligence that picks which among the ordered available messages is the best for a given consumer. They become orthogonal, and this allows various combinations of features to be supported. ** For example, with the non-grouped message case the MessageAllocator simply picks the head available message (see FifoAllocator impl).But for the message group implementation of the MessageAllocator (see MessageGroupManager impl), it not only uses the groupId of the available messages, but also considers the identify of the Consumer, the Consumer's position in the queue, and the availability of the group itself. All hidden
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13116618#comment-13116618 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-28 12:48:39, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h, line 37 bq. https://reviews.apache.org/r/1980/diff/3/?file=46024#file46024line37 bq. bq. How about QueueOrder? It defines the order that consumers see messages. Or perhaps something like MessageChooser bq. bq. Kenneth Giusti wrote: bq. A Rose by Any Other Name :) bq. bq. I had originally thought of calling it MessageSelector... - see below for my thoughts on the purpose of this class vs Messages class, and why I've separated the two. I didn't like MessageSelector because I felt it was likely to be confused with a selector i.e. filter as specified by a consumer. It is the component that determines which subscribers are 'allocated' particular messages, hence the current name. How about MessageDistributor? - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2127 --- On 2011-09-27 20:26:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13115586#comment-13115586 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2096 --- /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h https://reviews.apache.org/r/1980/#comment4756 I'm not sure about the class name: to me MessageAllocator implies a factory class that allocates messages (cf. mAlloc etc) Also I tend to agree with Gordon below that this should be a pure interface defined here. And actual objects of this class are made by the Queue implementations themselves, with a new Queue method called something like createMessageX() where X is a the better name for Allocator. Sigh, naming is really hard isn't it!? - Andrew On 2011-09-22 18:16:08, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-22 18:16:08) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq./trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 bq./trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 bq./trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq./trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13115888#comment-13115888 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2113 --- /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp https://reviews.apache.org/r/1980/#comment4880 Bad comment: should be any available msg - Kenneth On 2011-09-27 20:26:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-27 20:26:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/FifoAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq./trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 bq./trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 bq./trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq./trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION bq. /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1172628 bq./trunk/qpid/specs/management-schema.xml 1172628 bq./trunk/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1172628 bq. bq. Diff: https://reviews.apache.org/r/1980/diff bq. bq. bq. Testing bq. --- bq. bq. new unit tests tools added. Built cpp java on Windows various linux
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13114774#comment-13114774 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review2069 --- /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h https://reviews.apache.org/r/1980/#comment4665 I'd rather see this being a pure virtual interface, with a separately defined default implementation. For that default implementation passing in Messages rather than Queue would also seem neater. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h https://reviews.apache.org/r/1980/#comment4664 I don't like having the lock as a parameter to these methods. - Gordon On 2011-09-22 18:16:08, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-22 18:16:08) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq./trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 bq./trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 bq./trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq./trunk/qpid/cpp/src/tests/run_msg_group_tests_soak PRE-CREATION bq. /trunk/qpid/java/broker/src/main/java/org/apache/qpid/qmf/QMFService.java 1172628 bq./trunk/qpid/specs/management-schema.xml
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13112378#comment-13112378 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-21 13:43:48, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h, line 38 bq. https://reviews.apache.org/r/1980/diff/1/?file=43822#file43822line38 bq. bq. Suggest not introducing new terms here but using terms from the code, so the states would be: bq. enqueued, acquired, dequeued bq. bq. These match the names of the corresponding events below which is a good thing. I think using 'enqueued' as a state is confusing. An acquired messages is enqueued, but is not available. I'm neutral on locked v. acquired, but I prefer available as a state to enqueued. - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review1995 --- On 2011-09-20 15:14:26, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-20 15:14:26) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq./trunk/qpid/cpp/src/tests/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/tests/Makefile.am 1172628 bq./trunk/qpid/cpp/src/tests/QueueTest.cpp 1172628 bq./trunk/qpid/cpp/src/tests/cluster_tests.py 1172628 bq./trunk/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./trunk/qpid/cpp/src/tests/qpid-send.cpp 1172628 bq./trunk/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq./trunk/qpid/cpp/src/tests/run_msg_group_tests_soak
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13112534#comment-13112534 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-21 13:43:48, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h, line 38 bq. https://reviews.apache.org/r/1980/diff/1/?file=43822#file43822line38 bq. bq. Suggest not introducing new terms here but using terms from the code, so the states would be: bq. enqueued, acquired, dequeued bq. bq. These match the names of the corresponding events below which is a good thing. bq. bq. Gordon Sim wrote: bq. I think using 'enqueued' as a state is confusing. An acquired messages is enqueued, but is not available. I'm neutral on locked v. acquired, but I prefer available as a state to enqueued. Agreed that enqueued is confusing. Perhaps a little doxygen is needed to clarify what the terms means in the current codebase. I tend to think of enqueued as meaning on the in-memory queue which is wrong (and has bitten me before :) I think it really means in the store or would be if there was one or available for either delivery or re-delivery. That's not very elegantly expressed but you see what I mean. - Alan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review1995 --- On 2011-09-20 15:14:26, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-20 15:14:26) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/Connection.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13112760#comment-13112760 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-09-21 13:43:48, Alan Conway wrote: bq. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h, line 38 bq. https://reviews.apache.org/r/1980/diff/1/?file=43822#file43822line38 bq. bq. Suggest not introducing new terms here but using terms from the code, so the states would be: bq. enqueued, acquired, dequeued bq. bq. These match the names of the corresponding events below which is a good thing. bq. bq. Gordon Sim wrote: bq. I think using 'enqueued' as a state is confusing. An acquired messages is enqueued, but is not available. I'm neutral on locked v. acquired, but I prefer available as a state to enqueued. bq. bq. Alan Conway wrote: bq. Agreed that enqueued is confusing. Perhaps a little doxygen is needed to clarify what the terms means in the current codebase. I tend to think of enqueued as meaning on the in-memory queue which is wrong (and has bitten me before :) I think it really means in the store or would be if there was one or available for either delivery or re-delivery. That's not very elegantly expressed but you see what I mean. Actually - I'm going to propose something as part of the queue api restructuring shortly that will address this. For now, I'll go with available, acquired, and dequeued and address this in a separate jira/patch. - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review1995 --- On 2011-09-20 15:14:26, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-20 15:14:26) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueueObserver.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/SemanticState.h 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13112783#comment-13112783 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/ --- (Updated 2011-09-22 18:16:08.115567) Review request for qpid, Alan Conway and Gordon Sim. Changes --- Updated from first version: incorporate aconway's feedback. 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 50 --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 50 --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 (updated) - /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 Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13109504#comment-13109504 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/#review1995 --- It would be good to verify latency as well. qpid-cpp-benchmark will measure both together. what is the purpose of the new queue-query management method? e-move to message groups? Some useful code documentation added, and code simplified in several places good job. Looks good, a few comments questions. /trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp https://reviews.apache.org/r/1980/#comment4496 Shouldn't this return false if the message has already been acquired by some other consumer? /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h https://reviews.apache.org/r/1980/#comment4497 This looks a lot like the new cluster interface. There's definitely room for convergence of the broker activity watchers - but that's another JIRA. /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp https://reviews.apache.org/r/1980/#comment4498 The style guide for constants is CAPITAL_UNDERSCORES. /trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp https://reviews.apache.org/r/1980/#comment4499 I don't like the #ifdef with different code on each branch. You could say: assert(freegroups.find(qm.position) == freegroups.end()); freeGroups[qm.position] = state; /trunk/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1980/#comment4501 Are you sure it is thread safe to split this lock scope apart? /trunk/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1980/#comment4505 Is there a comment somewhere outlining the locking rules? E.g. must call acquired inside the messasgeLock, may call dequeue outside. While it's fresh in your mind ;) /trunk/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1980/#comment4507 This pattern of loop shows up a lot. Would be good to encapsulate it in some short-hand but I'm not sure how. /trunk/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1980/#comment4502 Shouldn't need a cast here. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h https://reviews.apache.org/r/1980/#comment4504 Suggest not introducing new terms here but using terms from the code, so the states would be: enqueued, acquired, dequeued These match the names of the corresponding events below which is a good thing. /trunk/qpid/cpp/src/qpid/broker/QueueObserver.h https://reviews.apache.org/r/1980/#comment4503 Should be acquired to be consistent with the naming in the code. - Alan On 2011-09-20 15:14:26, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-20 15:14:26) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13110037#comment-13110037 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- 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: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1980/ bq. --- bq. bq. (Updated 2011-09-20 15:14:26) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. This implements QPID-3346. I'd like to get this onto trunk. bq. bq. 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. bq. bq. Performance testing suggests that this patch has negligible effect on legacy traffic performance (non-grouped traffic): bq. bq. current trunk: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58096.556556.7 113124 110.473 bq. 62659.762195.4 124381 121.466 bq. 54278.254136.2 108265 105.728 bq. 55506.355501 110505 107.915 bq. 62176.858019.1 115500 112.793 bq. Averages: bq. 58543.557281.7 114355 111.675 bq. bq. + this patch: bq. [kgiusti@mrg42 tests]$ ./qpid-perftest -b 20.0.10.43 --iterations 5 --count 50 --npubs=1 --nsubs=1 --qt=1 --summary bq. 58346.155008.2 110510 107.92 bq. 59309.858747.5 117482 114.729 bq. 61323.358222.1 116436 113.707 bq. 57603 57600.4 115193 112.493 bq. 56091.856086.4 111667 109.05 bq. Averages: bq. 58534.857132.9 114258 111.58 bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./trunk/qpid/cpp/src/CMakeLists.txt 1172628 bq./trunk/qpid/cpp/src/Makefile.am 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Consumer.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1172628 bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageAllocator.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.h PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp PRE-CREATION bq./trunk/qpid/cpp/src/qpid/broker/Messages.h 1172628 bq./trunk/qpid/cpp/src/qpid/broker/Queue.h 1172628 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13108769#comment-13108769 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1980/ --- 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 50 --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 50 --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 Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13104484#comment-13104484 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1826/#review1888 --- Ship it! - Alan On 2011-09-13 15:55:22, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1826/ bq. --- bq. bq. (Updated 2011-09-13 15:55:22) bq. bq. bq. Review request for qpid, Alan Conway and Gordon Sim. bq. bq. bq. Summary bq. --- bq. bq. Added support for replicating message group state across the cluster. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1166299 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1166299 bq./branches/qpid-3346/qpid/cpp/src/tests/cluster_tests.py 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp 1164284 bq./branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests 1164284 bq. bq. Diff: https://reviews.apache.org/r/1826/diff bq. bq. bq. Testing bq. --- bq. bq. unit tests on the branch. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13103712#comment-13103712 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1826/ --- Review request for qpid, Alan Conway and Gordon Sim. Summary --- Added support for replicating message group state across the cluster. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageGroupManager.h 1166299 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp 1166299 /branches/qpid-3346/qpid/cpp/src/tests/cluster_tests.py 1158073 /branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp 1164284 /branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests 1164284 Diff: https://reviews.apache.org/r/1826/diff Testing --- unit tests on the branch. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13096335#comment-13096335 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1710/ --- Review request for qpid. Summary --- Added various options to control the generation of grouped messages. See the --help description for details. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs - /branches/qpid-3079/qpid/cpp/src/tests/qpid-send.cpp 1158073 Diff: https://reviews.apache.org/r/1710/diff Testing --- unit-test Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13095425#comment-13095425 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1690/#review1722 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h https://reviews.apache.org/r/1690/#comment3932 Perhaps we could simply revert to using the address of the consumer pointer? - Gordon On 2011-08-31 20:37:49, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1690/ bq. --- bq. bq. (Updated 2011-08-31 20:37:49) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Update to message group development: bq. bq. Created a functional test tool that verifies message group behavior across multiple clients consuming from a single queue. bq. bq. Also, bugfix: the tag used to identify a Consumer is not unique beyond the consumer's session. Queue's need to track consumers across different sessions, and using the tag was not unique enough. I've introduced a consumer name that is generated on the broker and should be unique broker-wide. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/Makefile.am 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/1690/diff bq. bq. bq. Testing bq. --- bq. bq. One simple functional test as an example - will flesh out the test script with multiple runs of the tool with different consumer and producer settings. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13095475#comment-13095475 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1690/#review1721 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h https://reviews.apache.org/r/1690/#comment3927 Why change the meaning of an existing member rather than introduce a new member like id? - Alan On 2011-08-31 20:37:49, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1690/ bq. --- bq. bq. (Updated 2011-08-31 20:37:49) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Update to message group development: bq. bq. Created a functional test tool that verifies message group behavior across multiple clients consuming from a single queue. bq. bq. Also, bugfix: the tag used to identify a Consumer is not unique beyond the consumer's session. Queue's need to track consumers across different sessions, and using the tag was not unique enough. I've introduced a consumer name that is generated on the broker and should be unique broker-wide. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/Makefile.am 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/1690/diff bq. bq. bq. Testing bq. --- bq. bq. One simple functional test as an example - will flesh out the test script with multiple runs of the tool with different consumer and producer settings. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13095515#comment-13095515 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1690/#review1725 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h https://reviews.apache.org/r/1690/#comment3936 Possibly - if the order of consumers in my container doesn't matter: think state replication across a cluster (I got burned by that exact issue before - different addresses caused ordering inconsistencies). I think order doesn't matter - I'll give this a try as it would clean things up a bit. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h https://reviews.apache.org/r/1690/#comment3937 Oops - actually, this diff doesn't show that I -introduced- this member on this branch awhile back. Originally, I removed the private tag from ConsumerImpl and replaced it with the name in the base class (so clients could be referred to by name). Per Gordon's comment, I'm going to try to revert these consumer changes (restore the trunk version) if possible. Stay tuned - Kenneth On 2011-08-31 20:37:49, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1690/ bq. --- bq. bq. (Updated 2011-08-31 20:37:49) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Update to message group development: bq. bq. Created a functional test tool that verifies message group behavior across multiple clients consuming from a single queue. bq. bq. Also, bugfix: the tag used to identify a Consumer is not unique beyond the consumer's session. Queue's need to track consumers across different sessions, and using the tag was not unique enough. I've introduced a consumer name that is generated on the broker and should be unique broker-wide. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/Makefile.am 1158073 bq./branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION bq./branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/1690/diff bq. bq. bq. Testing bq. --- bq. bq. One simple functional test as an example - will flesh out the test script with multiple runs of the tool with different consumer and producer settings. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13094877#comment-13094877 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1690/ --- Review request for qpid. Summary --- Update to message group development: Created a functional test tool that verifies message group behavior across multiple clients consuming from a single queue. Also, bugfix: the tag used to identify a Consumer is not unique beyond the consumer's session. Queue's need to track consumers across different sessions, and using the tag was not unique enough. I've introduced a consumer name that is generated on the broker and should be unique broker-wide. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/cluster/UpdateClient.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/tests/Makefile.am 1158073 /branches/qpid-3346/qpid/cpp/src/tests/msg_group_test.cpp PRE-CREATION /branches/qpid-3346/qpid/cpp/src/tests/run_msg_group_tests PRE-CREATION Diff: https://reviews.apache.org/r/1690/diff Testing --- One simple functional test as an example - will flesh out the test script with multiple runs of the tool with different consumer and producer settings. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13087772#comment-13087772 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/ --- (Updated 2011-08-19 15:44:59.613038) Review request for qpid. Changes --- Update: added heuristic for finding the next (oldest) free group in the queue. Added unit tests. Summary --- Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs (updated) - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1158686 /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1158073 Diff: https://reviews.apache.org/r/1312/diff Testing --- minimal - passes unit tests on linux. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085597#comment-13085597 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1472 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp https://reviews.apache.org/r/1312/#comment3374 What's the motivation for this change? /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h https://reviews.apache.org/r/1312/#comment3375 Does this get used? /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h https://reviews.apache.org/r/1312/#comment3376 I don't really like making this a public method on queue. The configuration of queues certainly needs some consolidation to make it a little more manageable. That seems to be the primary use case here - i.e. remembering the type of queue in use. The term Disposition is also best avoided I think as in 1.0 it has a quite different meaning. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1312/#comment3377 I don't think match should be part of the MessageAllocator interface. I think the filter perhaps should include a filter-type identifier which can be used to determine how the rest of the map should be interpreted. We may in future want to use the same filter on different types of queue, or different filters on the same type of queue. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1312/#comment3379 What is this used for? /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1312/#comment3378 I wonder if removeIf() would be a better choice here? Would allow removing the messages as we iterate. Could have the count built into the predicate or could have a max-count added to the signature (or to an doverloaded method). - Gordon On 2011-08-16 00:35:20, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-16 00:35:20) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 bq./branches/qpid-3346/qpid/specs/management-schema.xml 1144324
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085705#comment-13085705 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1473 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp https://reviews.apache.org/r/1312/#comment3380 Honestly, I have no idea! Probably something I changed but didn't completely revert - I'll remove it. /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h https://reviews.apache.org/r/1312/#comment3381 Not _really_ - the message group's getNextMessage() method uses it, so it really only applies to FIFO queues (not LVQs). And, because FIFO's are ordered by sequence number only, it can go away. I had added it to support my initial approach to support a Queue's move/reroute/purge, which I refactored to use foreach as per your suggestion. A much cleaner approach, btw. I'll remove this. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h https://reviews.apache.org/r/1312/#comment3382 Agreed - I don't like adding type info here, as it would probably be abused. You are correct - I wanted to validate the config, and added this to simplify the code. I'll back it out and specifically check the settings map for incompatible options. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1312/#comment3383 This is the default group identifier that would be applied to messages that arrive at the grouped queue _without_ containing a group identifier header. Enforces the requirement that all ungrouped messages are treated as belonging to the same default group. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp https://reviews.apache.org/r/1312/#comment3384 Originally tried that, but the eventual call to acquire() would fail as the message was already removed from the queue (we need to call acquire/dequeue to maintain correct state). I can use removeIf, but I'll have to provide a variant of acquire() that assumes the message has already been removed from the queue. - Kenneth On 2011-08-16 00:35:20, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-16 00:35:20) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085706#comment-13085706 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-08-16 09:08:13, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp, line 122 bq. https://reviews.apache.org/r/1312/diff/2/?file=32700#file32700line122 bq. bq. I don't think match should be part of the MessageAllocator interface. I think the filter perhaps should include a filter-type identifier which can be used to determine how the rest of the map should be interpreted. bq. bq. We may in future want to use the same filter on different types of queue, or different filters on the same type of queue. Yes, this can be moved out of the MessageAllocator. Can you give me a little more detail as to how you think the filter filter type should be represented? I'm thinking that we'd create a filter predicate based on the map, and that the move/reroute/purge methods would iterate over the messages applying the filter predicate (probably using the return value as the predicate to the removeIf). Does that make sense? - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1472 --- On 2011-08-16 00:35:20, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-16 00:35:20) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 bq./branches/qpid-3346/qpid/specs/management-schema.xml 1144324 bq. /branches/qpid-3346/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1144324 bq. bq. Diff: https://reviews.apache.org/r/1312/diff bq. bq. bq. Testing bq. --- bq. bq. minimal - passes unit tests on linux. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085719#comment-13085719 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-08-16 13:56:46, Kenneth Giusti wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h, line 419 bq. https://reviews.apache.org/r/1312/diff/2/?file=32699#file32699line419 bq. bq. Agreed - I don't like adding type info here, as it would probably be abused. You are correct - I wanted to validate the config, and added this to simplify the code. bq. bq. I'll back it out and specifically check the settings map for incompatible options. Perhaps what we want here is a QueueConfiguration class that is an internal only, strongly typed, explicitly named set of configuration options. We would create this from the arguments specified in one place (within this class itself), and then all other decisions based on configuration could use the simpler, more explicit interface. What do you think? bq. On 2011-08-16 13:56:46, Kenneth Giusti wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp, line 660 bq. https://reviews.apache.org/r/1312/diff/2/?file=32700#file32700line660 bq. bq. Originally tried that, but the eventual call to acquire() would fail as the message was already removed from the queue (we need to call acquire/dequeue to maintain correct state). bq. bq. I can use removeIf, but I'll have to provide a variant of acquire() that assumes the message has already been removed from the queue. bq. bq. Ah, yes I see what you mean... I was confusing Messages::remove() and Queue::acquire() there for a minute... That said however, is much of Queue::acquire() not actually irrelevant here (i.e. in purge/move/reroute) anyway? We don't actually want to check whether we can acquire the message. All we need to do (other than remove it from messages) is notify the observers. Or am I missing something? - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1473 --- On 2011-08-16 00:35:20, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-16 00:35:20) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085720#comment-13085720 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-08-16 09:08:13, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp, line 122 bq. https://reviews.apache.org/r/1312/diff/2/?file=32700#file32700line122 bq. bq. I don't think match should be part of the MessageAllocator interface. I think the filter perhaps should include a filter-type identifier which can be used to determine how the rest of the map should be interpreted. bq. bq. We may in future want to use the same filter on different types of queue, or different filters on the same type of queue. bq. bq. Kenneth Giusti wrote: bq. Yes, this can be moved out of the MessageAllocator. Can you give me a little more detail as to how you think the filter filter type should be represented? I'm thinking that we'd create a filter predicate based on the map, and that the move/reroute/purge methods would iterate over the messages applying the filter predicate (probably using the return value as the predicate to the removeIf). Does that make sense? Yes, that makes sense. We may not even need filter type - it would only be required if the map structure could take different forms for different filters. - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1472 --- On 2011-08-16 00:35:20, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-16 00:35:20) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 bq./branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 bq./branches/qpid-3346/qpid/specs/management-schema.xml 1144324 bq. /branches/qpid-3346/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1144324 bq. bq. Diff: https://reviews.apache.org/r/1312/diff bq. bq. bq. Testing bq. --- bq. bq. minimal - passes unit tests on linux. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key:
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13086013#comment-13086013 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/ --- (Updated 2011-08-16 22:04:42.180010) Review request for qpid. Changes --- Updated to incorporate review input: 1) removed message filter from MessageAllocator, created a more generic filter 2) removed unneeded stuff Summary --- Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs (updated) - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1158073 /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1158073 Diff: https://reviews.apache.org/r/1312/diff Testing --- minimal - passes unit tests on linux. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13085457#comment-13085457 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/ --- (Updated 2011-08-16 00:35:20.779142) Review request for qpid. Changes --- Latest changes pushed to branch. 1) flesh-out the UI changes 2) simple, non-optimal, approach for group support Need to optimize by allowing direct removal of queued items, rather than seeking to the appropriate item. Summary --- Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs (updated) - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Broker.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Fairshare.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageDeque.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/MessageMap.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Messages.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/PriorityQueue.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144319 /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144319 /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144319 /branches/qpid-3346/qpid/specs/management-schema.xml 1144324 /branches/qpid-3346/qpid/tests/src/py/qpid_tests/broker_0_10/management.py 1144324 Diff: https://reviews.apache.org/r/1312/diff Testing --- minimal - passes unit tests on linux. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13081595#comment-13081595 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-08-08 10:11:17, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp, line 39 bq. https://reviews.apache.org/r/1312/diff/1/?file=30946#file30946line39 bq. bq. The purpose of the check is to ensure that an acquire attempt for a message that has since been replaced, does not acquire the message that has replaced it instead. bq. bq. I believe it is still necessary, though I concede the form is ugly and unintuitive. bq. bq. Kenneth Giusti wrote: bq. Agreed that it is necessary for LVQ, but the abstract api for 'Messages::remove' can't explicitly enforce that the caller supply the actual target msg, just the position. Given that, it's pretty easy for the caller to get this wrong without discovering it (which actually happened to me - thank goodness for unit tests :). bq. bq. And the move message/purge message management operations probably could use a remove method that doesn't necessitate a preceding message find(). bq. bq. I'm thinking of two different message-removal use-cases: bq. bq. bool Messages::remove( position, QueuedMessage *result) - remove msg at position, return true and set result if found bq. bool Messages::remove( const QueuedMessage target ) - remove target msg, return true if msg was found. This variant could be used by 'acquire' or other paths that have already retrieved the QueuedMessage. bq. bq. Although, defining a iterator for the Messages class probably renders this whole issue moot. Yes, I think you are right that the confusion comes from trying to use the same codepath for these two cases and separating them would be clearer. - Gordon --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1317 --- On 2011-08-05 20:46:15, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-05 20:46:15) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144324 bq. bq. Diff: https://reviews.apache.org/r/1312/diff bq. bq. bq. Testing bq. --- bq. bq. minimal - passes unit tests on linux. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: UI.txt, msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13080882#comment-13080882 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1317 --- /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp https://reviews.apache.org/r/1312/#comment2990 Not too keen on this lookup. Can it be avoided? E.g. can we modify the Queue::acquire() to simply take the consumer name as the second parameter? (That is for the present at least all that is required) Alternatively the DeliveryRecord could be modified to hold a pointer to the Consumer rather than simply the tag. /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp https://reviews.apache.org/r/1312/#comment2991 The purpose of the check is to ensure that an acquire attempt for a message that has since been replaced, does not acquire the message that has replaced it instead. I believe it is still necessary, though I concede the form is ugly and unintuitive. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h https://reviews.apache.org/r/1312/#comment2992 I'm not keen on the terminology here. For me selector implies something subtly different from the role this object is serving (at least from the role I *think* it is serving). I'd prefer something like 'allocator'. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h https://reviews.apache.org/r/1312/#comment2993 Again, I'm not too keen on the term 'consumed' in this context. Though I can see how it is justified, it is potentially confusing in my view (could imply the actual dequeue of a message). I'd prefer 'acquired', 'allocated' or even just 'locked' as they are all less ambiguous on the state in question. - Gordon On 2011-08-05 20:46:15, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-05 20:46:15) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144324 bq. bq. Diff: https://reviews.apache.org/r/1312/diff bq. bq. bq. Testing bq. --- bq. bq. minimal - passes unit tests on linux. bq. bq. bq. Thanks, bq. bq. Kenneth bq. bq. Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13081204#comment-13081204 ] jirapos...@reviews.apache.org commented on QPID-3346: - bq. On 2011-08-08 10:11:17, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h, line 132 bq. https://reviews.apache.org/r/1312/diff/1/?file=30947#file30947line132 bq. bq. I'm not keen on the terminology here. For me selector implies something subtly different from the role this object is serving (at least from the role I *think* it is serving). bq. bq. I'd prefer something like 'allocator'. Me too - renamed to allocator. bq. On 2011-08-08 10:11:17, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h, line 148 bq. https://reviews.apache.org/r/1312/diff/1/?file=30947#file30947line148 bq. bq. Again, I'm not too keen on the term 'consumed' in this context. Though I can see how it is justified, it is potentially confusing in my view (could imply the actual dequeue of a message). bq. bq. I'd prefer 'acquired', 'allocated' or even just 'locked' as they are all less ambiguous on the state in question. Agreed - renamed to acquired. bq. On 2011-08-08 10:11:17, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp, line 158 bq. https://reviews.apache.org/r/1312/diff/1/?file=30945#file30945line158 bq. bq. Not too keen on this lookup. Can it be avoided? bq. bq. E.g. can we modify the Queue::acquire() to simply take the consumer name as the second parameter? (That is for the present at least all that is required) bq. bq. Alternatively the DeliveryRecord could be modified to hold a pointer to the Consumer rather than simply the tag. Modified Queue::acquire() to take the name instead - should be ok. I originally had added a pointer to the Consumer into the DeliveryRecord, but that raised issues with cluster replication (Consumer not present when replicating DeliveryRecord). May have to revisit this if it turns out we need the pointer. bq. On 2011-08-08 10:11:17, Gordon Sim wrote: bq. /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp, line 39 bq. https://reviews.apache.org/r/1312/diff/1/?file=30946#file30946line39 bq. bq. The purpose of the check is to ensure that an acquire attempt for a message that has since been replaced, does not acquire the message that has replaced it instead. bq. bq. I believe it is still necessary, though I concede the form is ugly and unintuitive. Agreed that it is necessary for LVQ, but the abstract api for 'Messages::remove' can't explicitly enforce that the caller supply the actual target msg, just the position. Given that, it's pretty easy for the caller to get this wrong without discovering it (which actually happened to me - thank goodness for unit tests :). And the move message/purge message management operations probably could use a remove method that doesn't necessitate a preceding message find(). I'm thinking of two different message-removal use-cases: bool Messages::remove( position, QueuedMessage *result) - remove msg at position, return true and set result if found bool Messages::remove( const QueuedMessage target ) - remove target msg, return true if msg was found. This variant could be used by 'acquire' or other paths that have already retrieved the QueuedMessage. Although, defining a iterator for the Messages class probably renders this whole issue moot. - Kenneth --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/#review1317 --- On 2011-08-05 20:46:15, Kenneth Giusti wrote: bq. bq. --- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/1312/ bq. --- bq. bq. (Updated 2011-08-05 20:46:15) bq. bq. bq. Review request for qpid. bq. bq. bq. Summary bq. --- bq. bq. Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. bq. bq. bq. This addresses bug qpid-3346. bq. https://issues.apache.org/jira/browse/qpid-3346 bq. bq. bq. Diffs bq. - bq. bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 bq./branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 bq.
[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.
[ https://issues.apache.org/jira/browse/QPID-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13080212#comment-13080212 ] jirapos...@reviews.apache.org commented on QPID-3346: - --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1312/ --- Review request for qpid. Summary --- Some initial refactoring of Queue/Consumer interface to allow for message grouping support. Very preliminary. This addresses bug qpid-3346. https://issues.apache.org/jira/browse/qpid-3346 Diffs - /branches/qpid-3346/qpid/cpp/src/qpid/broker/Consumer.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/DeliveryRecord.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/LegacyLVQ.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/Queue.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueEvents.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueueObserver.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/QueuePolicy.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/SemanticState.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1144324 /branches/qpid-3346/qpid/cpp/src/qpid/cluster/Connection.cpp 1144324 /branches/qpid-3346/qpid/cpp/src/tests/QueueTest.cpp 1144324 Diff: https://reviews.apache.org/r/1312/diff Testing --- minimal - passes unit tests on linux. Thanks, Kenneth Support message grouping with stricted sequence consumption across multiple consumers. -- Key: QPID-3346 URL: https://issues.apache.org/jira/browse/QPID-3346 Project: Qpid Issue Type: New Feature Components: C++ Broker Affects Versions: 0.12 Reporter: Ken Giusti Assignee: Ken Giusti Fix For: 0.14 Attachments: msg_groups_0.2.txt This feature is described in the attached QIP as Policy #2: Sequenced Consumers. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org