qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pavel Moravec" <pmora...@redhat.com>
Subject Re: Review Request 15094: QPID-5278 & QPID-5281: Queue flow limit validation ignores size parameters & Creating a queue with invalid settings results in no queue but only its management object exists
Date Tue, 19 Nov 2013 11:55:37 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15094/
-----------------------------------------------------------

(Updated Nov. 19, 2013, 11:55 a.m.)


Review request for qpid, Gordon Sim and Ted Ross.


Changes
-------

New patch reflects Gordon's comments.


Bugs: https://issues.apache.org/jira/browse/QPID-5278
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5278


Repository: qpid


Description
-------

Trivial copy&paste bug fixed. Anyway I am not sure what the proper broker behaviour in
one of tested use cases:

qpid-config add queue WrongQueue5 --max-queue-count=100 --flow-resume-count=50 --durable

should be. Currently, the flow-resume-count parameter is ignored as broker logs:

[Broker] info Queue "WrongQueue6": Flow limit created: flowStopCount=80, flowResumeCount=70,
flowStopSize=83886080, flowResumeSize=73400320

>From the source code, I have a feeling this is intentional - flow control defaults are
overwritten only if flow-stop-[count|size] parameter is used. Am I right? If so, then:
1) there should be a warning - like the one in patch.
2) queue settings (as provided in "qpid-config queues" output) should be modified accordingly
- currently it copy&paste the user settings that is then ignored. That means, "qpid-config
queues" provides wrong information. (this is not covered by the patch)

Any thoughts?


Diffs (updated)
-----

  /trunk/qpid/cpp/src/qpid/broker/QueueFactory.cpp 1537924 
  /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.h 1537924 
  /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1537924 
  /trunk/qpid/cpp/src/tests/QueueFlowLimitTest.cpp 1537924 

Diff: https://reviews.apache.org/r/15094/diff/


Testing
-------


Thanks,

Pavel Moravec


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message