Qpid
  1. Qpid
  2. QPID-3720

[Java Broker] Implement Message Grouping

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.15
    • Component/s: Java Broker
    • Labels:
      None

      Description

      Implement message grouping on the Java Broker (similar in style to that implemented in the C++ broker in QPID-3346)

      The Java Broker supports two implementations of message grouping; an implementation with the same semantics as defined by the C++ broker, and an alternative implementation.

      In contrast to the C++ implementation the alternative implementation has the following properties:

      1) Once a group has been assigned to a subscription, assign all further messages from the same group to the same group (whether or not all prior messages in the group have been acknowledged)
      2) The absence of a value in the designated header field for grouping as treated as indicative of a lack of desire for the message to be grouped. Messages with such a lack of a value will be distributed to any available consumer.

      Note that in general 1) is stricter than the condition that is enforced by the C++ broker - however in the case where a client cancels a subscription without first releasing/acknowledging all outstanding messages that have been delivered then you can get "out of order".

      In general the Java implementation is more liberal than the C++ implementation in that for both strategies available in the Java Broker in that

      a) Message grouping is allowed on priority queues, Conflation Queues (and Sorted Queues). (However it has no effect on browsers... which means if you using a ConflationQueue as an LVQ by browsing it you cannot use groups).
      b) The broker allows for non string values for the group header

      In order to choose the alternative strategy (rather than the C++ style grouping semantics) the qpid.shared_msg_group argument should not be set on queue construction (whether grouping is enforced or not depends on whether the qpid.group_header_key argument is non-null), if qpid.shared_msg_group is set to "1" the C++ style grouping is used, otherwise the alternative implementation is used.

        Issue Links

          Activity

          Rob Godfrey created issue -
          Rob Godfrey logged work - 03/Jan/12 19:46
          • Time Spent:
            6h
             
            <No comment>
          Rob Godfrey made changes -
          Field Original Value New Value
          Status Open [ 1 ] In Progress [ 3 ]
          Rob Godfrey made changes -
          Remaining Estimate 6h [ 21600 ] 0h [ 0 ]
          Time Spent 6h [ 21600 ]
          Worklog Id 12634 [ 12634 ]
          Hide
          Robbie Gemmell added a comment -

          The feature implementation itself seems fine, but the tests could use a couple of changes.

          The actual message recieved should be verified to ensure the correct ordering, as the message might be from the correct group but not be the expected message. Eg in testConsumerCloseGroupAssignment, the test would pass if consumer 2 got message 1, 3 or 4 in any order, but there is an expectation of particular messages and ordering so it should be verified. Also in the tests the group verification should have the expected group as the first of the two values instead of the second.

          Show
          Robbie Gemmell added a comment - The feature implementation itself seems fine, but the tests could use a couple of changes. The actual message recieved should be verified to ensure the correct ordering, as the message might be from the correct group but not be the expected message. Eg in testConsumerCloseGroupAssignment, the test would pass if consumer 2 got message 1, 3 or 4 in any order, but there is an expectation of particular messages and ordering so it should be verified. Also in the tests the group verification should have the expected group as the first of the two values instead of the second.
          Hide
          Robbie Gemmell added a comment -

          Following the addition of the C++ broker style groups (possibly worth noting in the JIRA description? ) I see what looks like an issue with the 'default group' check in the SimpleAMQQueue constructor:

                          String defaultGroup = String.valueOf(arguments.get(QPID_DEFAULT_MESSAGE_GROUP));
                          _messageGroupManager =
                                  new DefinedGroupMessageGroupManager(String.valueOf(arguments.get(QPID_GROUP_HEADER_KEY)),
                                          defaultGroup == null ? QPID_NO_GROUP : defaultGroup,
                                          this);
          

          The check for defaultGroup == null might never be true, because String.valueof() doesnt return null (except possibly in a case the Javadoc isnt that clear on, when it is provided with a non-null Object which has a toString() implementation which returns null ?) and instead gives the String value "null" when provided with a null reference.

          Show
          Robbie Gemmell added a comment - Following the addition of the C++ broker style groups (possibly worth noting in the JIRA description? ) I see what looks like an issue with the 'default group' check in the SimpleAMQQueue constructor: String defaultGroup = String .valueOf(arguments.get(QPID_DEFAULT_MESSAGE_GROUP)); _messageGroupManager = new DefinedGroupMessageGroupManager( String .valueOf(arguments.get(QPID_GROUP_HEADER_KEY)), defaultGroup == null ? QPID_NO_GROUP : defaultGroup, this ); The check for defaultGroup == null might never be true, because String.valueof() doesnt return null (except possibly in a case the Javadoc isnt that clear on, when it is provided with a non-null Object which has a toString() implementation which returns null ?) and instead gives the String value "null" when provided with a null reference.
          Rob Godfrey made changes -
          Status In Progress [ 3 ] Ready To Review [ 10006 ]
          Hide
          Rob Godfrey added a comment -

          Addressed the comment by Robbie - assignment of the default group should now work

          Show
          Rob Godfrey added a comment - Addressed the comment by Robbie - assignment of the default group should now work
          Rob Godfrey made changes -
          Assignee Rob Godfrey [ rgodfrey ] Robbie Gemmell [ gemmellr ]
          Hide
          Robbie Gemmell added a comment -

          Looks good. Updating the JIRA description and/or adding some docs would seem useful before closing out.

          Robbie.

          Show
          Robbie Gemmell added a comment - Looks good. Updating the JIRA description and/or adding some docs would seem useful before closing out. Robbie.
          Rob Godfrey made changes -
          Description Implement message grouping on the Java Broker (similar in style to that implemented in the C++ broker in QPID-3346)

          In contrast to the C++ implementation

          1) Once a group has been assigned to a subscription, assign all further messages from the same group to the same group (whether or not all prior messages in the group have been acknowledged)
          2) Message grouping is allowed on priority queues, Conflation Queues (and Sorted Queues). (However it has no effect on browsers... which means if you using a ConflationQueue as an LVQ by browsing it you cannot use groups).
          3) Allow for non string values for the group header
          4) Setting the qpid.shared_msg_group argument is not required on queue construction (whether grouping is enforced or not depends on whether the qpid.group_header_key argument is non-null).

          Note that in general 1) is stricter than the condition that is enforced by the C++ broker - however in the case where a client cancels a subscription without first releasing/acknowledging all outstanding messages that have been delivered then you can get "out of order".
          Implement message grouping on the Java Broker (similar in style to that implemented in the C++ broker in QPID-3346)

          The Java Broker supports two implementations of message grouping; an implementation with the same semantics as defined by the C++ broker, and an alternative implementation.

          In contrast to the C++ implementation the alternative implementation has the following properties:

          1) Once a group has been assigned to a subscription, assign all further messages from the same group to the same group (whether or not all prior messages in the group have been acknowledged)
          2) The absence of a value in the designated header field for grouping as treated as indicative of a lack of desire for the message to be grouped. Messages with such a lack of a value will be distributed to any available consumer.

          Note that in general 1) is stricter than the condition that is enforced by the C++ broker - however in the case where a client cancels a subscription without first releasing/acknowledging all outstanding messages that have been delivered then you can get "out of order".

          In general the Java implementation is more liberal than the C++ implementation in that for both strategies available in the Java Broker in that

          a) Message grouping is allowed on priority queues, Conflation Queues (and Sorted Queues). (However it has no effect on browsers... which means if you using a ConflationQueue as an LVQ by browsing it you cannot use groups).
          b) The broker allows for non string values for the group header

          In order to choose the alternative strategy (rather than the C++ style grouping semantics) the qpid.shared_msg_group argument should not be set on queue construction (whether grouping is enforced or not depends on whether the qpid.group_header_key argument is non-null), if qpid.shared_msg_group is set to "1" the C++ style grouping is used, otherwise the alternative implementation is used.

          Hide
          Robbie Gemmell added a comment -

          Closing out

          Show
          Robbie Gemmell added a comment - Closing out
          Robbie Gemmell made changes -
          Status Ready To Review [ 10006 ] Resolved [ 5 ]
          Fix Version/s 0.15 [ 12319043 ]
          Resolution Fixed [ 1 ]
          Philip Harvey made changes -
          Link This issue is related to QPID-4817 [ QPID-4817 ]
          Rob Godfrey made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open In Progress In Progress
          9s 1 Rob Godfrey 03/Jan/12 19:46
          In Progress In Progress Reviewable Reviewable
          12d 21h 17m 1 Rob Godfrey 16/Jan/12 17:03
          Reviewable Reviewable Resolved Resolved
          7d 1h 37m 1 Robbie Gemmell 23/Jan/12 18:41
          Resolved Resolved Closed Closed
          1115d 1h 26m 1 Rob Godfrey 11/Feb/15 20:07

            People

            • Assignee:
              Robbie Gemmell
              Reporter:
              Rob Godfrey
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 6h
                6h
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 6h
                6h

                  Development