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

Add bucket number limit check in Riak CS process #950

Merged
merged 4 commits into from
Aug 20, 2014

Conversation

kuenishi
Copy link
Contributor

The size of an user record is proportional to bucket number. If too
many buckets were created, then the user's record gets large and
access to anything owned by the user gets slow. This commit adds
bucket number limitation in bucket creation, to prevent user record to
grow too large. As this is not a strict check in a critical section,
there might be a race condition in multiple bucket creations where
they validate total number of owned buckets and then send requests to
Stanchion. But bucket creation is a rare operation, this won't happen
so frequently.

The case where this commit can't prevent growth of an user object is,
when buckets are frequently created and deleted; in that case the user
record may have many deleted bucket records. But that will be pruned.

Addressed by #927

The size of an user record is proportional to bucket number. If too
many buckets were created, then the user's record gets large and
access to anything owned by the user gets slow. This commit adds
bucket number limitation in bucket creation, to prevent user record to
grow too large. As this is not a strict check in a critical section,
there might be a race condition in multiple bucket creations where
they validate total number of owned buckets and then send requests to
Stanchion. But bucket creation is a rare operation, this won't happen
so frequently.

The case where this commit can't prevent growth of an user object is,
when buckets are frequently created and deleted; in that case the user
record may have many deleted bucket records. But that will be pruned.
@kuenishi kuenishi added this to the 1.5.1 milestone Aug 12, 2014
@kuenishi kuenishi changed the title Add bucket number limit check in Riak CS process [wip]Add bucket number limit check in Riak CS process Aug 12, 2014
@kuenishi
Copy link
Contributor Author

TODO: add tests

@kuenishi kuenishi changed the title [wip]Add bucket number limit check in Riak CS process Add bucket number limit check in Riak CS process Aug 12, 2014
@shino
Copy link
Contributor

shino commented Aug 19, 2014

Can we set unlimited?

@kuenishi
Copy link
Contributor Author

100 =< unlimited is always true (applies to any integers). I found it jut now.

@shino
Copy link
Contributor

shino commented Aug 19, 2014

Nice 😄

@shino
Copy link
Contributor

shino commented Aug 19, 2014

Tight corner case.
When disable_local_bucket_check=true and a user has 100 buckets then request to create bucket with existing bucket name results in 400 TooManyBuckets.
If disable_local_bucket_check=false (default), the status code is 200.

A workaround is ... delete one bucket :)
If this is hard to fix, then I think the workaround is acceptable.

@kuenishi
Copy link
Contributor Author

Added my workaround. Allow 101st bucket like S3 and error on 102nd bucket creation. Also, I cannot add disable-local-bucket-check orelse because we don't do bucket number check in Stanchion.

@kuenishi kuenishi force-pushed the feature/limit-bucket-numbers branch from 4f62e63 to 1983e0e Compare August 20, 2014 00:50
@kuenishi
Copy link
Contributor Author

Reverted my workaround. If allowing 101st bucket needed, just set {max_buckets_per_user, 101} in app.config

borshop added a commit that referenced this pull request Aug 20, 2014
Add bucket number limit check in Riak CS process

Reviewed-by: shino
@shino
Copy link
Contributor

shino commented Aug 20, 2014

@borshop merge

@borshop borshop merged commit 5a3aaa1 into release/1.5 Aug 20, 2014
@kuenishi kuenishi deleted the feature/limit-bucket-numbers branch August 20, 2014 03:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants