Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Return FORBIDDEN if a user's batch is Over Quota #848

Merged
merged 4 commits into from
Oct 12, 2020
Merged

Conversation

jrconlin
Copy link
Member

@jrconlin jrconlin commented Oct 6, 2020

Issue #418

Description

If a user is over quota during a batch operation, the server would return OK with a list of failed ids. The client would retry them, leading to potentially a lot of useless network traffic.

Testing

Enable the QUOTA system.
Create a batch that contains more data than should be allowed by the QUOTA.
Confirm that the returned result is a 403 FORBIDDEN status with the body of 14

(Note, I used the spanner load test script for this.)

Issue(s)

Issue #418

@jrconlin jrconlin requested a review from a team October 6, 2020 00:29
Copy link
Contributor

@fzzzy fzzzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Wonderful 🎉

fzzzy
fzzzy previously approved these changes Oct 6, 2020
src/web/handlers.rs Outdated Show resolved Hide resolved
@jrconlin jrconlin marked this pull request as ready for review October 6, 2020 01:57
Copy link
Member

@pjenvey pjenvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could take the opportunity to fix #418 here but I'm fine with a quicker fix such as this. This does not fix #418 though, so let's log a "propagate quota errors in batch commit" issue for this PR (I really should have logged it for my comment describing the issue ) and fixup the commit message to point to it

src/web/handlers.rs Outdated Show resolved Hide resolved
@jrconlin
Copy link
Member Author

jrconlin commented Oct 9, 2020

updated commit to reflect that this is an "issue" resolution to #418 rather than "closes"

@jrconlin jrconlin requested review from pjenvey and a team October 9, 2020 19:01
@jrconlin jrconlin merged commit d24dcdb into master Oct 12, 2020
@jrconlin jrconlin deleted the bug/418 branch October 12, 2020 21:24
@jrconlin jrconlin mentioned this pull request Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants