From: Matt Mullins <mmullins@mmlx.us>
Date: Mon, 4 Jan 2016 07:28:57 +0000 (-0800)
Subject: Factor out the rate limiting behavior from irc.
X-Git-Tag: v15~6
X-Git-Url: http://git.mmlx.us/?a=commitdiff_plain;h=e9aca78d5eafd64baab2162ee3dbfb345744f03a;p=erlbot.git

Factor out the rate limiting behavior from irc.

This will be more generically used to rate limit any code path that depends on
connecting to an external service.  Particularly: AMQP.

The goal is that external service availability should not tear down entire
supervisor hierarchies, and instead just the single worker connected to it.
---

diff --git a/core/rate_limit.erl b/core/rate_limit.erl
new file mode 100644
index 0000000..66fc72b
--- /dev/null
+++ b/core/rate_limit.erl
@@ -0,0 +1,42 @@
+% @doc Use a timer to rate limit a code path.
+%
+% Internally, this uses an ETS table to handle the mutable timer reference.
+% Erlang timers are not renewable, so we need mutable storage that can be
+% referenced in a supervisor's childspec.
+-module(rate_limit).
+-export([
+          create/0,
+          wait_and_reset/3
+        ]).
+
+% @doc Create storage for a rate limit timer.
+%
+% If the calling process exits, resources are cleaned up.  Hence, this must be
+% called from a process that will outlive the caller of
+% {@link wait_and_refresh/3}.
+%
+% @spec () -> integer()
+create() ->
+    % This should be public, since its owner should *not* be the same process
+    % as the writer -- the intended *use* of wait_and_reset/3 is at
+    % process-restart time.
+    ets:new(rate_limit, [set, public]).
+
+% @doc Wait for the named timer, if it has been set, then start a new timer.
+% @spec (integer(), atom(), Millis::integer()) -> ok
+wait_and_reset(TableId, TimerName, Time) ->
+    case ets:lookup(TableId, TimerName) of
+        % TODO: handle badarg, if the TableId is no longer valid
+        [{TimerName, TimerId}] ->
+            case erlang:read_timer(TimerId) of
+                false -> ok;
+                Millis ->
+                    % sleep that long in this process, since the process that set
+                    % the timer is long dead.
+                    timer:sleep(Millis)
+            end;
+        [] -> ok
+    end,
+    NewTimerId = erlang:start_timer(Time, 'dummy$$process', ok),
+    ets:insert(TableId, {TimerName, NewTimerId}),
+    ok.
diff --git a/irc/irc_conn.erl b/irc/irc_conn.erl
index 6cdd81e..463b975 100644
--- a/irc/irc_conn.erl
+++ b/irc/irc_conn.erl
@@ -47,22 +47,10 @@ send_amqp_command(Pid, Command, Body, Source) ->
 init({Instance, Supervisor, TableId}) ->
     ets:insert(TableId, {irc_conn_pid, self()}), % record the connection PID in the table
 
-    % The reconnect_timer sends a dummy message to a process that does not exist,
-    % simply so that we can read whether it has fired or not.  This lets us prevent
-    % reconnecting to the server too quickly.
-    case ets:lookup(TableId, reconnect_timer) of
-        [{reconnect_timer, TimerId}] ->
-            case erlang:read_timer(TimerId) of
-                false -> ok;
-                Millis ->
-                    % sleep that long in this process, since the process that set
-                    % the timer is long dead.
-                    timer:sleep(Millis)
-            end;
-        [] -> ok
-    end,
-    NewTimerId = erlang:start_timer(?RECONNECT_TIME, irc_dummy, ok),
-    ets:insert(TableId, {reconnect_timer, NewTimerId}),
+    % Don't reconnect to IRC too quickly.  Because of this, we should also
+    % never hit the irc_net_sup's restart intensity limit, so connection errors
+    % will never propagate to the top level supervisor.
+    rate_limit:wait_and_reset(TableId, reconnect_timer, ?RECONNECT_TIME),
 
     gen_server:cast(self(), create_object_sup), % This process is tasked with creating
                                                 % the irc_object_sup, but we defer until