This is an automated email from the ASF dual-hosted git repository. nickva pushed a commit to branch fabric_ring_fix in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 372a7c086a6fe35e51a47e3580dcd38a44a3cef7 Author: Nick Vatamaniuc <[email protected]> AuthorDate: Fri Jun 19 00:51:18 2026 -0400 Fix maintenance mode function_clause for _time_seq Previously, when fetching the `_time_seq` histogram with one node in mm we could get a function_clause error as show here: [0] `_time_seq` and a few other operations use `all` as fabric ring option. When using that option the accumulator used by fabric_ring uses a simplified 2-tuple shape `{Shard, Result}` as opposed to a 3-tuple `{Range, Shard, Result}` used in most other cases [1]. Because of that when checking if progress is possible we didn't correctly handle the case of 2-tuples and always expected a 3-tuple format. The fix is to expect a 2-tuple and 3-tuple and handle both. [0] ``` function_clause#012 [ <<"fabric_ring:-range_bounds/2-fun-0-/1 L274">>, <<"lists:map/2 L2077">>, <<"fabric_ring:range_bounds/2 L274">>, <<"fabric_ring:handle_error/4 L80">>, <<"fabric_time_seq:histogram_handle_message/3 L101">> ... ] ``` [1] The reason for both is a 2-tuple and 3-tuple is that in the default case we care to record how many responses arrived per range interval and also `mem3_util:get_ring/3` works with ranges only. For `all` or `{any, ...}` options we don't really need to track per range results, so we keep the accumulator simpler a bit. --- src/fabric/src/fabric_ring.erl | 54 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/fabric/src/fabric_ring.erl b/src/fabric/src/fabric_ring.erl index c8ba5e466..d5423b59e 100644 --- a/src/fabric/src/fabric_ring.erl +++ b/src/fabric/src/fabric_ring.erl @@ -271,11 +271,20 @@ get_shard_replacements_int(UnusedShards, UsedShards) -> ). range_bounds(Workers, Responses) -> - RespRanges = lists:map(fun({R, _, _}) -> R end, Responses), + RespRanges = lists:map(fun response_range/1, Responses), Ranges = fabric_util:worker_ranges(Workers) ++ RespRanges, {Bs, Es} = lists:unzip(Ranges), {lists:min(Bs), lists:max(Es)}. +% Responses can be in two shapes depending on ring options +% * {{B, E}, Shard, Response} : default and {rmin, R} rings +% * {Shard, Response} for all and any ring +% +response_range({{B, E}, #shard{}, _Response}) -> + {B, E}; +response_range({#shard{range = [B, E]}, _Response}) -> + {B, E}. + get_responses([], _) -> []; get_responses([Range | Ranges], [{Range, Shard, Value} | Resps]) -> @@ -561,6 +570,49 @@ handle_response_ring_opts_all_test() -> Result3 = handle_response(W3, 44, Workers3, [], [all], undefined), ?assertMatch({stop, [_ | _]}, Result3). +handle_error_ring_opts_all_test() -> + Shard1 = mk_shard("n1", [0, ?RING_END]), + Shard2 = mk_shard("n2", [0, ?RING_END]), + Shard3 = mk_shard("n3", [0, ?RING_END]), + + ShardList = [Shard1, Shard2, Shard3], + [W1, W2, W3] = WithRefs = [S#shard{ref = make_ref()} || S <- ShardList], + Workers1 = fabric_dict:init(WithRefs, nil), + + % One response is valid + {ok, {Workers2, Responses1}} = handle_response(W1, 42, Workers1, [], [all], undefined), + ?assertEqual([{W1, 42}], Responses1), + % Then an error happens + Result1 = handle_error(W2, Workers2, Responses1, [all]), + ?assertMatch({ok, _}, Result1), + {ok, Workers3} = Result1, + ?assertEqual(fabric_dict:erase(W2, Workers2), Workers3), + + % Once the last worker errors out there is no progress possible + ?assertEqual(error, handle_error(W3, Workers3, Responses1, [all])). + +node_down_ring_opts_all_test() -> + Shard1 = mk_shard("n1", [0, ?RING_END]), + Shard2 = mk_shard("n2", [0, ?RING_END]), + Shard3 = mk_shard("n3", [0, ?RING_END]), + + ShardList = [Shard1, Shard2, Shard3], + [W1, _W2, W3] = WithRefs = [S#shard{ref = make_ref()} || S <- ShardList], + Workers1 = fabric_dict:init(WithRefs, nil), + + % n1 response, with all option we have a single {Shard, Response} tuple + {ok, {Workers2, Responses1}} = handle_response(W1, 42, Workers1, [], [all], undefined), + ?assertEqual([{W1, 42}], Responses1), + + % n2 goes down but n3 is still around to make progress + Result1 = node_down(n2, Workers2, Responses1, [all]), + ?assertMatch({ok, _}, Result1), + {ok, Workers3} = Result1, + ?assertEqual([{W3, nil}], Workers3), + + % With n3 down too there is nothing left + ?assertEqual(error, node_down(n3, Workers3, Responses1, [all])). + handle_error_test() -> Shard1 = mk_shard("n1", [0, 5]), Shard2 = mk_shard("n1", [10, ?RING_END]),
