From 28aae507627f1e591a2ee0e4af9a8997b48961d1 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Sat, 25 May 2024 21:01:25 +0300 Subject: [PATCH 1/9] Remove leftover from amoc-rest in config doc --- guides/configuration.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/guides/configuration.md b/guides/configuration.md index 9e6a5608..2cbb0058 100644 --- a/guides/configuration.md +++ b/guides/configuration.md @@ -9,9 +9,6 @@ Amoc supports the following generic configuration parameters: * default value - empty list (`[]`) * example: `AMOC_NODES="['amoc@amoc-1', 'amoc@amoc-2']"` -* `api_port` - a port for the amoc REST interfaces: - * default value - 4000 - * example: `AMOC_API_PORT="4000"` * `interarrival` - a delay (in ms, for each node in the cluster independently) between creating the processes for two consecutive users: From 152af102a57873108680756d7c74b45f953c480c Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 09:49:35 +0200 Subject: [PATCH 2/9] Upgrade erlang version --- .github/workflows/ci.yml | 6 +++--- src/throttle/amoc_throttle_process.erl | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83a4beef..9ce18d07 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,10 +11,10 @@ jobs: name: ${{ matrix.test-type }} test on OTP ${{matrix.otp_vsn}} strategy: matrix: - otp_vsn: ['26.2', '25.3', '24.3'] - rebar_vsn: ['3.22.0'] + otp_vsn: ['27', '26', '25'] + rebar_vsn: ['3.23.0'] test-type: ['regular', 'integration'] - runs-on: 'ubuntu-22.04' + runs-on: 'ubuntu-24.04' steps: - uses: actions/checkout@v4 - uses: erlef/setup-beam@v1 diff --git a/src/throttle/amoc_throttle_process.erl b/src/throttle/amoc_throttle_process.erl index 9a56b37e..4b67d69a 100644 --- a/src/throttle/amoc_throttle_process.erl +++ b/src/throttle/amoc_throttle_process.erl @@ -24,7 +24,7 @@ handle_info/2, handle_cast/2, handle_continue/2, - format_status/2]). + format_status/1]). -define(PG_SCOPE, amoc_throttle). -define(DEFAULT_MSG_TIMEOUT, 60000).%% one minute @@ -143,12 +143,13 @@ handle_continue(maybe_run_fn, State) -> NewState = maybe_run_fn(State), {noreply, NewState, timeout(NewState)}. --spec format_status(term(), term()) -> term(). -format_status(_Opt, [_PDict, State]) -> +-spec format_status(gen_server:format_status()) -> gen_server:format_status(). +format_status(#{state := #state{} = State} = FormatStatus) -> ScheduleLen = length(State#state.schedule), ScheduleRevLen = length(State#state.schedule_reversed), State1 = setelement(#state.schedule, State, ScheduleLen), - setelement(#state.schedule_reversed, State1, ScheduleRevLen). + State2 = setelement(#state.schedule_reversed, State1, ScheduleRevLen), + FormatStatus#{state := State2}. %%------------------------------------------------------------------------------ %% internal functions From 79762be9ea432fdb28450a7046f8e74c42f58798 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 10:00:22 +0200 Subject: [PATCH 3/9] Fix types for amoc_users_worker_sup --- src/amoc_scenario.erl | 2 +- src/users/amoc_user.erl | 5 +- src/users/amoc_users_worker_sup.erl | 75 +++++++++++++++++------------ 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/src/amoc_scenario.erl b/src/amoc_scenario.erl index 1209d1c4..d43d3c8e 100644 --- a/src/amoc_scenario.erl +++ b/src/amoc_scenario.erl @@ -73,7 +73,7 @@ terminate(Scenario, State) -> %% if scenario module exports both functions, `Scenario:start/2' is used. %% %% Runs on the user process and spans a `[amoc, scenario, user, _]' telemetry event. --spec start(amoc:scenario(), user_id(), state()) -> any(). +-spec start(amoc:scenario(), user_id(), state()) -> term(). start(Scenario, Id, State) -> Metadata = #{scenario => Scenario, state => State, user_id => Id}, Span = case {erlang:function_exported(Scenario, start, 2), diff --git a/src/users/amoc_user.erl b/src/users/amoc_user.erl index 8690b533..663b750a 100644 --- a/src/users/amoc_user.erl +++ b/src/users/amoc_user.erl @@ -14,7 +14,7 @@ start_link(Scenario, Id, State) -> proc_lib:start_link(?MODULE, init, [self(), Scenario, Id, State]). --spec stop() -> no_return(). +-spec stop() -> ok. stop() -> stop(self(), false). @@ -22,8 +22,7 @@ stop() -> stop(Pid, Force) when is_pid(Pid) -> amoc_users_sup:stop_child(Pid, Force). --spec init(pid(), amoc:scenario(), amoc_scenario:user_id(), state()) -> - no_return(). +-spec init(pid(), amoc:scenario(), amoc_scenario:user_id(), state()) -> term(). init(Parent, Scenario, Id, State) -> proc_lib:init_ack(Parent, {ok, self()}), process_flag(trap_exit, true), diff --git a/src/users/amoc_users_worker_sup.erl b/src/users/amoc_users_worker_sup.erl index fd1c6dcb..03e050a4 100644 --- a/src/users/amoc_users_worker_sup.erl +++ b/src/users/amoc_users_worker_sup.erl @@ -23,15 +23,15 @@ -record(state, { index :: non_neg_integer(), - tid :: ets:tid(), - tasks = #{} :: #{reference() => pid()} + table :: ets:table(), + tasks = #{} :: #{reference() := pid()} }). -type state() :: #state{}. -define(SHUTDOWN_TIMEOUT, 2000). %% 2 seconds %% @private --spec start_link(non_neg_integer()) -> {ok, pid()}. +-spec start_link(non_neg_integer()) -> gen_server:start_ret(). start_link(N) -> gen_server:start_link(?MODULE, N, []). @@ -55,8 +55,8 @@ terminate_all_children(Sup) -> stop_child(Pid, false) -> exit(Pid, shutdown), ok; -stop_child(Pids, true) -> - spawn(shutdown_and_kill_after_timeout_fun(Pids)), +stop_child(Pid, true) -> + spawn(shutdown_and_kill_after_timeout_fun(Pid)), ok. -spec get_all_children(pid()) -> [{pid(), amoc_scenario:user_id()}]. @@ -64,21 +64,25 @@ get_all_children(Sup) -> gen_server:call(Sup, get_all_children, infinity). %% @private --spec init(non_neg_integer()) -> {ok, term()}. +-spec init(non_neg_integer()) -> {ok, state()}. init(N) -> process_flag(trap_exit, true), Name = list_to_atom(atom_to_list(?MODULE) ++ "_" ++ integer_to_list(N)), - Tid = ets:new(Name, [ordered_set, protected, named_table]), - {ok, #state{index = N, tid = Tid}}. + Table = ets:new(Name, [ordered_set, protected, named_table]), + {ok, #state{index = N, table = Table}}. %% @private -spec handle_call(any(), any(), state()) -> {reply, term(), state()}. -handle_call(get_all_children, _From, #state{tid = Tid} = State) -> - Children = ets:tab2list(Tid), +handle_call(get_all_children, _From, #state{table = Table} = State) -> + Children = ets:tab2list(Table), {reply, Children, State}. %% @private --spec handle_cast(any(), state()) -> {noreply, state()}. +-spec handle_cast(Request, state()) -> {noreply, state()} when + Request :: {start_child, amoc:scenario(), amoc_scenario:user_id(), amoc_scenario:state()} | + {start_children, amoc:scenario(), [amoc_scenario:user_id()], amoc_scenario:state()} | + {stop_children, non_neg_integer(), boolean()} | + terminate_all_children. handle_cast({start_child, Scenario, Id, ScenarioState}, State) -> do_start_child(Scenario, Id, ScenarioState, State), {noreply, State}; @@ -87,8 +91,8 @@ handle_cast({start_children, Scenario, Ids, ScenarioState}, State) -> {noreply, State}; handle_cast({stop_children, 0, _}, State) -> {noreply, State}; -handle_cast({stop_children, Int, ForceRemove}, #state{tid = Tid} = State) -> - Pids = case ets:match_object(Tid, '$1', Int) of +handle_cast({stop_children, Int, ForceRemove}, #state{table = Table} = State) -> + Pids = case ets:match_object(Table, '$1', Int) of '$end_of_table' -> []; {Objects, _} -> @@ -103,39 +107,42 @@ handle_cast(_Msg, State) -> {noreply, State}. %% @private --spec handle_info(any(), state()) -> {noreply, state()}. +-spec handle_info(Request, state()) -> {noreply, state()} when + Request :: {child_up, pid(), amoc_scenario:user_id()} | + {'DOWN', reference(), process, pid(), term()} | + {'EXIT', pid(), term()}. handle_info({'DOWN', Ref, process, _Pid, _Reason}, #state{tasks = Tasks} = State) -> {noreply, State#state{tasks = maps:remove(Ref, Tasks)}}; -handle_info({'EXIT', Pid, _Reason}, #state{index = N, tid = Tid} = State) -> - handle_down_user(Tid, Pid, N), +handle_info({'EXIT', Pid, _Reason}, #state{index = N, table = Table} = State) -> + handle_down_user(Table, Pid, N), {noreply, State}; handle_info(_Info, State) -> {noreply, State}. %% @private --spec terminate(term(), state()) -> any(). +-spec terminate(term(), state()) -> state(). terminate(_Reason, State) -> do_terminate_all_my_children(State). %% Helpers -spec do_start_child(module(), amoc_scenario:user_id(), term(), state()) -> any(). -do_start_child(Scenario, Id, ScenarioState, #state{index = N, tid = Tid}) -> +do_start_child(Scenario, Id, ScenarioState, #state{index = N, table = Table}) -> case amoc_user:start_link(Scenario, Id, ScenarioState) of {ok, Pid} -> - handle_up_user(Tid, Pid, Id, N); + handle_up_user(Table, Pid, Id, N); _ -> ok end. --spec handle_up_user(ets:tid(), pid(), amoc_scenario:user_id(), non_neg_integer()) -> ok. -handle_up_user(Tid, Pid, Id, SupNum) -> - ets:insert(Tid, {Pid, Id}), +-spec handle_up_user(ets:table(), pid(), amoc_scenario:user_id(), non_neg_integer()) -> any(). +handle_up_user(Table, Pid, Id, SupNum) -> + ets:insert(Table, {Pid, Id}), amoc_users_sup:incr_no_of_users(SupNum). --spec handle_down_user(ets:tid(), pid(), non_neg_integer()) -> ok. -handle_down_user(Tid, Pid, SupNum) -> - ets:delete(Tid, Pid), +-spec handle_down_user(ets:table(), pid(), non_neg_integer()) -> ok. +handle_down_user(Table, Pid, SupNum) -> + ets:delete(Table, Pid), amoc_users_sup:decr_no_of_users(SupNum). %% @doc Stop a list of users in parallel. @@ -151,21 +158,27 @@ maybe_track_task_to_stop_my_children(#state{tasks = Tasks} = State, Pids, true) {Pid, Ref} = spawn_monitor(shutdown_and_kill_after_timeout_fun(Pids)), State#state{tasks = Tasks#{Pid => Ref}}. --spec shutdown_and_kill_after_timeout_fun([pid()]) -> fun(() -> term()). -shutdown_and_kill_after_timeout_fun(Pids) -> +-spec shutdown_and_kill_after_timeout_fun(pid() | [pid()]) -> fun(() -> term()). +shutdown_and_kill_after_timeout_fun([_ | _] = Pids) -> fun() -> [ exit(Pid, shutdown) || Pid <- Pids ], timer:sleep(?SHUTDOWN_TIMEOUT), [ exit(Pid, kill) || Pid <- Pids ] + end; +shutdown_and_kill_after_timeout_fun(Pid) when is_pid(Pid) -> + fun() -> + exit(Pid, shutdown), + timer:sleep(?SHUTDOWN_TIMEOUT), + exit(Pid, kill) end. --spec do_terminate_all_my_children(state()) -> any(). -do_terminate_all_my_children(#state{tid = Tid} = State) -> - Match = ets:match_object(Tid, '$1', 200), +-spec do_terminate_all_my_children(state()) -> state(). +do_terminate_all_my_children(#state{table = Table} = State) -> + Match = ets:match_object(Table, '$1', 200), do_terminate_all_my_children(State, Match). %% ets:continuation/0 type is unfortunately not exported from the ets module. --spec do_terminate_all_my_children(state(), {tuple(), term()} | '$end_of_table') -> state(). +-spec do_terminate_all_my_children(state(), {[tuple()], term()} | '$end_of_table') -> state(). do_terminate_all_my_children(State, {Objects, Continuation}) -> Pids = [Pid || {Pid, _Id} <- Objects], NewState = maybe_track_task_to_stop_my_children(State, Pids, true), From a807f6feab88b0f8c928996a514be503aa15ffde Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 10:01:30 +0200 Subject: [PATCH 4/9] Rework integratin test helper for better readability and error reporting --- .../extra_code_paths/path1/dummy_helper.erl | 109 ++++++++++-------- 1 file changed, 61 insertions(+), 48 deletions(-) diff --git a/integration_test/extra_code_paths/path1/dummy_helper.erl b/integration_test/extra_code_paths/path1/dummy_helper.erl index 6a3e82ac..c0c34a47 100644 --- a/integration_test/extra_code_paths/path1/dummy_helper.erl +++ b/integration_test/extra_code_paths/path1/dummy_helper.erl @@ -1,7 +1,12 @@ -module(dummy_helper). --required_variable(#{name=>dummy_var, description=>"dummy_var", - default_value=>default_value}). +-include_lib("stdlib/include/assert.hrl"). + +-required_variable(#{name => dummy_var, + description => "dummy_var", + default_value => default_value}). + +-define(comment(U), io_lib:format("Condition failed with last users distribution ~n~p", [U])). %% amoc_dist testing function -export([test_amoc_dist/0]). @@ -11,65 +16,73 @@ test_amoc_dist() -> Master = amoc_cluster:master_node(), Slaves = amoc_cluster:slave_nodes(), %% check the status of the nodes - disabled = rpc:call(Master, amoc_controller, get_status, []), - [{running, #{scenario := dummy_scenario}} = rpc:call(Node, amoc_controller, get_status, []) - || Node <- Slaves], - %% check user ids - {N1, Nodes1, Ids1, Max1} = get_users_info(Slaves), - true = N1 > 0, - N1 = Max1, - Ids1 = lists:seq(1, N1), + ?assertEqual(disabled, get_status(Master)), + [ ?assertMatch({running, #{scenario := dummy_scenario}}, get_status(Node)) || Node <- Slaves], + %% check user ids, users have all been started at the first two nodes + {N1, Max1, Nodes1, Ids1, Users1} = get_users_info(Slaves), + ?assert(N1 > 0), + ?assertEqual(N1, Max1, ?comment(Users1)), + ?assertEqual(Ids1, lists:seq(1, N1), ?comment(Users1)), [AddedNode] = Slaves -- Nodes1, %% add 20 users - {ok, _} = rpc:call(Master, amoc_dist, add, [20]), - timer:sleep(3000), - {N2, Nodes2, Ids2, Max2} = get_users_info(Slaves), - N2 = Max2, - Ids2 = lists:seq(1, N2), - [AddedNode] = Nodes2 -- Nodes1, - N2 = N1 + 20, + add_and_wait(Master, 20), + {N2, Max2, Nodes2, Ids2, Users2} = get_users_info(Slaves), + ?assertEqual(N2, Max2, ?comment(Users2)), + ?assertEqual(Ids2, lists:seq(1, N2), ?comment(Users2)), + ?assertEqual([AddedNode], Nodes2 -- Nodes1, ?comment(Users2)), + ?assertEqual(N2, N1 + 20, ?comment(Users2)), %% remove 10 users - {ok, _} = rpc:call(Master, amoc_dist, remove, [10, true]), - timer:sleep(3000), - {N3, Nodes3, _Ids3, Max3} = get_users_info(Slaves), - Nodes2 = Nodes3, - Max3 = Max2, - N2 = N3 + 10, + remove_and_wait(Master, 10), + {N3, Max3, Nodes3, _Ids3, Users3} = get_users_info(Slaves), + ?assertEqual(N2 - 10, N3, ?comment(Users3)), + ?assertEqual(Max2, Max3, ?comment(Users3)), + ?assertEqual(Nodes2, Nodes3, ?comment(Users3)), %% try to remove N3 users - {ok, Ret} = rpc:call(Master, amoc_dist, remove, [N3, true]), + Ret = remove_and_wait(Master, N3), RemovedN = lists:sum([N || {_, N} <- Ret]), - timer:sleep(3000), - {N4, Nodes4, Ids4, _Max4} = get_users_info(Slaves), - Nodes1 = Nodes4, - N3 = N4 + RemovedN, - true = RemovedN < N3, + {N4, _Max4, Nodes4, Ids4, Users4} = get_users_info(Slaves), + ?assertEqual(N3 - RemovedN, N4, ?comment(Users4)), + ?assertEqual(Nodes1, Nodes4, ?comment(Users4)), + ?assert(RemovedN < N3), %% add 20 users - {ok, _} = rpc:call(Master, amoc_dist, add, [20]), - timer:sleep(3000), - {N5, Nodes5, Ids5, Max5} = get_users_info(Slaves), - Nodes2 = Nodes5, - Max5 = Max2 + 20, - N5 = N4 + 20, - true = Ids5 -- Ids4 =:= lists:seq(Max2 + 1, Max5), + add_and_wait(Master, 20), + {N5, Max5, Nodes5, Ids5, Users5} = get_users_info(Slaves), + ?assertEqual(Nodes2, Nodes5, ?comment(Users5)), + ?assertEqual(Max5, Max2 + 20, ?comment(Users5)), + ?assertEqual(N5, N4 + 20, ?comment(Users5)), + ?assertEqual(Ids5 -- Ids4, lists:seq(Max2 + 1, Max5), ?comment(Users5)), %% terminate scenario - {ok,_} = rpc:call(Master, amoc_dist, stop, []), - timer:sleep(3000), - [{finished, dummy_scenario} = rpc:call(Node, amoc_controller, get_status, []) - || Node <- Slaves], + stop(Master), + [ ?assertEqual({finished, dummy_scenario}, get_status(Node)) || Node <- Slaves], %% return expected value amoc_dist_works_as_expected catch C:E:S -> - {error, {C, E, S}} + {C, E, S} end. get_users_info(SlaveNodes) -> - Users = [{Node, Id} || - Node <- SlaveNodes, - {_Pid, Id} <- rpc:call(Node, amoc_users_sup, get_all_children, [])], - Ids = lists:usort([Id || {_, Id} <- Users]), - Nodes = lists:usort([Node || {Node, _} <- Users]), + Distrib = [ {Node, erpc:call(Node, amoc_users_sup, get_all_children, [])} || Node <- SlaveNodes ], + Ids = lists:usort([Id || {_Node, Users} <- Distrib, {_, Id} <- Users]), + Nodes = lists:usort([Node || {Node, Users} <- Distrib, [] =/= Users]), N = length(Ids), - N = length(Users), MaxId = lists:max(Ids), - {N, Nodes, Ids, MaxId}. + {N, MaxId, Nodes, Ids, Distrib}. + +add_and_wait(Master, Num) -> + {ok, Ret} = erpc:call(Master, amoc_dist, add, [Num]), + timer:sleep(3000), + Ret. + +remove_and_wait(Master, Num) -> + {ok, Ret} = erpc:call(Master, amoc_dist, remove, [Num, true]), + timer:sleep(3000), + Ret. + +stop(Master) -> + {ok, Ret} = erpc:call(Master, amoc_dist, stop, []), + timer:sleep(3000), + Ret. + +get_status(Node) -> + erpc:call(Node, amoc_controller, get_status, []). From 6110d1ade93d0a99b4bf4d771c44fd898e08ad93 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 10:30:54 +0200 Subject: [PATCH 5/9] Prefer erpc for safety --- src/dist/amoc_dist.erl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dist/amoc_dist.erl b/src/dist/amoc_dist.erl index 4016bd98..73b0c12e 100644 --- a/src/dist/amoc_dist.erl +++ b/src/dist/amoc_dist.erl @@ -94,7 +94,7 @@ get_state() -> case {amoc_cluster:master_node(), get_param(state)} of {undefined, undefined} -> idle; {_, {ok, State}} -> State; - {Node, undefined} -> rpc:call(Node, ?MODULE, ?FUNCTION_NAME, []) + {Node, undefined} -> erpc:call(Node, ?MODULE, ?FUNCTION_NAME, []) end. %% ------------------------------------------------------------------ @@ -172,7 +172,7 @@ setup_slave_node(Node) -> {ok, _} -> {ok, Scenario} = get_param(scenario), {ok, Settings} = get_param(settings), - rpc:call(Node, amoc_controller, start_scenario, [Scenario, Settings]); + erpc:call(Node, amoc_controller, start_scenario, [Scenario, Settings]); Error -> Error end. @@ -196,7 +196,7 @@ add_users(Result, LastId, Count, [Node | T] = Nodes) -> 0 -> add_users([{Node, {ok, node_skipped}} | Result], LastId, Count, T); N -> - Ret = rpc:call(Node, amoc_controller, add_users, [LastId + 1, LastId + N]), + Ret = erpc:call(Node, amoc_controller, add_users, [LastId + 1, LastId + N]), add_users([{Node, Ret} | Result], LastId + N, Count - N, T) end. @@ -213,7 +213,7 @@ remove_users(Result, Count, ForceRemove, [Node | T] = Nodes) -> 0 -> remove_users([{Node, {ok, node_skipped}} | Result], Count, ForceRemove, T); N -> - Ret = rpc:call(Node, amoc_controller, remove_users, [N, ForceRemove]), + Ret = erpc:call(Node, amoc_controller, remove_users, [N, ForceRemove]), remove_users([{Node, Ret} | Result], Count - N, ForceRemove, T) end. @@ -225,7 +225,7 @@ update_settings_on_nodes(Settings, Nodes) -> -spec update_settings_on_node(amoc_config:settings(), node()) -> ok | {badrpc, any()} | {error, any()}. update_settings_on_node(Settings, Node) -> - rpc:call(Node, amoc_controller, update_settings, [Settings]). + erpc:call(Node, amoc_controller, update_settings, [Settings]). -spec stop_cluster() -> {ok, any()} | {error, any()}. stop_cluster() -> @@ -234,7 +234,7 @@ stop_cluster() -> {_, []} -> {error, no_slave_nodes}; {MasterNode, Slaves} -> set_state(stopped), - Result = [{Node, rpc:call(Node, amoc_controller, stop_scenario, [])} || + Result = [{Node, erpc:call(Node, amoc_controller, stop_scenario, [])} || Node <- Slaves], maybe_error(Result); {_, _} -> {error, not_a_master} From 6d51ff05d2f33f82ee12f041e2b4669626dbc217 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 11:30:12 +0200 Subject: [PATCH 6/9] Run GHA on all PRs --- .github/workflows/ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ce18d07..f17d01c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,8 @@ on: push: branches: [ master ] pull_request: - branches: [ master ] + workflow_dispatch: + jobs: test: From cbe4265a4764b2cbafb3be1731e2ce1b00cc0f0b Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 11:59:40 +0200 Subject: [PATCH 7/9] Prefer hash over rem when distributing users to sups If the test has some deviation on the behaviour of clients according to their IDs, for example if every N clients the Nth will do extra work, then the distribution of clients across supervisors might not be uniform. For this, prefer hashing them. --- src/users/amoc_users_sup.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/users/amoc_users_sup.erl b/src/users/amoc_users_sup.erl index 16be9c5b..c513d8d7 100644 --- a/src/users/amoc_users_sup.erl +++ b/src/users/amoc_users_sup.erl @@ -139,14 +139,14 @@ terminate_all_children() -> -spec get_sup_for_user_id(amoc_scenario:user_id()) -> pid(). get_sup_for_user_id(Id) -> #storage{sups = Supervisors, sups_count = SupCount} = persistent_term:get(?MODULE), - Index = Id rem SupCount + 1, + Index = erlang:phash2(Id, SupCount) + 1, element(Index, Supervisors). %% assign which users each worker will be requested to add -spec assign_users_to_sups(pos_integer(), tuple(), [amoc_scenario:user_id()], Acc) -> Acc when Acc :: #{pid() := [amoc_scenario:user_id()]}. assign_users_to_sups(SupCount, Supervisors, [Id | Ids], Acc) -> - Index = Id rem SupCount + 1, + Index = erlang:phash2(Id, SupCount) + 1, ChosenSup = element(Index, Supervisors), Vs = maps:get(ChosenSup, Acc), NewAcc = Acc#{ChosenSup := [Id | Vs]}, From f4b9f812b4946875a263407e4857888019a9c62c Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 22:37:04 +0200 Subject: [PATCH 8/9] Types and cosmetics for amoc_sup and amoc_controller --- src/amoc_controller.erl | 2 +- src/amoc_sup.erl | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/amoc_controller.erl b/src/amoc_controller.erl index 94a84f9b..4fe35ef8 100644 --- a/src/amoc_controller.erl +++ b/src/amoc_controller.erl @@ -79,7 +79,7 @@ %% ------------------------------------------------------------------ %% @private --spec start_link() -> {ok, pid()}. +-spec start_link() -> gen_server:start_ret(). start_link() -> gen_server:start_link({local, ?SERVER}, ?MODULE, [], []). diff --git a/src/amoc_sup.erl b/src/amoc_sup.erl index 89c9fb9b..fdc81d03 100644 --- a/src/amoc_sup.erl +++ b/src/amoc_sup.erl @@ -11,14 +11,14 @@ -export([init/1]). %% Helper macro for declaring children of supervisor --define(WORKER(I, Type), {I, {I, start_link, []}, permanent, 5000, Type, [I]}). --define(SUP(I, Type), {I, {I, start_link, []}, permanent, infinity, Type, [I]}). +-define(WORKER(I), {I, {I, start_link, []}, permanent, 5000, worker, [I]}). +-define(SUP(I), {I, {I, start_link, []}, permanent, infinity, supervisor, [I]}). %% =================================================================== %% API functions %% =================================================================== --spec start_link() -> {ok, Pid :: pid()}. +-spec start_link() -> supervisor:startlink_ret(). start_link() -> supervisor:start_link({local, ?MODULE}, ?MODULE, []). @@ -30,10 +30,10 @@ start_link() -> init([]) -> {ok, {#{strategy => one_for_all, intensity => 0}, [ - ?SUP(amoc_users_sup, supervisor), - ?SUP(amoc_throttle_sup, supervisor), - ?SUP(amoc_coordinator_sup, supervisor), - ?WORKER(amoc_controller, worker), - ?WORKER(amoc_cluster, worker), - ?WORKER(amoc_code_server, worker) + ?SUP(amoc_users_sup), + ?SUP(amoc_throttle_sup), + ?SUP(amoc_coordinator_sup), + ?WORKER(amoc_controller), + ?WORKER(amoc_cluster), + ?WORKER(amoc_code_server) ]}}. From 7d9299b369363ec2c7b3fc840b0772751f842fae Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 Jun 2024 22:48:32 +0200 Subject: [PATCH 9/9] Increase test coverage --- test/amoc_SUITE.erl | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/amoc_SUITE.erl b/test/amoc_SUITE.erl index 387f3ad3..dab640d1 100644 --- a/test/amoc_SUITE.erl +++ b/test/amoc_SUITE.erl @@ -13,6 +13,7 @@ all() -> start_and_then_force_remove_some_users, start_and_then_soft_remove_some_users, start_and_then_force_remove_more_users_than_running, + force_remove_more_users_with_no_running, start_and_then_soft_remove_users_that_ignore_the_error, start_and_then_stop_cannot_rerun, after_reset_can_run_again @@ -81,6 +82,14 @@ start_and_then_force_remove_more_users_than_running(_) -> ?assertEqual({ok, 2}, Removed), test_helpers:wait_until_scenario_has_users(testing_scenario, 0, 2). +force_remove_more_users_with_no_running(_) -> + Ret = amoc_do(testing_scenario, 0), + ?assertEqual(ok, Ret), + test_helpers:wait_until_scenario_has_users(testing_scenario, 0, 0), + Removed = amoc:remove(10, true), + ?assertEqual({ok, 0}, Removed), + test_helpers:wait_until_scenario_has_users(testing_scenario, 0, 0). + start_and_then_soft_remove_users_that_ignore_the_error(_) -> Ret = amoc_do(testing_scenario_with_state, 2, test_helpers:all_vars_with_state()), ?assertEqual(ok, Ret),