Bug 871774 - Browser may read messages acquired by other consumer on message group queue
Browser may read messages acquired by other consumer on message group queue
Status: CLOSED ERRATA
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-cpp (Show other bugs)
Development
Unspecified Unspecified
high Severity medium
: 2.3
: ---
Assigned To: Ken Giusti
Petr Matousek
: Regression
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-31 07:00 EDT by Petr Matousek
Modified: 2013-03-06 13:52 EST (History)
4 users (show)

See Also:
Fixed In Version: qpid-cpp-0.18-6
Doc Type: Bug Fix
Doc Text:
Cause: The algorithm for browsing messages changed, but the message group code was not updated correctly. Consequence: A message acquired by a client would still remain visible to browsing clients. Fix: The message group code was updated to the new algorithm. Result: Acquired group messages are no longer visible to other browsing clients.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-03-06 13:52:38 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Petr Matousek 2012-10-31 07:00:39 EDT
Description of problem:

Automated tests shows that the way of browsing message group queue has changes.

When browsing a message group queue, the browser can see messages acquired-but-not-acknowledged by other consumers. This is message group queue related issue, common queues do not suffer from that.

Version-Release number of selected component (if applicable):
qpid-cpp-*-0.18-2

How reproducible:
100%

Steps to Reproduce:
run the following single test from qpid_ptest_unit_test: 
custom_py_units.queue.HLAPITests.test_message_groups_sequenced_consumers_common

If you need further instruction how to execute the test, please visit:
https://wiki.test.redhat.com/MRG/MRGQETestInfrastructure#Customunittestbasedonqpidpythontest

OR

1. create a message group queue an enqueue several messages
2. acquire a message from that queue, but do not acknowledge yet
3. browse the queue with other client, and realize that the acquired message is listed

Actual results:
Browser can read already acquired messages

Expected results:
Same approach as on common queue, the acquired messages can't be read by the browser.

Additional info:
Comment 3 Ken Giusti 2012-10-31 16:01:08 EDT
I think I have a fix for this:

diff --git a/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp b/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp
index bc5d797..c30f009 100644
--- a/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp
+++ b/qpid/cpp/src/qpid/broker/MessageGroupManager.cpp
@@ -260,7 +260,7 @@ bool MessageGroupManager::allocate(const std::string& consumer, const QueuedMess
 bool MessageGroupManager::nextBrowsableMessage( Consumer::shared_ptr& c, QueuedMessage& next )
 {
     // browse: allow access to any available msg, regardless of group ownership (?ok?)
-    return messages.browse(c->getPosition(), next, false);
+    return messages.browse(c->getPosition(), next, !c->browseAcquired());
 }
 
 void MessageGroupManager::query(qpid::types::Variant::Map& status) const

this allows the reproducer to pass, but for some reason one of the message groups unit test is failing.  I'll have to look at it a bit more.
Comment 4 Ken Giusti 2012-11-01 08:59:57 EDT
Ok, I understand this a bit more:

There was a change to the qpid::broker::Consumer class that added a flag that controlled whether acquired messages are browsable [Consumer::browseAcquired()].  This flag defaults to false (do not browse acquired messages).

The msg group code was not updated to reference this flag.

The following patch is also needed to correct one of the message group tests:

diff --git a/qpid/tests/src/py/qpid_tests/broker_0_10/msg_groups.py b/qpid/tests/src/py/qpid_tests/broker_0_10/msg_groups.py
index ace7611..c9c498b 100644
--- a/qpid/tests/src/py/qpid_tests/broker_0_10/msg_groups.py
+++ b/qpid/tests/src/py/qpid_tests/broker_0_10/msg_groups.py
@@ -767,10 +767,10 @@ class MultiConsumerMsgGroupTests(Base):
                        'filter_params' : { 'header_key' : "THE-GROUP",
                                            'header_value' : "A" }}
         assert queue.msgDepth == 6
-        rc = queue.purge(1, msg_filter)
+        rc = queue.purge(1, msg_filter)  # should purge A-2 (first available)
         assert rc.status == 0
         queue.update()
-        queue.msgDepth == 5   # the pending acquired A still counts!
+        queue.msgDepth == 4
 
         # verify all other A's removed....
         s2 = self.setup_session()
@@ -787,7 +787,7 @@ class MultiConsumerMsgGroupTests(Base):
         except Empty:
             pass
         assert count == 3   # non-A's
-        assert a_count == 2 # pending acquired message included in browse results
+        assert a_count == 1 # Only one (unacquired) A msg
         s1.acknowledge()    # ack the consumed A-0
         self.qmf_session.delBroker(self.qmf_broker)
   
It appears that the test was incorrectly modified to work with browsing acquired messages as part of the original patch that added the browseAcquired() flag.

Justin - do you agree?
Comment 5 Justin Ross 2012-11-01 10:28:03 EDT
This all makes sense to me.  Thanks, Ken.
Comment 7 Petr Matousek 2012-11-04 06:52:47 EST
This issue has been fixed in the broker (qpid-cpp-0.18-6).

qpid-tests package needs to be rebuild to correct the unit test mentioned in comment 4 (successfully tested with the patch).

Leaving open until that time.
Comment 10 Petr Matousek 2012-11-12 11:05:23 EST
This issue has been fixed.

Verified on rhel5 and rhel6 (both i386,x86_64)

packages used for testing:
qpid-cpp-*-0.18-7

-> VERIFIED
Comment 12 errata-xmlrpc 2013-03-06 13:52:38 EST
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHSA-2013-0561.html

Note You need to log in before you can comment on or make changes to this bug.