Remove the extension QuotaService::SustainedLimit and its last client, the

Remove the extension QuotaService::SustainedLimit and its last client, the
storage.sync MAX_SUSTAINED_WRITE_OPERATIONS_PER_MINUTE limit. I'll be relaxing
the quota over a few CLs and having 2 limiting mechanisms just complicates
things.

BUG=406406
R=zea@chromium.org

Committed: https://crrev.com/2c396aacf26fefdd174c86fb7a83c05c2212bd1d
Cr-Commit-Position: refs/heads/master@{#302352}

[not at google - send to devlin, 2014-10-30 22:37:27]

My plan:
1. This CL. I chose to remove the Sustained limit because it's hard to
understand vs just ops-per-hour.
2. Change 1000/hour to 10,000/hour or 200/minute or something.
3. Remove the logic where once-failed-forever-fail (because people never restart
Chrome, and it's a PITA to have things always broken) and just reset the quota
every <time period>.

Alternative plan:
1. Similar to this CL, but remove the per-hour quota instead.
2. Raise the 10/minute sustained quota to 200, then try to explain it better.
3. Same.

What sounds most sensible to you?

[Nicolas Zea, 2014-10-31 21:06:18]

This mostly LG. For my understanding, what's the once-failed-forever-fail issue?
Do apps not retry periodically when throttled?

[not at google - send to devlin, 2014-10-31 21:20:33]

On 2014/10/31 21:06:18, Nicolas Zea wrote:
> This mostly LG. For my understanding, what's the once-failed-forever-fail
issue?
> Do apps not retry periodically when throttled?

It's this:
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...

Once an extension exceeds quota it is never allowed again. I'm not sure how much
code I would need to modify to change that behaviour.

[Nicolas Zea, 2014-10-31 21:29:03]

On 2014/10/31 21:20:33, kalman wrote:
> On 2014/10/31 21:06:18, Nicolas Zea wrote:
> > This mostly LG. For my understanding, what's the once-failed-forever-fail
> issue?
> > Do apps not retry periodically when throttled?
> 
> It's this:
>
https://code.google.com/p/chromium/codesearch#chromium/src/extensions/browser...
> 
> Once an extension exceeds quota it is never allowed again. I'm not sure how
much
> code I would need to modify to change that behaviour.

Got it. LGTM.

Where did the 10000/hr vs 200/minute values come from btw? Or are they just
arbitrary?

[not at google - send to devlin, 2014-10-31 21:35:16]

I think they're arbitrary and conservative, and we never changed them.

So this approach seems right? I'm really open to whichever seems best to you.

[Nicolas Zea, 2014-10-31 21:48:50]

On 2014/10/31 21:35:16, kalman wrote:
> I think they're arbitrary and conservative, and we never changed them.
> 
> So this approach seems right? I'm really open to whichever seems best to you.

Yeah, I agree that ops/hour or ops/minute is easier to understand than the
notion of rate over a sustained period of time. And getting rid of the
once-failed-forever-fail thing definitely seems the right way to go. I'm not
entirely sure what the proper ops/minute value should be. 

200/minute seems okay, although to be on the safe side the per-hour amount
should maybe be more conservative (emulating the "sustained" notion earlier).
What is the window over which these limits are enforced?

[not at google - send to devlin, 2014-10-31 21:57:10]

On 2014/10/31 21:48:50, Nicolas Zea wrote:
> On 2014/10/31 21:35:16, kalman wrote:
> > I think they're arbitrary and conservative, and we never changed them.
> > 
> > So this approach seems right? I'm really open to whichever seems best to
you.
> 
> Yeah, I agree that ops/hour or ops/minute is easier to understand than the
> notion of rate over a sustained period of time. And getting rid of the
> once-failed-forever-fail thing definitely seems the right way to go. I'm not
> entirely sure what the proper ops/minute value should be. 
> 
> 200/minute seems okay, although to be on the safe side the per-hour amount
> should maybe be more conservative (emulating the "sustained" notion earlier).
> What is the window over which these limits are enforced?

So have both a per-hour and per-minute limit? I'm not sure what you mean by
window... do you mean like when does the time period start and end? I'm not
sure, I think on the first event that occurs the time starts, but that's just
from reading the code. It's not really documented and I didn't write it
initially.

[Nicolas Zea, 2014-10-31 22:14:34]

On 2014/10/31 21:57:10, kalman wrote:
> On 2014/10/31 21:48:50, Nicolas Zea wrote:
> > On 2014/10/31 21:35:16, kalman wrote:
> > > I think they're arbitrary and conservative, and we never changed them.
> > > 
> > > So this approach seems right? I'm really open to whichever seems best to
> you.
> > 
> > Yeah, I agree that ops/hour or ops/minute is easier to understand than the
> > notion of rate over a sustained period of time. And getting rid of the
> > once-failed-forever-fail thing definitely seems the right way to go. I'm not
> > entirely sure what the proper ops/minute value should be. 
> > 
> > 200/minute seems okay, although to be on the safe side the per-hour amount
> > should maybe be more conservative (emulating the "sustained" notion
earlier).
> > What is the window over which these limits are enforced?
> 
> So have both a per-hour and per-minute limit? I'm not sure what you mean by
> window... do you mean like when does the time period start and end? I'm not
> sure, I think on the first event that occurs the time starts, but that's just
> from reading the code. It's not really documented and I didn't write it
> initially.

Yeah, I think having both makes sense. A high per-minute allows for some
burstiness, while a more conservative per-hour means it shouldn't be held.

Looking at the code it seems like its not really using a sliding window approach
to the quota (which is what I've often seen for rate limits), but instead just
resets the token bucket count every |interval|, where interval would be a minute
or an hour. That's fine for our usage I think.

[not at google - send to devlin, 2014-10-31 22:25:04]

Alright, sounds good. I'll do that. Expect a few more patches.

[not at google - send to devlin, 2014-10-31 22:25:09]

The CQ bit was checked by kalman@chromium.org

[commit-bot: I haz the power, 2014-10-31 22:26:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/685603003/20001

[commit-bot: I haz the power, 2014-10-31 23:31:46]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-31 23:32:28]

Patchset 2 (id:??) landed as
https://crrev.com/2c396aacf26fefdd174c86fb7a83c05c2212bd1d
Cr-Commit-Position: refs/heads/master@{#302352}