----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15094/#review29106 -----------------------------------------------------------
Ship it! Nice work! - Gordon Sim On Nov. 19, 2013, 11:55 a.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > 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. > > > 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/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 > >
