-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15094/#review27777
-----------------------------------------------------------
Good find.
There may be a better approach to configuring these limits than what's there
today (mea culpa). Perhaps a better implementation would take into account
both the default ratios (if present) and any specific settings (if present -
give these priority). Once all that is done, verify that both a Stop and
Resume are either given (non-zero), or not (zero) for each metric, failing if
only one of the stop/resume pair was set.
Quick pseudo code of the idea:
flowStopSize = flowResumeSize = 0;
flowStopCount = flowResumeCount = 0;
if (defaultFlowStopRatio) { // broker has a default ratio setup...
uint64_t maxByteCount = settings.maxDepth.hasSize() ?
settings.maxDepth.getSize() : defaultMaxSize;
flowStopSize = (uint64_t)(maxByteCount * (defaultFlowStopRatio/100.0) +
0.5);
uint32_t maxMsgCount = settings.maxDepth.hasCount() ?
settings.maxDepth.getCount() : 0;
flowStopCount = (uint32_t)(maxMsgCount * (defaultFlowStopRatio/100.0) +
0.5);
}
if (defaultFlowResumeRatio) {
uint64_t maxByteCount = settings.maxDepth.hasSize() ?
settings.maxDepth.getSize() : defaultMaxSize;
flowResumeSize = (uint64_t)(maxByteCount *
(defaultFlowResumeRatio/100.0) + 0.5);
uint32_t maxMsgCount = settings.maxDepth.hasCount() ?
settings.maxDepth.getCount() : 0;
flowResumeCount = (uint32_t)(maxMsgCount *
(defaultFlowResumeRatio/100.0));
}
if (settings.flowStop.hasSize()
flowStopSize = settings.flowStop.getSize()
if (settings.flowResume.hasSize()
flowResumeSize = settings.flowResume.getSize()
And repeat for the flow count parameters. Then validate that each pair is
either set or unset:
if ((flowResumeSize && !flowStopSize) || (!flowResumeSize && flowStopSize))
Log error
flowResumeSize = flowStopSize = 0;
And repeat for flow count...
- Kenneth Giusti
On Oct. 30, 2013, 12:41 p.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15094/
> -----------------------------------------------------------
>
> (Updated Oct. 30, 2013, 12:41 p.m.)
>
>
> Review request for qpid, Gordon Sim and Ted Ross.
>
>
> 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
> -----
>
> /trunk/qpid/cpp/src/qpid/broker/QueueFlowLimit.cpp 1535368
>
> Diff: https://reviews.apache.org/r/15094/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Pavel Moravec
>
>
