[jira] [Commented] (QPID-3346) Support message grouping with stricted sequence consumption across multiple consumers.

2011-10-24 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-10-22 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-10-20 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-30 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-29 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-29 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-28 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-28 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-28 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

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

2011-09-26 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-22 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-22 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-22 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-22 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-21 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-21 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-20 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-14 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-13 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-02 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-01 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-01 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-09-01 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-31 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-19 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-16 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-15 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-09 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-08 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-08 Thread jirapos...@reviews.apache.org (JIRA)

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

2011-08-05 Thread jirapos...@reviews.apache.org (JIRA)

[ 
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