Permalink
Browse files

Update new partition claim algorithm after review + bug fixes

Change claim_simulation.erl eunit test to run a simulation with both the
new and old claim algorithm as suggested.

Rename riak_core_new_claim:new_claim/2 to new_choose_claim/2 to match
default_choose_claim/2.

Fix two bugs in riak_core_new_claim.erl that are on code paths that cannot
occur in 1.0 due to existing invariants, but should be fixed nevertheless:
- Match error in prefilter_violations: change CNth to {CNth, _}.
- Handle case where new_choose_claim fails to claim partitions by falling
  back to claim_rebalance_n.
  • Loading branch information...
1 parent 86d506f commit c0e29adcfce9d58d8164e8e52f2fdde9f9217ab5 @jtuple jtuple committed Sep 21, 2011
Showing with 37 additions and 19 deletions.
  1. +16 −11 src/riak_core_new_claim.erl
  2. +21 −8 test/claim_simulation.erl
@@ -21,7 +21,7 @@
%% -------------------------------------------------------------------
-module(riak_core_new_claim).
--export([new_wants_claim/2, new_claim/2]).
+-export([new_wants_claim/2, new_choose_claim/2]).
new_wants_claim(Ring, Node) ->
Active = riak_core_ring:claiming_members(Ring),
@@ -38,7 +38,7 @@ new_wants_claim(Ring, Node) ->
{yes, Avg - Count}
end.
-new_claim(Ring, Node) ->
+new_choose_claim(Ring, Node) ->
Active = riak_core_ring:claiming_members(Ring),
Owners = riak_core_ring:all_owners(Ring),
Counts = get_counts(Active, Owners),
@@ -95,14 +95,17 @@ new_claim(Ring, Node) ->
riak_core_ring:transfer_node(Idx, Node, Ring0)
end, Ring, Claim2),
- case {EnoughNodes, riak_core_claim:meets_target_n(NewRing, TargetN)} of
- {false, _} ->
- NewRing;
- {true, {true, _}} ->
- NewRing;
- {true, false} ->
- %% Last resort, fallback to re-diagonalization
- riak_core_claim:claim_rebalance_n(Ring, Node)
+ RingChanged = ([] /= Claim2),
+ RingMeetsTargetN = riak_core_claim:meets_target_n(NewRing, TargetN),
+ case {RingChanged, EnoughNodes, RingMeetsTargetN} of
+ {false, _, _} ->
+ %% Unable to claim, fallback to re-diagonalization
+ riak_core_claim:claim_rebalance_n(Ring, Node);
+ {_, true, false} ->
+ %% Failed to meet target_n, fallback to re-diagonalization
+ riak_core_claim:claim_rebalance_n(Ring, Node);
+ _ ->
+ NewRing
end.
%% Counts up the number of partitions owned by each node
@@ -124,7 +127,7 @@ prefilter_violations(Ring, Node, AllIndices, Indices, TargetN, RingSize) ->
CurrentIndices = riak_core_ring:indices(Ring, Node),
CurrentNth = [lists:keyfind(Idx, 2, AllIndices) || Idx <- CurrentIndices],
[{Nth, Idx} || {Nth, Idx} <- Indices,
- lists:all(fun(CNth) ->
+ lists:all(fun({CNth, _}) ->
spaced_by_n(CNth, Nth, TargetN, RingSize)
end, CurrentNth)].
@@ -136,6 +139,8 @@ prefilter_violations(Ring, Node, AllIndices, Indices, TargetN, RingSize) ->
%% expected ownership. In other words, if A owns 5 partitions and
%% the desired ownership is 3, then we try to claim at most 2 partitions
%% from A.
+select_indices(_Owners, _Deltas, [], _TargetN, _RingSize) ->
+ [];
select_indices(Owners, Deltas, Indices, TargetN, RingSize) ->
OwnerDT = dict:from_list(Owners),
{FirstNth, _} = hd(Indices),
@@ -24,7 +24,7 @@
-module(claim_simulation).
-compile(export_all).
--define(SIMULATE,1).
+%%-define(SIMULATE,1).
-ifdef(SIMULATE).
-ifdef(TEST).
-include_lib("eunit/include/eunit.hrl").
@@ -33,14 +33,24 @@
-define(get(K, PL, D), proplists:get_value(K, PL, D)).
basic_test_() ->
- {timeout, 60000, fun basic/0}.
+ {timeout, 60000, [fun basic_default/0,
+ fun basic_new/0]}.
-basic() ->
- Opts = [
- %%{wc_mf, {riak_core_claim, default_wants_claim}},
- %%{cc_mf, {riak_core_claim, default_choose_claim}},
+basic_default() ->
+ Opts = [{suffix, "_default"},
+ {wc_mf, {riak_core_claim, default_wants_claim}},
+ {cc_mf, {riak_core_claim, default_choose_claim}},
+ {target_n_val, 4},
+ {ring_size, 32},
+ {node_count, 8},
+ {node_capacity, 24}
+ ],
+ run(Opts).
+
+basic_new() ->
+ Opts = [{suffix, "_new"},
{wc_mf, {riak_core_new_claim, new_wants_claim}},
- {cc_mf, {riak_core_new_claim, new_claim}},
+ {cc_mf, {riak_core_new_claim, new_choose_claim}},
{target_n_val, 4},
{ring_size, 32},
{node_count, 8},
@@ -54,6 +64,8 @@ run(Opts) ->
WCMod = ?get(wc_mf, Opts, {riak_core_claim, default_wants_claim}),
CCMod = ?get(cc_mf, Opts, {riak_core_claim, default_choose_claim}),
TargetN = ?get(target_n_val, Opts, 4),
+ Suffix = ?get(suffix, Opts, ""),
+
application:set_env(riak_core, wants_claim_fun, WCMod),
application:set_env(riak_core, choose_claim_fun, CCMod),
application:set_env(riak_core, target_n_val, TargetN),
@@ -86,7 +98,8 @@ run(Opts) ->
MeetTargetN = [riak_core_claim:meets_target_n(R, TargetN) || R <- Rings],
- FName = io_lib:format("/tmp/rings_~w_~w.txt", [RingSize, NodeCount]),
+ FName = io_lib:format("/tmp/rings_~w_~w~s.txt",
+ [RingSize, NodeCount, Suffix]),
{ok, Out} = file:open(FName, [write]),
[print_info(Out, O, N, M, lists:nth(N - 1, MeetTargetN),
RingSize, NodeCapacity)

0 comments on commit c0e29ad

Please sign in to comment.