On Thu, Jun 23, 2011 at 8:53 AM, Jim Schutt <jaschut@xxxxxxxxxx> wrote:
> Hi Colin,
>
> Colin McCabe wrote:
>>
>> On Wed, Jun 22, 2011 at 11:26 AM, Jim Schutt <jaschut@xxxxxxxxxx> wrote:
>>>
>>> This will enable SimpleMessenger to send keepalives to clients while
>>> blocked in the policy throttler.
>>>
>>> Signed-off-by: Jim Schutt <jaschut@xxxxxxxxxx>
>>> ---
>>> src/common/Throttle.h | 40 ++++++++++++++++++++++++++++++++++++++++
>>> 1 files changed, 40 insertions(+), 0 deletions(-)
>>
>> This looks good. Only a small nitpick-- it's better not to pass
>> utime_t by value, since it's a struct. There's a bunch of other code
>> doing it, but it's better to pass a const reference. Also, I guess we
>> need to rebase this because g_clock has been replaced by
>> ceph_clock_now() in master. Should be easy though.
>
> Given Sage's much cleaner approach to fixing the osd reset
> issue, is this change useful on its own? If so, I'm happy
> to fix it up and resubmit.
I have a feeling that adding timeouts to the Throttler would be useful
in general. I guess we'll probably wait until we have a user for the
function first, though.
We should definitely pull the first (cleanup) patch for Throttle.h, though.
sincerely,
Colin
>
> Let me know.
>
> -- Jim
>
>>
>> cheers,
>> Colin
>>
>>
>>> diff --git a/src/common/Throttle.h b/src/common/Throttle.h
>>> index 8fd2625..5a028cc 100644
>>> --- a/src/common/Throttle.h
>>> +++ b/src/common/Throttle.h
>>> @@ -27,6 +27,9 @@ private:
>>> ((c < max && count + c > max) || // normally stay under max
>>> (c >= max && count > max)); // except for large c
>>> }
>>> +
>>> + /* Returns true if it had to block/wait, false otherwise.
>>> + */
>>> bool _wait(int64_t c) {
>>> bool waited = false;
>>> if (_should_wait(c)) {
>>> @@ -44,6 +47,28 @@ private:
>>> return waited;
>>> }
>>>
>>> + /* Returns true if it timed out while blocked/waiting,
>>> + * false otherwise.
>>> + */
>>> + bool _timed_wait(utime_t interval, int64_t c) {
>>> + if (_should_wait(c)) {
>>> + utime_t timeout_at = g_clock.now() + interval;
>>> + waiting += c;
>>> + do {
>>> + if (cond.WaitUntil(lock, timeout_at)) {
>>> + waiting -= c;
>>> + return true;
>>> + }
>>> + } while (_should_wait(c));
>>> + waiting -= c;
>>> +
>>> + // wake up the next guy
>>> + if (waiting)
>>> + cond.SignalOne();
>>> + }
>>> + return false;
>>> + }
>>> +
>>> public:
>>> int64_t get_current() {
>>> Mutex::Locker l(lock);
>>> @@ -79,6 +104,21 @@ public:
>>> count += c;
>>> }
>>>
>>> + /* Returns true if it got the requested amount within
>>> + * the specified interval, false otherwise.
>>> + */
>>> + bool timed_get(utime_t interval, int64_t c = 1, int64_t m = 0) {
>>> + assert(c >= 0);
>>> + Mutex::Locker l(lock);
>>> + if (m) {
>>> + assert(m > 0);
>>> + _reset_max(m);
>>> + }
>>> + if (_timed_wait(interval, c)) return false;
>>> + count += c;
>>> + return true;
>>> + }
>>> +
>>> /* Returns true if it successfully got the requested amount,
>>> * or false if it would block.
>>> */
>>> --
>>> 1.7.1
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
