On Monday, January 21, 2013 at 5:44 AM, Loic Dachary wrote:
>
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 01/21/2013 12:02 AM, Gregory Farnum wrote:
> > On Sunday, January 20, 2013 at 5:39 AM, Loic Dachary wrote:
> > > Hi,
> > >
> > > While working on unit tests for Throttle.{cc,h} I tried to figure out a
> > > use case related to the Throttle::wait method but couldn't
> > >
> > > https://github.com/ceph/ceph/pull/34/files#L3R258
> > >
> > > Although it was not a blocker and I managed to reach 100% coverage
> > > anyway, it got me curious and I would very much appreciate pointers to
> > > understand the rationale.
> > >
> > > wait() can be called to set a new maximum before waiting for all pending
> > > threads to get get what they asked for. Since the maximum has changed,
> > > wait() wakes up the first thread : the conditions under which it decided
> > > to go to sleep have changed and the conclusion may be different.
> > >
> > > However, it only does so when the new maximum is less than current one.
> > > For instance
> > >
> > > A) decision does not change
> > >
> > > max = 10, current 9
> > > thread 1 tries to get 5 but only 1 is available, it goes to sleep
> > > wait(8)
> > > max = 8, current 9
> > > wakes up thread 1
> > > thread 1 tries to get 5 but current is already beyond the maximum, it
> > > goes to sleep
> > >
> > > B) decision changes
> > >
> > > max = 10, current 1
> > > thread 1 tries to get 10 but only 9 is available, it goes to sleep
> > > wait(9)
> > > max = 9, current 1
> > > wakes up thread 1
> > > thread 1 tries to get 10 which is above the maximum : it succeeds because
> > > current is below the new maximum
> > >
> > > It will not wake up a thread if the maximum increases, for instance:
> > >
> > > max = 10, current 9
> > > thread 1 tries to get 5 but only 1 is available, it goes to sleep
> > > wait(20)
> > > max = 20, current 9
> > > does *not* wake up thread 1
> > > keeps waiting until another thread put(N) with N >= 0 although there now
> > > is 11 available and it would allow it to get 5 out of it
> > >
> > > Why is it not desirable for thread 1 to wake up in this case ? When
> > > debugging a real world situation, I think it would show as a thread
> > > blocked although the throttle it is waiting on has enough to satisfy its
> > > request. What am I missing ?
> > >
> > > Cheers
> > >
> > >
> > > Attachments:
> > > - loic.vcf
> >
> >
> >
> >
> > 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
>
> >
> > 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.
>
> I'm not sure if I should have posted the proposed Throttle unit test to the
> list instead of proposing it as a pull request
> https://github.com/ceph/ceph/pull/34
>
> What is best ?
Pull requests are good; you just sent it in on a weekend and we've all got a
queue before we evaluate code pulls. :)
Thanks!
-Greg
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
