On 02/05/2013 01:22 AM, Gregory Farnum wrote: > Loic, > Sorry for the delay in getting back to you about these patches. :( I > finally got some time to look over them, and in general it's all good! > I do have some comments, though. > > On Mon, Jan 21, 2013 at 5:44 AM, Loic Dachary <[email protected]> wrote: >>> Looking through the history of that test (in _reset_max), I think it's an >>> accident and we actually want to be waking up the front if the maximum >>> increases (or possibly in all cases, in case the front is a very large >>> request we're going to let through anyway). Want to submit a patch? :) >> :-) Here it is. "make check" does not complain. I've not run teuthology + >> qa-suite though. I figured out how to run teuthology but did not yet try >> qa-suite. >> >> http://marc.info/?l=ceph-devel&m=135877502606311&w=4 > > This patch to reverse the conditional is obviously fine. > >>> The other possibility I was trying to investigate is that it had something >>> to do with handling get() requests larger than the max correctly, but I >>> can't find any evidence of that one... >> I've run the Throttle unit tests after uncommenting >> https://github.com/ceph/ceph/pull/34/files#L3R269 >> and commenting out >> https://github.com/ceph/ceph/pull/34/files#L3R266 >> and it passes. > > Regarding these unit tests, I have a few questions which I left on > Github. Can you address them and then give a single pull request which > includes both the Throttle fix and the tests? :)
I will, thanks :-) -- Loïc Dachary, Artisan Logiciel Libre
signature.asc
Description: OpenPGP digital signature
