Merge pull request #8911 from lafirest/fix/retainer_start_error

fix(retainer): fix that EMQX can't start when the retainer is disabled
emqx · Sep 9, 2022 · 6794551 · 6794551
2 parents 8062084 + 1a835e9
commit 6794551
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 9 deletions.
diff --git a/CHANGES-5.0.md b/CHANGES-5.0.md
@@ -8,6 +8,7 @@
 * Speed up dispatching of shared subscription messages in a cluster [#8893](https://github.com/emqx/emqx/pull/8893)
 * Fix the extra / prefix when CoAP gateway parsing client topics. [#8658](https://github.com/emqx/emqx/pull/8658)
 * Speed up updating the configuration, When some nodes in the cluster are down. [#8857](https://github.com/emqx/emqx/pull/8857)
+* Fix that EMQX can't start when the retainer is disabled [#8911](https://github.com/emqx/emqx/pull/8911)
 
 ## Enhancements
 

diff --git a/apps/emqx_retainer/src/emqx_retainer.app.src b/apps/emqx_retainer/src/emqx_retainer.app.src
@@ -2,7 +2,7 @@
 {application, emqx_retainer, [
     {description, "EMQX Retainer"},
     % strict semver, bump manually!
-    {vsn, "5.0.4"},
+    {vsn, "5.0.5"},
     {modules, []},
     {registered, [emqx_retainer_sup]},
     {applications, [kernel, stdlib, emqx]},

diff --git a/apps/emqx_retainer/src/emqx_retainer.erl b/apps/emqx_retainer/src/emqx_retainer.erl
@@ -348,16 +348,12 @@ enable_retainer(
     #{context_id := ContextId} = State,
     #{
         msg_clear_interval := ClearInterval,
-        backend := BackendCfg,
-        flow_control := FlowControl
+        backend := BackendCfg
     }
 ) ->
     NewContextId = ContextId + 1,
     Context = create_resource(new_context(NewContextId), BackendCfg),
     load(Context),
-    emqx_limiter_server:add_bucket(
-        ?APP, internal, maps:get(batch_deliver_limiter, FlowControl, undefined)
-    ),
     State#{
         enable := true,
         context_id := NewContextId,
@@ -373,7 +369,6 @@ disable_retainer(
     } = State
 ) ->
     unload(),
-    emqx_limiter_server:del_bucket(?APP, internal),
     ok = close_resource(Context),
     State#{
         enable := false,

diff --git a/apps/emqx_retainer/src/emqx_retainer_app.erl b/apps/emqx_retainer/src/emqx_retainer_app.erl
@@ -18,15 +18,28 @@
 
 -behaviour(application).
 
+-include("emqx_retainer.hrl").
+
 -export([
     start/2,
     stop/1
 ]).
 
 start(_Type, _Args) ->
     ok = emqx_retainer_mnesia_cli:load(),
+    init_bucket(),
     emqx_retainer_sup:start_link().
 
 stop(_State) ->
     ok = emqx_retainer_mnesia_cli:unload(),
+    delete_bucket(),
     ok.
+
+init_bucket() ->
+    #{flow_control := FlowControl} = emqx:get_config([retainer]),
+    emqx_limiter_server:add_bucket(
+        ?APP, internal, maps:get(batch_deliver_limiter, FlowControl, undefined)
+    ).
+
+delete_bucket() ->
+    emqx_limiter_server:del_bucket(?APP, internal).
diff --git a/apps/emqx_retainer/test/emqx_retainer_SUITE.erl b/apps/emqx_retainer/test/emqx_retainer_SUITE.erl
@@ -31,14 +31,16 @@ all() ->
     [
         {group, mnesia_without_indices},
         {group, mnesia_with_indices},
-        {group, mnesia_reindex}
+        {group, mnesia_reindex},
+        {group, test_disable_then_start}
     ].
 
 groups() ->
     [
         {mnesia_without_indices, [sequence], common_tests()},
         {mnesia_with_indices, [sequence], common_tests()},
-        {mnesia_reindex, [sequence], [t_reindex]}
+        {mnesia_reindex, [sequence], [t_reindex]},
+        {test_disable_then_start, [sequence], [test_disable_then_start]}
     ].
 
 common_tests() ->
@@ -624,6 +626,19 @@ t_get_basic_usage_info(_Config) ->
     ?assertEqual(#{retained_messages => 5}, emqx_retainer:get_basic_usage_info()),
     ok.
 
+%% test whether the app can start normally after disabling emqx_retainer
+%% fix: https://github.com/emqx/emqx/pull/8911
+test_disable_then_start(_Config) ->
+    emqx_retainer:update_config(#{<<"enable">> => false}),
+    ?assertNotEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)),
+    ok = application:stop(emqx_retainer),
+    timer:sleep(100),
+    ?assertEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)),
+    ok = application:ensure_started(emqx_retainer),
+    timer:sleep(100),
+    ?assertNotEqual([], gproc_pool:active_workers(emqx_retainer_dispatcher)),
+    ok.
+
 %%--------------------------------------------------------------------
 %% Helper functions
 %%--------------------------------------------------------------------