Skip to content

Commit 6295123

Browse files
committed
rabbit_alarm: Prefer maps to dicts
This is not a functional change, just a refactor to eliminate dicts and use maps instead. This cleans up some helper functions like dict_append/3, and we can use map comprehensions in some places to avoid intermediary lists.
1 parent 2116c0c commit 6295123

File tree

1 file changed

+38
-46
lines changed

1 file changed

+38
-46
lines changed

deps/rabbit/src/rabbit_alarm.erl

Lines changed: 38 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@
4242

4343
%%----------------------------------------------------------------------------
4444

45-
-record(alarms, {alertees :: dict:dict(pid(), rabbit_types:mfargs()),
46-
alarmed_nodes :: dict:dict(node(), [resource_alarm_source()]),
47-
alarms :: [alarm()]}).
45+
-record(alarms, {alertees = #{} :: #{pid() => rabbit_types:mfargs()},
46+
alarmed_nodes = #{} :: #{node() => [resource_alarm_source()]},
47+
alarms = [] :: [alarm()]}).
4848

4949
-export_type([alarm/0]).
5050
-type local_alarm() :: 'file_descriptor_limit'.
@@ -90,7 +90,7 @@ stop() -> ok.
9090
%% called like this: `apply(M, F, A ++ [Pid, Source, Alert])', where `Source'
9191
%% has the type of resource_alarm_source() and `Alert' has the type of resource_alert().
9292

93-
-spec register(pid(), rabbit_types:mfargs()) -> [atom()].
93+
-spec register(pid(), rabbit_types:mfargs()) -> [resource_alarm_source()].
9494

9595
register(Pid, AlertMFA) ->
9696
gen_event:call(?SERVER, ?MODULE, {register, Pid, AlertMFA}, infinity).
@@ -177,12 +177,10 @@ remote_conserve_resources(Pid, Source, {false, _, _}) ->
177177
%%----------------------------------------------------------------------------
178178

179179
init([]) ->
180-
{ok, #alarms{alertees = dict:new(),
181-
alarmed_nodes = dict:new(),
182-
alarms = []}}.
180+
{ok, #alarms{}}.
183181

184182
handle_call({register, Pid, AlertMFA}, State = #alarms{alarmed_nodes = AN}) ->
185-
{ok, lists:usort(lists:append([V || {_, V} <- dict:to_list(AN)])),
183+
{ok, lists:usort(lists:append(maps:values(AN))),
186184
internal_register(Pid, AlertMFA, State)};
187185

188186
handle_call(get_alarms, State) ->
@@ -236,14 +234,11 @@ handle_event({node_up, Node}, State) ->
236234
{ok, State};
237235

238236
handle_event({node_down, Node}, #alarms{alarmed_nodes = AN} = State) ->
239-
AlarmsForDeadNode = case dict:find(Node, AN) of
240-
{ok, V} -> V;
241-
error -> []
242-
end,
237+
AlarmsForDeadNode = maps:get(Node, AN, []),
243238
{ok, lists:foldr(fun(Source, AccState) ->
244239
?LOG_WARNING("~ts resource limit alarm cleared for dead node ~tp",
245240
[Source, Node]),
246-
maybe_alert(fun dict_unappend/3, Node, Source, false, AccState)
241+
maybe_alert(fun map_unappend/3, Node, Source, false, AccState)
247242
end, State, AlarmsForDeadNode)};
248243

249244
handle_event({register, Pid, AlertMFA}, State) ->
@@ -254,7 +249,7 @@ handle_event(_Event, State) ->
254249

255250
handle_info({'DOWN', _MRef, process, Pid, _Reason},
256251
State = #alarms{alertees = Alertees}) ->
257-
{ok, State#alarms{alertees = dict:erase(Pid, Alertees)}};
252+
{ok, State#alarms{alertees = maps:remove(Pid, Alertees)}};
258253

259254
handle_info(_Info, State) ->
260255
{ok, State}.
@@ -267,30 +262,25 @@ code_change(_OldVsn, State, _Extra) ->
267262

268263
%%----------------------------------------------------------------------------
269264

270-
dict_append(Key, Val, Dict) ->
271-
L = case dict:find(Key, Dict) of
272-
{ok, V} -> V;
273-
error -> []
274-
end,
275-
dict:store(Key, lists:usort([Val|L]), Dict).
276-
277-
dict_unappend(Key, Val, Dict) ->
278-
L = case dict:find(Key, Dict) of
279-
{ok, V} -> V;
280-
error -> []
281-
end,
265+
map_append(Key, Val, Map) ->
266+
maps:update_with(Key, fun(Vs) -> [Val | Vs] end, [Val], Map).
282267

268+
map_unappend(Key, Val, Map) ->
269+
L = maps:get(Key, Map, []),
283270
case lists:delete(Val, L) of
284-
[] -> dict:erase(Key, Dict);
285-
X -> dict:store(Key, X, Dict)
271+
[] -> maps:remove(Key, Map);
272+
X -> Map#{Key := X}
286273
end.
287274

288275
maybe_alert(UpdateFun, Node, Source, WasAlertAdded,
289276
State = #alarms{alarmed_nodes = AN,
290277
alertees = Alertees}) ->
291278
AN1 = UpdateFun(Node, Source, AN),
292279
%% Is alarm for Source still set on any node?
293-
StillHasAlerts = lists:any(fun ({_Node, NodeAlerts}) -> lists:member(Source, NodeAlerts) end, dict:to_list(AN1)),
280+
StillHasAlerts = rabbit_misc:maps_any(
281+
fun(_Node, NodeAlerts) ->
282+
lists:member(Source, NodeAlerts)
283+
end, AN1),
294284
case StillHasAlerts of
295285
true -> ok;
296286
false -> ?LOG_WARNING("~ts resource limit alarm cleared across the cluster", [Source])
@@ -311,22 +301,24 @@ alert_remote(Alert, Alertees, Source) ->
311301

312302
alert(Alertees, Source, Alert, NodeComparator) ->
313303
Node = node(),
314-
dict:fold(fun (Pid, {M, F, A}, ok) ->
315-
case NodeComparator(Node, node(Pid)) of
316-
true -> apply(M, F, A ++ [Pid, Source, Alert]);
317-
false -> ok
318-
end
319-
end, ok, Alertees).
304+
maps:foreach(fun (Pid, {M, F, A}) ->
305+
case NodeComparator(Node, node(Pid)) of
306+
true -> apply(M, F, A ++ [Pid, Source, Alert]);
307+
false -> ok
308+
end
309+
end, Alertees).
320310

321311
internal_register(Pid, {M, F, A} = AlertMFA,
322312
State = #alarms{alertees = Alertees}) ->
323313
_MRef = erlang:monitor(process, Pid),
324-
_ = case dict:find(node(), State#alarms.alarmed_nodes) of
325-
{ok, Sources} -> [apply(M, F, A ++ [Pid, R, {true, true, node()}]) || R <- Sources];
326-
error -> ok
314+
Node = node(),
315+
_ = case State#alarms.alarmed_nodes of
316+
#{Node := Sources} ->
317+
[apply(M, F, A ++ [Pid, R, {true, true, node()}]) || R <- Sources];
318+
_ ->
319+
ok
327320
end,
328-
NewAlertees = dict:store(Pid, AlertMFA, Alertees),
329-
State#alarms{alertees = NewAlertees}.
321+
State#alarms{alertees = Alertees#{Pid => AlertMFA}}.
330322

331323
handle_set_resource_alarm(Source, Node, State) ->
332324
?LOG_WARNING(
@@ -335,7 +327,7 @@ handle_set_resource_alarm(Source, Node, State) ->
335327
"*** Publishers will be blocked until this alarm clears ***~n"
336328
"**********************************************************~n",
337329
[Source, Node]),
338-
{ok, maybe_alert(fun dict_append/3, Node, Source, true, State)}.
330+
{ok, maybe_alert(fun map_append/3, Node, Source, true, State)}.
339331

340332
handle_set_alarm({file_descriptor_limit, []}, State) ->
341333
?LOG_WARNING(
@@ -351,7 +343,7 @@ handle_set_alarm(Alarm, State) ->
351343
handle_clear_resource_alarm(Source, Node, State) ->
352344
?LOG_WARNING("~ts resource limit alarm cleared on node ~tp",
353345
[Source, Node]),
354-
{ok, maybe_alert(fun dict_unappend/3, Node, Source, false, State)}.
346+
{ok, maybe_alert(fun map_unappend/3, Node, Source, false, State)}.
355347

356348
handle_clear_alarm(file_descriptor_limit, State) ->
357349
?LOG_WARNING("file descriptor limit alarm cleared~n"),
@@ -361,14 +353,14 @@ handle_clear_alarm(Alarm, State) ->
361353
{ok, State}.
362354

363355
is_node_alarmed(Source, Node, #alarms{alarmed_nodes = AN}) ->
364-
case dict:find(Node, AN) of
365-
{ok, Sources} ->
356+
case AN of
357+
#{Node := Sources} ->
366358
lists:member(Source, Sources);
367-
error ->
359+
_ ->
368360
false
369361
end.
370362

371363
compute_alarms(#alarms{alarms = Alarms,
372364
alarmed_nodes = AN}) ->
373365
Alarms ++ [ {{resource_limit, Source, Node}, []}
374-
|| {Node, Sources} <- dict:to_list(AN), Source <- Sources ].
366+
|| Node := Sources <- AN, Source <- Sources ].

0 commit comments

Comments
 (0)