-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15094/
-----------------------------------------------------------
(Updated Nov. 1, 2013, 1:31 p.m.)
Review request for qpid, Gordon Sim and Ted Ross.
Changes
-------
Updated patch reflecting Gordon's comment (option 2 chosen) and Ken's idea from
pseudo-code.
The patch changes behaviour in two cases:
$ qpid-config add queue MyQueue15 --durable --max-queue-count=100
--flow-resume-count=200
Failed: Exception: Exception from Agent: {u'error_code': 7, u'error_text':
'invalid-argument: Queue "MyQueue15": qpid.flow_resume_count=200 must be less
or equal to qpid.flow_stop_count=80
(/data_xfs/qpid-trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp:52)'}
$
This in my opinion makes sense to prohibit (while unpatched broker allows such
queue, silently ignoring user's flow-resume-count).
$ qpid-config add queue MyQueue27 --durable --flow-stop-size=90
Failed: Exception: Exception from Agent: {u'error_code': 7, u'error_text':
'invalid-argument: Queue "MyQueue27": qpid.flow_resume_size=73400320 must be
less or equal to qpid.flow_stop_size=90
(/data_xfs/qpid-trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp:52)'}
$
Similar example where the patched broker follows flow setup rules just more
strictly (note: 73400320 is (default) 70% of (default) 104857600 that is
default-queue-limit).
BUT: with this patch, unit tests timeout without any obvious reason. Debugging
it gives no clue to me:
ctest --force-new-ctest-process -vv --debug -R unit_test
..
/builddir/build/BUILD/cmake-2.6.4/Source/cmCTest.cxx:1185
0% 10 20 30 40 50 60 70 80 90 100%
|----|----|----|----|----|----|----|----|----|----|
(and now nothing).
Any idea what could be wrong? (I had to change
cpp/src/tests/QueueFlowLimitTest.cpp to reflect QueueFlowLimit class API
changes, cant be relevant?)
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
