fix: use infinity shutdown for supervisor, use permanent restart for all

.github/workflows/erlang.yml

@@ -22,6 +22,8 @@ jobs:
             rebar3: "3.22"
           - otp: "27"
             rebar3: "3.24"
+          - otp: "28"
+            rebar3: "3.25"
 
     steps:
     - uses: actions/checkout@v3

src/ehttpc_pool_sup.erl

@@ -28,6 +28,11 @@ start_link(Pool, Opts) ->
     supervisor:start_link(?MODULE, [Pool, Opts]).
 
 init([Pool, Opts]) ->
+    SupFlags = #{
+        strategy => one_for_all,
+        intensity => 10,
+        period => 100
+    },
     Specs = [
         #{
             id => pool,
@@ -40,10 +45,10 @@ init([Pool, Opts]) ->
         #{
             id => worker_sup,
             start => {ehttpc_worker_sup, start_link, [Pool, Opts]},
-            restart => transient,
-            shutdown => 5000,
+            restart => permanent,
+            shutdown => infinity,
             type => supervisor,
             modules => [ehttpc_worker_sup]
         }
     ],
-    {ok, {{one_for_all, 10, 100}, Specs}}.
+    {ok, {SupFlags, Specs}}.

src/ehttpc_sup.erl

@@ -76,8 +76,8 @@ pool_spec(Pool, Opts) ->
     #{
         id => child_id(Pool),
         start => {ehttpc_pool_sup, start_link, [Pool, Opts]},
-        restart => transient,
-        shutdown => 5000,
+        restart => permanent,
+        shutdown => infinity,
         type => supervisor,
         modules => [ehttpc_pool_sup]
     }.

src/ehttpc_worker_sup.erl

@@ -37,8 +37,9 @@ init([Pool, Opts]) ->
         #{
             id => {Pool, Id},
             start => {ehttpc, start_link, [Pool, Id, Opts]},
-            restart => transient,
-            shutdown => 5000,
+            restart => permanent,
+            %% important: use brutal_kill for fast shutdown
+            shutdown => brutal_kill,
             type => worker,
             modules => [ehttpc]
         }

test/ehttpc_sup_tests.erl

@@ -55,55 +55,12 @@ wait_for_down([]) ->
     ok;
 wait_for_down(Pids) ->
     receive
-        {'DOWN', _, process, Pid, killed} ->
+        {'DOWN', _, process, Pid, _} ->
             wait_for_down(Pids -- [Pid])
     after 10_000 ->
         error(timeout)
     end.
 
-%% Smoke tests for `ecpool:check_pool_integrity`, which should report an error when worker
-%% supervisor is down.
-check_pool_integrity_test() ->
-    Pool = atom_to_binary(?MODULE),
-    Opts = [
-        {host, "google.com"},
-        {port, "80"},
-        {enable_pipelining, 1},
-        {pool_size, 15},
-        {pool_type, random},
-        {connect_timeout, 5000},
-        {prioritise_latest, true}
-    ],
-    application:ensure_all_started(ehttpc),
-    {ok, _} = ehttpc_sup:start_pool(Pool, Opts),
-    ?assertEqual(ok, ehttpc:check_pool_integrity(Pool)),
-    ok = ehttpc_sup:stop_pool(Pool),
-    {ok, _} = ehttpc_sup:start_pool(Pool, Opts),
-    Killer = spawn_link(fun Recur() ->
-        Workers = ehttpc:workers(Pool),
-        MRefs = lists:map(fun({_, P}) -> monitor(process, P) end, Workers),
-        lists:foreach(fun({_, P}) -> exit(P, kill) end, Workers),
-        lists:foreach(
-            fun(MRef) ->
-                receive
-                    {'DOWN', MRef, process, _, _} ->
-                        ok
-                end
-            end,
-            MRefs
-        ),
-        timer:sleep(1),
-        Recur()
-    end),
-    %% Give it some time to reach maximum restart intensity
-    timer:sleep(100),
-    ?assertEqual({error, {processes_down, [worker_sup]}}, ehttpc:check_pool_integrity(Pool)),
-    unlink(Killer),
-    exit(Killer, kill),
-    ok = ehttpc_sup:stop_pool(Pool),
-    ?assertEqual({error, not_found}, ehttpc:check_pool_integrity(Pool)),
-    ok.
-
 %% Previously, we had a fixed restart intensity for the worker supervisor, meaning that if
 %% a large pool dies once, it brings down the supervisor.  This checks that we have an
 %% intensity proportional to the pool size, so the whole pool may restart at once without