Only fetch bucket properties once per request

[martinsumner]

Using the general_api_perf test, the ets:lookup function is now prominent in the profile of various flavours of read/write test.

The microseconds per call is bigger than expected (0.50), but alos the frequency of the call.

The bucket properties for non-typed buckets are fetched from ETS - but for every GET/PUT they must be fetched twice. They are first fetched by riak_client to determine if this is a normal or consistent GET/PUT, then by the FSM to get the properties to use in the processing of the GET/PUT.

The code should be refactored to only fetch bucket properties once. In the longer term, perhaps persistent_term not ets shoudd be used for holding bucket properties.

It should also be noted that bucket properties are always remerged with default properties every request:

riak_kv/src/riak_kv_get_fsm.erl

Lines 259 to 268 in 644d04e

	BucketProps = riak_core_bucket:get_bucket(Bucket),
	%% typed buckets never fall back to defaults
	Props =
	case is_tuple(Bucket) of
	false ->
	lists:keymerge(1, lists:keysort(1, BucketProps),
	lists:keysort(1, DefaultProps));
	true ->
	BucketProps
	end,

This should probably happen once on storage, not every time on read.

Also of potential relevance is the riak_object:dvv_enabled/1 check:

riak_kv/src/riak_object.erl

Lines 973 to 980 in 752c754

	%% @private is dvv enabled on this node?
	-spec dvv_enabled(bucket()) -> boolean().
	dvv_enabled(Bucket) ->
	BProps = riak_core_bucket:get_bucket(Bucket),
	%% default to `true`, since legacy buckets should have `false` by
	%% default, and typed should have `true` and `undefined` is not a
	%% valid return.
	proplists:get_value(dvv_enabled, BProps, true).

Could this be optimised to not require repeated bucket property fetched each time it is called? [Note - this will normally be only called once for each PUT, at the coordinator]

[martinsumner]

To fetch the bucket properties for a typed bucket requires 4 ets:lookup/2 calls. The properties for the Type and the Bucket are both be fetched from the metadata manager separately, and each fetch from the metadata manager requires a lookup to find the table, and a lookup in the table.

See https://github.com/OpenRiak/riak_core/blob/openriak-3.2/src/riak_core_metadata_manager.erl#L619-L643 for the ets table lookup.

For a PUT there are at least 4 + N requests for the bucket properties:

• To check if the bucket is consistent, if and only-if not-modified or non-match is set (within the API);
• To check if the bucket is consistent, before choosing the PUT path in riak_client;
• To check if the bucket is write_once, before choosing the PUT path in riak_client;
• Within the PUT_FSM to get the properties for the PUT;
• On every vnode receiving the PUT to extract allow_mult and
• On the coordinating vnode when incrementing the vclock (to check for dvv_enabled);
• On any other vnodes if merge conflicts require further updates (to check for dvv_enabled).

So for a single PUT on a typed bucket there are at least 28 ets:lookup/2 calls (with a n_val of 3).

There is also an ets:lookup/2 required in exometer_core for every stat update - and there will be at least N + 2 stat updates per PUT.

[WarpEngineer]

Would this be related to PR #58?