Draft
Conversation
There are o(100) `ets:lookup/2` function calls in order to do a single n=3 counter update. This is a first path to try and reduce the number of individual calls for bucket properties. Note each request for a typed bucket property currently requires 4 x `ets:lookup/2`.
2 x bucket-property requests were required as part of aae_update. Also property requests for object update/merge.
Rather than repeatedly calling `lists:keyfind/2` or `proplists:get_value/3` However, there are issues with the way bucket properties are merged with defaults. Depending on the function used, thye could be merged using ukeysort - so there was a unique enty for every property, or keysort. If it was merged with keysort there would be multiple properties, but only the first would be found (by proplists:get_value or lists:keyfind). This now tries to standardise bucket property fetches (that may impact the PUT path) to produce genuinely merged properties.
The coordinator should detect this. If it has been accepted elsewhere (i.e. it is a replicated PUT, or it has been added on a misconfigured node), then accept it here. Changes the layout of the code to separate the size_check and sibling_check rather than split the encode_and_put fun. This should reduce the number of calls to application:get_env (and consequent calls to ets:lookup/2)
Walk the options on prepare (as well as validate) within the PUT_FSM Fix the object_format to v1 (tests will need to intercept the function) Only perform the size and sibling checks on a normal PUT at the coordinator. Allow bucket properties to be kept in a map at the riak_kv_put_fsm, and still have options fetched. Only calculate node_confirms, if node_confirms is a requirement for `enough`. Calculate it more efficiently when required Revert changes to remove put functions from riak_client - required for riak_test.
lists:keyfind is done in a NIF and is faster for small amounts of properties (i.e. the three being extracted in riak_kv_vnode), where the input list or properties will be larger than the output. There is no advantage from using a map.
This reverts commit dc161a4.
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduce the number of re-requests for the bucket property in the PUT path.
Running the
general_counter_perftest, this branch reduces the calls toets:lookup/2from 115 to 65. Primarily by fetching the bucket properties once at the start of the PUT, and re-using the properties throughout the PUT process (rather than re-requesting them via ETS lookups).