From e33a5bfc890b04e77246251a4f3f1e70420b2e5c Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Fri, 31 Mar 2023 14:17:06 +0800
Subject: [PATCH] fix: update rule_cache when get_rules/0

---
 .../src/emqx_rule_engine.appup.src            | 32 ++++++-------------
 .../src/emqx_rule_registry.erl                | 15 +++++++--
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
index 3c677abe5..b2a339fe5 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
+++ b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src
@@ -4,21 +4,19 @@
   [{"4.4.16",
     [{load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine_app,brutal_purge,soft_purge,[]},
-     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}}
-     ]},
+     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}
+    ]},
    {"4.4.15",
     [{load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
-     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}}]},
+     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}
+    ]},
    {<<"4\\.4\\.1[3-4]">>,
     [{load_module,emqx_rule_engine_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_monitor,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
@@ -26,7 +24,6 @@
     [{load_module,emqx_rule_engine_app,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_monitor,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
@@ -42,7 +39,6 @@
      {update,emqx_rule_engine_jwt_sup,supervisor},
      {load_module,emqx_rule_engine_jwt,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_engine_sup,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
      {apply,{emqx_rule_engine_sup,ensure_api_delegator_started,[]}},
@@ -64,8 +60,8 @@
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
-     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}}]},
+     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}
+    ]},
    {"4.4.9",
     [{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {add_module,emqx_rule_engine_jwt},
@@ -83,7 +79,6 @@
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
    {"4.4.8",
@@ -102,7 +97,6 @@
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
@@ -124,8 +118,8 @@
      {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
-     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}}]},
+     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}
+    ]},
    {"4.4.5",
     [{add_module,emqx_rule_engine_jwt},
      {add_module,emqx_rule_engine_jwt_worker},
@@ -142,7 +136,6 @@
      {load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
@@ -164,7 +157,6 @@
      {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}]},
@@ -189,7 +181,6 @@
      {add_module,emqx_rule_date},
      {load_module,emqx_rule_maps,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
    {"4.4.2",
     [{add_module,emqx_rule_engine_jwt},
@@ -209,7 +200,6 @@
      {load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
      {add_module,emqx_rule_date},
@@ -237,9 +227,8 @@
      {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
      {add_module,emqx_rule_date},
-     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}}
-     ]},
+     {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}
+    ]},
    {"4.4.0",
     [{add_module,emqx_rule_engine_jwt},
      {add_module,emqx_rule_engine_jwt_worker},
@@ -259,7 +248,6 @@
      {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
      {add_module,emqx_rule_date},
      {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
-     {apply,{emqx_rule_registry,update_rules_cache,[]}},
      {update,emqx_rule_metrics,{advanced,["4.4.0"]}},
      {load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
      {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
diff --git a/apps/emqx_rule_engine/src/emqx_rule_registry.erl b/apps/emqx_rule_engine/src/emqx_rule_registry.erl
index 36950da87..fd73ba25a 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_registry.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_registry.erl
@@ -165,6 +165,10 @@ dump() ->
 start_link() ->
     gen_server:start_link({local, ?REGISTRY}, ?MODULE, [], []).
 
+%% Use a single process to protect the cache updating to avoid race conditions
+update_rules_cache_locally() ->
+    gen_server:cast(?REGISTRY, update_rules_cache).
+
 %%------------------------------------------------------------------------------
 %% Rule Management
 %%------------------------------------------------------------------------------
@@ -172,7 +176,9 @@ start_link() ->
 -spec(get_rules() -> list(emqx_rule_engine:rule())).
 get_rules() ->
     case get_rules_from_cache() of
-        not_found -> get_all_records(?RULE_TAB);
+        not_found ->
+            update_rules_cache_locally(),
+            get_all_records(?RULE_TAB);
         CachedRules -> CachedRules
     end.
 
@@ -186,7 +192,8 @@ update_rules_cache() ->
     put_rules_to_cache(get_all_records(?RULE_TAB)).
 
 clear_rules_cache() ->
-    persistent_term:erase(?PK_RULE_TAB).
+    _ = persistent_term:erase(?PK_RULE_TAB),
+    ok.
 
 get_rules_ordered_by_ts() ->
     lists:keysort(#rule.created_at, get_rules()).
@@ -472,6 +479,10 @@ handle_call(Req, _From, State) ->
     ?LOG(error, "[RuleRegistry]: unexpected call - ~p", [Req]),
     {reply, ignored, State}.
 
+handle_cast(update_rules_cache, State) ->
+    _ = update_rules_cache(),
+    {noreply, State};
+
 handle_cast(Msg, State) ->
     ?LOG(error, "[RuleRegistry]: unexpected cast ~p", [Msg]),
     {noreply, State}.