-
Notifications
You must be signed in to change notification settings - Fork 233
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 warning/max object limits #680
Conversation
Reading or writing objects whose binary representation is bigger than warn_object_size will generate a warning in the logs identifying the bucket/key and size for the object. Putting an object whose binary size is greater than max_object_size will fail. We will now warn if writing more than warn_siblings and fail to put if more than max_siblings.
Configuration schema changes for this are in basho/riak#397 and tests in basho/riak_test#398 |
@@ -1701,13 +1711,55 @@ return_encoded_binary_object(Method, EncodedObject) -> | |||
{{error, Reason::term(), UpdModState::term()}, EncodedObj::binary()}. | |||
|
|||
encode_and_put(Obj, Mod, Bucket, Key, IndexSpecs, ModState) -> | |||
NumSiblings = riak_object:value_count(Obj), | |||
case NumSiblings > app_helper:get_env(riak_kv, max_siblings) of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we handle the case that someone has set the number to some non_integer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any non integer term is larger than any integer in Erlang. So it handles itself: it would behave as if there wasn't a threshold
A potential optimization here would be to stuff all of these values into a 4-tuple and have a function to unpack them. As yet I have no idea if all of the extra ets calls are going to be an issue. Code looks good, on to some testing. |
You would be surprised by how efficient these ETS lookups are when measured with DTrace in the general case. Still, I added them as is because I have some plans to optimize configuration that I will tell you about elsewhere. It would be a post-ricon thing and would benefit from keeping plain app_helper:get_env calls instead of trying to optimize by hand. I have a todo item on that. |
No noticeable performance impacts in its current form, code seems sensible and does what is described. |
Add warning/max object limits
CSEs have wanted more visibility into sibling and object size explosions for a while. A warning threshold would make sure that the logs can be used to pinpoint or event prevent size and sibling number explosions.
We have also discussed putting hard limits on size and sibling number. Writes would fail for very large objects or objects with too many siblings. This should only be used as a last resort to prevent a cluster from experiencing severe performance problems that can result in cascaded deaths even