Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix alarm setting typo treshold -> threshold #350

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions doc/src/guide/listeners.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -332,14 +332,14 @@ processes.

The `alarms` transport option allows you to configure alarms
which will be triggered when the number of connections tracked
by one connection supervisor reaches or exceeds the defined treshold.
by one connection supervisor reaches or exceeds the defined threshold.

The `alarms` transport option takes a map with alarm names as keys and alarm
options as values.

Any term is allowed as an alarm name.

Alarm options include the alarm type and a treshold that, when reached,
Alarm options include the alarm type and a threshold that, when reached,
triggers the given callback. A cooldown prevents the alarm from being
triggered too often.

Expand All @@ -350,7 +350,7 @@ triggered too often.
Alarms = #{
my_alarm => #{
type => num_connections,
treshold => 100,
threshold => 100,
callback => fun(Ref, Name, ConnSup, ConnPids]) ->
logger:warning("Warning (~s): "
"Supervisor ~s of listener ~s "
Expand Down
2 changes: 1 addition & 1 deletion doc/src/guide/migrating_from_2.0.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ for Erlang/OTP 21 has been removed.

* Alarms can now be configured. The only alarm currently
available is `num_connections`. When the number of
connections goes over a configurable treshold Ranch
connections goes over a configurable threshold Ranch
will call the given callback. This can be used to
programmatically shut down idle connections to
make up space for new connections, for example.
Expand Down
8 changes: 4 additions & 4 deletions doc/src/manual/ranch.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ transport_opts(SocketOpts) = #{
alarms => #{
term() => #{
type := num_connections,
treshold := non_neg_integer(),
threshold := non_neg_integer(),
callback := fun((ref(), term(), pid(), [pid()]) -> any()),
cooldown => non_neg_integer()
}
Expand All @@ -118,7 +118,7 @@ None of the options are required.
alarms (#{})::

Alarms to call a function when the number of connections tracked
by one connection supervisor reaches or exceeds a defined treshold.
by one connection supervisor reaches or exceeds a defined threshold.
+
The map keys are the alarm names, which can be any `term`. The
associated values are the respective alarm options, again in a map
Expand All @@ -128,9 +128,9 @@ type:::

Must be set to `num_connections`.

treshold:::
threshold:::

Treshold value, which must be a `non_neg_integer`. When the
Threshold value, which must be a `non_neg_integer`. When the
number of connections tracked by a single connection supervisor
reaches or exceeds this value, The alarm will trigger and call
the function defined in the `callback` key (see below).
Expand Down
31 changes: 27 additions & 4 deletions src/ranch.erl
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
-export([require/1]).
-export([log/4]).

%% Internal
-export([compat_normalize_alarms_option/1]).

-type max_conns() :: non_neg_integer() | infinity.
-export_type([max_conns/0]).

Expand All @@ -59,7 +62,7 @@
-type alarm(Type, Callback) :: #{
type := Type,
callback := Callback,
treshold := non_neg_integer(),
threshold := non_neg_integer(),
cooldown => non_neg_integer()
}.

Expand Down Expand Up @@ -135,6 +138,7 @@ validate_transport_opt(max_connections, infinity, _) ->
validate_transport_opt(max_connections, Value, _) ->
is_integer(Value) andalso Value >= 0;
validate_transport_opt(alarms, Alarms, _) ->
Alarms1 = compat_normalize_alarms_option(Alarms),
maps:fold(
fun
(_, Opts, true) ->
Expand All @@ -143,7 +147,7 @@ validate_transport_opt(alarms, Alarms, _) ->
false
end,
true,
Alarms);
Alarms1);
validate_transport_opt(logger, Value, _) ->
is_atom(Value);
validate_transport_opt(num_acceptors, Value, _) ->
Expand All @@ -166,9 +170,9 @@ validate_transport_opt(socket_opts, _, _) ->
validate_transport_opt(_, _, _) ->
false.

validate_alarm(Alarm = #{type := num_connections, treshold := Treshold,
validate_alarm(Alarm = #{type := num_connections, threshold := Threshold,
callback := Callback}) ->
is_integer(Treshold) andalso Treshold >= 0
is_integer(Threshold) andalso Threshold >= 0
andalso is_function(Callback, 4)
andalso case Alarm of
#{cooldown := Cooldown} ->
Expand Down Expand Up @@ -632,3 +636,22 @@ log(Level, Format, Args, _) ->
debug -> info_msg
end,
error_logger:Function(Format, Args).

%% For backwards compatibility with the misspelled alarm
%% setting `treshold`.
%% See https://github.com/ninenines/ranch/issues/349
-spec compat_normalize_alarms_option(any()) -> any().
compat_normalize_alarms_option(Alarms = #{}) ->
maps:map(
fun
(_, Alarm = #{threshold := _}) ->
maps:remove(treshold, Alarm);
(_, Alarm = #{treshold := Threshold}) ->
maps:put(threshold, Threshold, maps:remove(treshold, Alarm));
(_, Alarm) ->
Alarm
end,
Alarms
);
compat_normalize_alarms_option(Alarms) ->
Alarms.
5 changes: 3 additions & 2 deletions src/ranch_conns_sup.erl
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ trigger_alarms(Ref, Alarms, CurConns) ->
Alarms
).

trigger_alarm(Ref, AlarmName, {Opts=#{treshold := Treshold, callback := Callback}, undefined}, CurConns) when CurConns >= Treshold ->
trigger_alarm(Ref, AlarmName, {Opts=#{threshold := Threshold, callback := Callback}, undefined}, CurConns) when CurConns >= Threshold ->
ActiveConns = [Pid || {Pid, active} <- get()],
case Callback of
{Mod, Fun} ->
Expand All @@ -306,6 +306,7 @@ schedule_activate_alarm(_, _) ->
undefined.

get_alarms(#{alarms := Alarms}) when is_map(Alarms) ->
Alarms1 = ranch:compat_normalize_alarms_option(Alarms),
maps:fold(
fun
(Name, Opts = #{type := num_connections, cooldown := _}, Acc) ->
Expand All @@ -315,7 +316,7 @@ get_alarms(#{alarms := Alarms}) when is_map(Alarms) ->
(_, _, Acc) -> Acc
end,
#{},
Alarms
Alarms1
);
get_alarms(_) ->
#{}.
Expand Down
4 changes: 3 additions & 1 deletion test/acceptor_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,9 @@ misc_connection_alarms(_) ->
Self ! {connection_alarm, {Ref, AlarmName, length(ActiveConns)}}
end,
Alarms0 = #{
test1 => Alarm1 = #{type => num_connections, treshold => 2, cooldown => 0, callback => AlarmCallback},
test1 => Alarm1 = #{type => num_connections, threshold => 2, cooldown => 0, callback => AlarmCallback},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably keep a test that uses treshold.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, see the next line after this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahah understood. In that case a comment before the next line should say that we are testing for backward compatibility but that it should be removed in 3.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in last commit.

%% The test2 alarm uses the misspelled treshold key to test for backwards compatibility.
%% @TODO: Change to use the proper spelling when treshold gets removed in Ranch 3.0.
test2 => Alarm2 = #{type => num_connections, treshold => 3, cooldown => 0, callback => AlarmCallback}
},
ConnectOpts = [binary, {active, false}, {packet, raw}],
Expand Down