Bug 1223730
| Summary: | [MRG 2.*] Rerouting messages from priority queue reverses priority ordering | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise MRG | Reporter: | Pavel Moravec <pmoravec> | ||||||
| Component: | qpid-cpp | Assignee: | messaging-bugs <messaging-bugs> | ||||||
| Status: | CLOSED ERRATA | QA Contact: | Petra Svobodová <psvobodo> | ||||||
| Severity: | high | Docs Contact: | |||||||
| Priority: | high | ||||||||
| Version: | 2.5 | CC: | jross, mcressma, mtoth, pematous, pmoravec | ||||||
| Target Milestone: | 2.5.17 | ||||||||
| Target Release: | --- | ||||||||
| Hardware: | All | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Fixed In Version: | qpid-cpp-0.18-39 | Doc Type: | Bug Fix | ||||||
| Doc Text: |
If a priority queue contained messages with differing priorities, rerouting the queue contents to a non-priority destination queue caused the messages to be ordered from lowest to highest, instead of highest to lowest. A fix to the MoveMessages::removeIf and PriorityQueue::removeIf changes the message traversal order to highest-to-lowest, which ensures that any rerouted messages with priority assigned are processed first in non-priority destination queues.
|
Story Points: | --- | ||||||
| Clone Of: | Environment: | ||||||||
| Last Closed: | 2015-07-16 13:25:37 UTC | Type: | Bug | ||||||
| Regression: | --- | Mount Type: | --- | ||||||
| Documentation: | --- | CRM: | |||||||
| Verified Versions: | Category: | --- | |||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||
| Embargoed: | |||||||||
| Bug Depends On: | |||||||||
| Bug Blocks: | 1236594, 1236597 | ||||||||
| Attachments: |
|
||||||||
Such a silly bug..
PriorityQueue.cpp:
void PriorityQueue::removeIf(Predicate p)
{
for (int priority = 0; priority < levels; ++priority) {
for (Deque::iterator i = messages[priority].begin(); i != messages[priority].end();) {
if ((*i)->status == QueuedMessage::AVAILABLE && p(**i)) {
(*i)->status = QueuedMessage::REMOVED; // Updates fifo index
i = messages[priority].erase(i);
clearCache();
} else {
++i;
}
}
}
fifo.recount();
fifo.clean();
}
Potential patch to be tried:
--- cpp/src/qpid/broker/PriorityQueue.cpp 2015-05-05 15:05:41.147559504 +0200
+++ cpp/src/qpid/broker/PriorityQueue.cpp.new 2015-05-21 16:26:05.634563035 +0200
@@ -137,7 +137,7 @@ void PriorityQueue::foreach(Functor f)
void PriorityQueue::removeIf(Predicate p)
{
- for (int priority = 0; priority < levels; ++priority) {
+ for (int priority = levels-1; priority >= 0; --priority) {
for (Deque::iterator i = messages[priority].begin(); i != messages[priority].end();) {
if ((*i)->status == QueuedMessage::AVAILABLE && p(**i)) {
(*i)->status = QueuedMessage::REMOVED; // Updates fifo index
Created attachment 1028336 [details]
patch proposal
Patch fixing the issue.
When calling both QMF methods queueMoveMessages or reroute, messages->removeIf is called either way. For PriorityQueue, PriorityQueue::removeIf is called.
Originally, this method traversed messages field from the lowest priorities to the highest. The fix just reverses the traversal from the highest to the lowest priorities.
I havent run any automatic tests, but both queueMoveMessages and reroute QMF methods applied to the reproducer work well.
Created attachment 1044619 [details]
sources of move_from_queue_to_another.cpp
To compile it:
g++ -Wall -lqpidclient -lqpidcommon -lqpidmessaging -lqpidtypes move_from_queue_to_another.cpp -o move_from_queue_to_another
An alternative way of moving messages is using qpid-tool:
qpid-tool -> wait 10 seconds -> list broker -> (get the ID of the broker, further I assume it is 122) -> call 122 queueMoveMessages q w 0 {}
Now messages are ordered from highest to lowest priority in the destination queue (after rerouting the priority queue content to the destination queue). Verified on Rhel6.6 (i686 and x86_64), Rhel5.11 (i386 and x86_64) and Rhel7.1-x86_64 on qpid-cpp-0.18-39. --> VERIFIED Hey Pavel. Could you please just sanity check the Release Note for this bug. Once verified, I'll add it into the linked errata. (In reply to Jared MORGAN from comment #7) > Hey Pavel. Could you please just sanity check the Release Note for this bug. > Once verified, I'll add it into the linked errata. Sounds fine, thanks! 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. https://rhn.redhat.com/errata/RHBA-2015-1234.html |
Description of problem: Having a priority queue with messages with different priorities, rerouting this queue content to a different queue (non-priority one), the destination queue has the messages ordered from the lowest priority to the highest. While the messages should be ordered from highest to lowest priority (or at least unordered like browsing the prio queue serves them). Version-Release number of selected component (if applicable): qpid-cpp-server-0.18-38 How reproducible: 100% Steps to Reproduce: service qpidd restart qpid-config add queue q --argument=qpid.priorities=10 qpid-config add queue w qpid-send -a q -m 1 --priority=2 qpid-send -a q -m 1 --priority=6 qpid-send -a q -m 1 --priority=4 qpid-receive -a "q; {mode:browse}" -m3 --print-content=no --print-headers=yes | grep Priority Priority: Priority: Priority: (they are 2,6,4) ~/Cpp_MRG/QMF_examples/move_from_queue_to_another q w 0 {_arguments:{destQueue:w, qty:0, srcQueue:q}, _method_name:queueMoveMessages, _object_id:{_object_name:org.apache.qpid.broker:broker:amqp-broker}} Response: OK qpid-receive -a "w; {mode:browse}" -m3 --print-content=no --print-headers=yes | grep Priority Priority: Priority: Priority: (they are 2,4,6) Actual results: queue w has messages with priorities 2,4,6 (in this ordering). Expected results: queue w should have messages with priorities 6,4,2 (in this ordering) Additional info: MRG 3.1 works well here, just a request of backporting / fixing the flaw on MRG 2.*.