Permalink
Browse files

Generalize the coverage behavior used to list keys and move much

of the coverage work into riak_core.

Fixes: az496

This pull request generalizes the coverage behavior used by key
listing and makes it directly usable for other operations.

Also added function to riak_core_ring to determine the ring index that
is the successor of a chashed index value directly given the number of ring
partitions instead of having to calculate the entire preference
list. There is also a related function added to the chash module and
a quickcheck test to exercise this function.

This pull request also includes some changes to tidy up some of the
chash and riak_core_ring modules by adding some formatting, converting
edoc to specs, etc.
  • Loading branch information...
1 parent 945fd24 commit 274db4afd3653962de83785f43c3b340cd42bbbf Kelly McLaughlin committed Jun 23, 2011
View
@@ -21,6 +21,8 @@
riak_core_cinfo_core,
riak_core_claim,
riak_core_config,
+ riak_core_coverage_fsm,
+ riak_core_coverage_plan,
riak_core_eventhandler_guard,
riak_core_eventhandler_sup,
riak_core_gossip,
@@ -14,11 +14,16 @@
sender=ignore :: sender(),
request :: vnode_req()}).
+-record(riak_coverage_req_v1, {
+ index :: partition(),
+ keyspaces :: [{partition(), [partition()]}],
+ sender=ignore :: sender(),
+ request :: vnode_req()}).
-record(riak_core_fold_req_v1, {
foldfun :: fun(),
acc0 :: term()}).
-define(VNODE_REQ, #riak_vnode_req_v1).
+-define(COVERAGE_REQ, #riak_coverage_req_v1).
-define(FOLD_REQ, #riak_core_fold_req_v1).
-
View
@@ -1,6 +1,8 @@
{erl_first_files, ["src/gen_nb_server.erl", "src/gen_server2.erl"]}.
{cover_enabled, true}.
{edoc_opts, [{preprocess, true}]}.
+{erl_opts, [debug_info, warnings_as_errors]}.
+
{deps, [
{protobuffs, "0.6.0", {git, "git://github.com/basho/erlang_protobuffs",
{tag, "protobuffs-0.6.0"}}},
View
@@ -2,7 +2,7 @@
%%
%% chash: basic consistent hashing
%%
-%% Copyright (c) 2007-2010 Basho Technologies, Inc. All Rights Reserved.
+%% Copyright (c) 2007-2011 Basho Technologies, Inc. All Rights Reserved.
%%
%% This file is provided to you under the Apache License,
%% Version 2.0 (the "License"); you may not use this file
@@ -24,6 +24,9 @@
%% coincides with SHA-1 hashes, and so any two keys producing the same
%% SHA-1 hash are considered identical within the ring.
%%
+%% Warning: It is not recommended that code outside this module make use
+%% of the structure of a chash.
+%%
%% @reference Karger, D.; Lehman, E.; Leighton, T.; Panigrahy, R.; Levine, M.;
%% Lewin, D. (1997). "Consistent hashing and random trees". Proceedings of the
%% twenty-ninth annual ACM symposium on Theory of computing: 654~663. ACM Press
@@ -33,72 +36,150 @@
-author('Justin Sheehy <justin@basho.com>').
-author('Andy Gross <andy@basho.com>').
--export([fresh/2,update/3,lookup/2,members/1,size/1,nodes/1,
- successors/2,successors/3,
- predecessors/2,predecessors/3,
- contains_name/2,key_of/1,
- merge_rings/2]).
+-export([contains_name/2,
+ fresh/2,
+ lookup/2,
+ key_of/1,
+ members/1,
+ merge_rings/2,
+ next_index/2,
+ nodes/1,
+ predecessors/2,
+ predecessors/3,
+ ring_increment/1,
+ size/1,
+ successors/2,
+ successors/3,
+ update/3]).
+
+-export_type([chash/0]).
-define(RINGTOP, trunc(math:pow(2,160)-1)). % SHA-1 space
+
+-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
+-endif.
-% @type chash() = {NumPartitions, [NodeEntry]}
-% NumPartitions = integer()
-% NodeEntry = {IndexAsInt, Node}
-% IndexAsInt = integer()
-% Node = chash_node().
-% It is not recommended that code outside this module make use
-% of the structure of a chash.
-
-% @type index() = binary().
-% Indices into the ring, used as keys for object location, are binary
-% representations of 160-bit integers.
-
-% @type chash_node() = term().
-% A Node is the unique identifier for the owner of a given partition.
-% An Erlang Pid works well here, but the chash module allows it to
-% be any term.
-
-% @doc Create a brand new ring. The size and seednode are specified;
-% initially all partitions are owned by the seednode. If NumPartitions
-% is not much larger than the intended eventual number of
-% participating nodes, then performance will suffer.
-% @spec fresh(NumPartitions :: integer(), SeedNode :: chash_node()) -> chash()
+-type chash() :: {num_partitions(), [node_entry()]}.
+%% A Node is the unique identifier for the owner of a given partition.
+%% An Erlang Pid works well here, but the chash module allows it to
+%% be any term.
+-type chash_node() :: term().
+%% Indices into the ring, used as keys for object location, are binary
+%% representations of 160-bit integers.
+-type index() :: binary().
+-type index_as_int() :: integer().
+-type node_entry() :: {index_as_int(), chash_node()}.
+-type num_partitions() :: integer().
+
+%% ===================================================================
+%% Public API
+%% ===================================================================
+
+%% @doc Return true if named Node owns any partitions in the ring, else false.
+-spec contains_name(Name :: chash_node(), CHash :: chash()) -> boolean().
+contains_name(Name, CHash) ->
+ {_NumPartitions, Nodes} = CHash,
+ [X || {_,X} <- Nodes, X == Name] =/= [].
+
+%% @doc Create a brand new ring. The size and seednode are specified;
+%% initially all partitions are owned by the seednode. If NumPartitions
+%% is not much larger than the intended eventual number of
+%% participating nodes, then performance will suffer.
+-spec fresh(NumPartitions :: num_partitions(), SeedNode :: chash_node()) -> chash().
fresh(NumPartitions, SeedNode) ->
- Inc = ?RINGTOP div NumPartitions,
+ Inc = ring_increment(NumPartitions),
{NumPartitions, [{IndexAsInt, SeedNode} ||
IndexAsInt <- lists:seq(0,(?RINGTOP-1),Inc)]}.
-% @doc Find the Node that owns the partition identified by IndexAsInt.
-% @spec lookup(IndexAsInt :: integer(), CHash :: chash()) -> chash_node()
+%% @doc Find the Node that owns the partition identified by IndexAsInt.
+-spec lookup(IndexAsInt :: index_as_int(), CHash :: chash()) -> chash_node().
lookup(IndexAsInt, CHash) ->
{_NumPartitions, Nodes} = CHash,
{IndexAsInt, X} = proplists:lookup(IndexAsInt, Nodes),
X.
-% @doc Return true if named Node owns any partitions in the ring, else false.
-% @spec contains_name(Name :: chash_node(), CHash :: chash()) -> bool()
-contains_name(Name, CHash) ->
+%% @doc Given any term used to name an object, produce that object's key
+%% into the ring. Two names with the same SHA-1 hash value are
+%% considered the same name.
+-spec key_of(ObjectName :: term()) -> index().
+key_of(ObjectName) ->
+ crypto:sha(term_to_binary(ObjectName)).
+
+%% @doc Return all Nodes that own any partitions in the ring.
+-spec members(CHash :: chash()) -> [chash_node()].
+members(CHash) ->
{_NumPartitions, Nodes} = CHash,
- [X || {_,X} <- Nodes, X == Name] =/= [].
+ lists:usort([X || {_Idx,X} <- Nodes]).
-% @doc Make the partition beginning at IndexAsInt owned by Name'd node.
-% @spec update(IndexAsInt :: integer(), Name :: chash_node(), CHash :: chash())
-% -> chash()
-update(IndexAsInt, Name, CHash) ->
- {NumPartitions, Nodes} = CHash,
- NewNodes = lists:keyreplace(IndexAsInt, 1, Nodes, {IndexAsInt, Name}),
- {NumPartitions, NewNodes}.
+%% @doc Return a randomized merge of two rings.
+%% If multiple nodes are actively claiming nodes in the same
+%% time period, churn will occur. Be prepared to live with it.
+-spec merge_rings(CHashA :: chash(), CHashB :: chash()) -> chash().
+merge_rings(CHashA,CHashB) ->
+ {NumPartitions, NodesA} = CHashA,
+ {NumPartitions, NodesB} = CHashB,
+ {NumPartitions, [{I,random_node(A,B)} ||
+ {{I,A},{I,B}} <- lists:zip(NodesA,NodesB)]}.
+
+%% @doc Given the integer representation of a chash key,
+%% return the next ring index integer value.
+-spec next_index(IntegerKey :: integer(), CHash :: chash()) -> index_as_int().
+next_index(IntegerKey, {NumPartitions, _}) ->
+ Inc = ring_increment(NumPartitions),
+ (((IntegerKey div Inc) + 1) rem NumPartitions) * Inc.
+
+%% @doc Return the entire set of NodeEntries in the ring.
+-spec nodes(CHash :: chash()) -> [node_entry()].
+nodes(CHash) ->
+ {_NumPartitions, Nodes} = CHash,
+ Nodes.
+
+%% @doc Given an object key, return all NodeEntries in order starting at Index.
+-spec ordered_from(Index :: index(), CHash :: chash()) -> [node_entry()].
+ordered_from(Index, {NumPartitions, Nodes}) ->
+ <<IndexAsInt:160/integer>> = Index,
+ Inc = ring_increment(NumPartitions),
+ {A, B} = lists:split((IndexAsInt div Inc)+1, Nodes),
+ B ++ A.
+
+%% @doc Given an object key, return all NodeEntries in reverse order
+%% starting at Index.
+-spec predecessors(Index :: index(), CHash :: chash()) -> [node_entry()].
+predecessors(Index, CHash) ->
+ {NumPartitions, _Nodes} = CHash,
+ predecessors(Index, CHash, NumPartitions).
+%% @doc Given an object key, return the next N NodeEntries in reverse order
+%% starting at Index.
+-spec predecessors(Index :: index(), CHash :: chash(), N :: integer())
+ -> [node_entry()].
+predecessors(Index, CHash, N) ->
+ Num = max_n(N, CHash),
+ {Res, _} = lists:split(Num, lists:reverse(ordered_from(Index,CHash))),
+ Res.
+
+%% @doc Return increment between ring indexes given
+%% the number of ring partitions.
+-spec ring_increment(NumPartitions :: pos_integer()) -> pos_integer().
+ring_increment(NumPartitions) ->
+ ?RINGTOP div NumPartitions.
-% @doc Given an object key, return all NodeEntries in order starting at Index.
-% @spec successors(Index :: index(), CHash :: chash()) -> [NodeEntry]
+%% @doc Return the number of partitions in the ring.
+-spec size(CHash :: chash()) -> integer().
+size(CHash) ->
+ {_NumPartitions,Nodes} = CHash,
+ length(Nodes).
+
+%% @doc Given an object key, return all NodeEntries in order starting at Index.
+-spec successors(Index :: index(), CHash :: chash()) -> [node_entry()].
successors(Index, CHash) ->
{NumPartitions, _Nodes} = CHash,
successors(Index, CHash, NumPartitions).
-% @doc Given an object key, return the next N NodeEntries in order
-% starting at Index.
-% @spec successors(Index :: index(), CHash :: chash(), N :: integer())
-% -> [NodeEntry]
+
+%% @doc Given an object key, return the next N NodeEntries in order
+%% starting at Index.
+-spec successors(Index :: index(), CHash :: chash(), N :: integer())
+ -> [node_entry()].
successors(Index, CHash, N) ->
Num = max_n(N, CHash),
Ordered = ordered_from(Index, CHash),
@@ -110,73 +191,34 @@ successors(Index, CHash, N) ->
Res
end.
-% @doc Given an object key, return all NodeEntries in reverse order
-% starting at Index.
-% @spec predecessors(Index :: index(), CHash :: chash()) -> [NodeEntry]
-predecessors(Index, CHash) ->
- {NumPartitions, _Nodes} = CHash,
- predecessors(Index, CHash, NumPartitions).
-% @doc Given an object key, return the next N NodeEntries in reverse order
-% starting at Index.
-% @spec predecessors(Index :: index(), CHash :: chash(), N :: integer())
-% -> [NodeEntry]
-predecessors(Index, CHash, N) ->
- Num = max_n(N, CHash),
- {Res, _} = lists:split(Num, lists:reverse(ordered_from(Index,CHash))),
- Res.
+%% @doc Make the partition beginning at IndexAsInt owned by Name'd node.
+-spec update(IndexAsInt :: index_as_int(), Name :: chash_node(), CHash :: chash())
+ -> chash().
+update(IndexAsInt, Name, CHash) ->
+ {NumPartitions, Nodes} = CHash,
+ NewNodes = lists:keyreplace(IndexAsInt, 1, Nodes, {IndexAsInt, Name}),
+ {NumPartitions, NewNodes}.
-% @doc Return either N or the number of partitions in the ring, whichever
-% is lesser.
-% @spec max_n(N :: integer(), CHash :: chash()) -> integer()
+%% ====================================================================
+%% Internal functions
+%% ====================================================================
+
+%% @private
+%% @doc Return either N or the number of partitions in the ring, whichever
+%% is lesser.
+-spec max_n(N :: integer(), CHash :: chash()) -> integer().
max_n(N, {NumPartitions, _Nodes}) ->
erlang:min(N, NumPartitions).
-% @doc Given an object key, return all NodeEntries in order starting at Index.
-% @spec ordered_from(Index :: index(), CHash :: chash()) -> [NodeEntry]
-ordered_from(Index, {NumPartitions, Nodes}) ->
- <<IndexAsInt:160/integer>> = Index,
- Inc = ?RINGTOP div NumPartitions,
- {A, B} = lists:split((IndexAsInt div Inc)+1, Nodes),
- B ++ A.
-
-% @doc Given any term used to name an object, produce that object's key
-% into the ring. Two names with the same SHA-1 hash value are
-% considered the same name.
-% @spec key_of(ObjectName :: term()) -> index()
-key_of(ObjectName) ->
- crypto:sha(term_to_binary(ObjectName)).
+%% @private
+-spec random_node(NodeA :: chash_node(), NodeB :: chash_node()) -> chash_node().
+random_node(NodeA,NodeA) -> NodeA;
+random_node(NodeA,NodeB) -> lists:nth(random:uniform(2),[NodeA,NodeB]).
-% @doc Return all Nodes that own any partitions in the ring.
-% @spec members(CHash :: chash()) -> [Node]
-members(CHash) ->
- {_NumPartitions, Nodes} = CHash,
- lists:usort([X || {_Idx,X} <- Nodes]).
-
-% @doc Return the entire set of NodeEntries in the ring.
-% @spec nodes(CHash :: chash()) -> [NodeEntry]
-nodes(CHash) ->
- {_NumPartitions, Nodes} = CHash,
- Nodes.
-
-% @doc Return a randomized merge of two rings.
-% If multiple nodes are actively claiming nodes in the same
-% time period, churn will occur. Be prepared to live with it.
-% @spec merge_rings(CHashA :: chash(), CHashB :: chash()) -> chash()
-merge_rings(CHashA,CHashB) ->
- {NumPartitions, NodesA} = CHashA,
- {NumPartitions, NodesB} = CHashB,
- {NumPartitions, [{I,randomnode(A,B)} ||
- {{I,A},{I,B}} <- lists:zip(NodesA,NodesB)]}.
-
-% @spec randomnode(NodeA :: chash_node(), NodeB :: chash_node()) -> chash_node()
-randomnode(NodeA,NodeA) -> NodeA;
-randomnode(NodeA,NodeB) -> lists:nth(random:uniform(2),[NodeA,NodeB]).
-
-% @doc Return the number of partitions in the ring.
-% @spec size(CHash :: chash()) -> integer()
-size(CHash) ->
- {_NumPartitions,Nodes} = CHash,
- length(Nodes).
+%% ===================================================================
+%% EUnit tests
+%% ===================================================================
+-ifdef(TEST).
update_test() ->
Node = 'old@host', NewNode = 'new@host',
@@ -218,3 +260,5 @@ merge_test() ->
CHashB = chash:update(0,node_one,chash:fresh(8,node_two)),
CHash = chash:merge_rings(CHashA,CHashB),
?assertEqual(node_one,chash:lookup(0,CHash)).
+
+-endif.
Oops, something went wrong.

0 comments on commit 274db4a

Please sign in to comment.