New system for quotas needed

RESOLVED FIXED

Status

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: telliott, Assigned: telliott)

Tracking

({push-needed})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
The new algorithm we're implementing is documented at https://wiki.mozilla.org/Labs/Weave/QuotaAlgo

Expected initial quota is 8M,
(Assignee)

Comment 1

9 years ago
Created attachment 456512 [details] [diff] [review]
Patch to implement quotas according to https://wiki.mozilla.org/Labs/Weave/QuotaAlgo
Attachment #456512 - Flags: review?(tarek)
The code changes looks fine, but we need to add some tests in the python test suite for these new API. 

Do you want to add the tests in the same patch ? If not I'll +R and I guess we can work on a new patch here.
(Assignee)

Comment 3

9 years ago
I'm actually not sure how to test a lot of this as part of the test suite. We can't have it as part of the buildbot - since we wouldn't want to try to fill an account each time and we can't reduce quota size on the live server! We could lower a threshold on stage, but unless we're very careful, it'll start messing with the other tests.

We should have a test for the actual API call, but I didn't want to mess with that until you'd got everything else into shape and into hg. Feel free to add it.
yes, this would definitely be running only on stage. I think its important to have it so the new python version can rely on this test. How would it mess with other tests btw ? since the teardown is supposedly putting back the environment in the previous state.
Attachment #456512 - Flags: review?(tarek) → review+
(Assignee)

Comment 5

9 years ago
If we set the quota sufficiently low that we can reasonably test it, it's possible that it'll trigger during some of the other tests (hence the "very careful" caveat)
Presence of the X-Weave-Quota header will indicate that the user is over the lower threshold. Its value currently is the number of used space in bytes. I think the amount of total available space should also be included, e.g.:

  X-Weave-Quota: <bytes used> <total bytes available>

Furthermore, when info/collection_usage is queried, one would probably also want to know the total available space. We could either enhance the return value of info/collection_usage with that info or add another API call, e.g. info/quota. I would prefer the former.
The total bytes available is not something that will change very often, so maybe having X-Weave-Quota return the bytes *left* instead would be more useful from a client point of view.
And more details on an explicit API call.
(In reply to comment #7)
> The total bytes available is not something that will change very often, so
> maybe having X-Weave-Quota return the bytes *left* instead would be more useful
> from a client point of view.

Excellent point. +1

> And more details on an explicit API call.

Well, what other details are there? info/collection_usage already gives us how much we're using. We only need one more info at that point, either how much space we've got left or what the total space is. It seems a bit silly to me adding another API call to the server just for that.

I propose adding another entry to the JSON hash returned by info/collection_usage. Toby was worried about potentially clashing with collection names, but we could easily just use a key that's impossible for a collection name, e.g. put a slash in it. For instance, info/collection_usage could return:

  {"tabs": 1234,
   "bookmarks: 1234,
   ...
   "quota/usage": 1234567,
   "quota/total": 1234567890}
(Assignee)

Comment 9

9 years ago
http://hg.mozilla.org/services/sync-server/rev/e67a0a339554

I've added this code to the tip now. It won't activate unless WEAVE_QUOTA is set in constants.

I changed the name of the header to X-Weave-Quota-Remaining so that it wouldn't be ambiguous. It'll start to show up when you hit the lower bound. Reluctant to add it to the collection hash, as that's stretching the semantics pretty painfully, but we can talk about some of that at the onsite.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: push-needed
Resolution: --- → FIXED
Blocks: 580672
Blocks: 592376
Duplicate of this bug: 602307
Duplicate of this bug: 616392
You need to log in before you can comment on or make changes to this bug.