Merge pull request #3889 from esl/remove_domain/incremental_db_deletes

Remove domain/incremental db deletes

big_tests/tests/domain_removal_SUITE.erl

@@ -19,6 +19,7 @@ all() ->
      {group, mam_removal},
      {group, mam_removal_incremental},
      {group, inbox_removal},
+     {group, inbox_removal_incremental},
      {group, muc_light_removal},
      {group, muc_removal},
      {group, private_removal},
@@ -39,6 +40,7 @@ groups() ->
      {mam_removal_incremental, [], [mam_pm_removal,
                                     mam_muc_removal]},
      {inbox_removal, [], [inbox_removal]},
+     {inbox_removal_incremental, [], [inbox_removal]},
      {muc_light_removal, [], [muc_light_removal,
                               muc_light_blocking_removal]},
      {muc_removal, [], [muc_removal]},
@@ -108,6 +110,8 @@ group_to_modules(muc_removal) ->
     [{mod_muc, muc_helper:make_opts(Opts)}];
 group_to_modules(inbox_removal) ->
     [{mod_inbox, inbox_helper:inbox_opts()}];
+group_to_modules(inbox_removal_incremental) ->
+    [{mod_inbox, (inbox_helper:inbox_opts())#{delete_domain_limit => 1}}];
 group_to_modules(private_removal) ->
     [{mod_private, #{iqdisc => one_queue, backend => rdbms}}];
 group_to_modules(roster_removal) ->

doc/modules/mod_inbox.md

@@ -48,6 +48,14 @@ How old entries in the bin can be before the automatic bin cleaner collects them
 
 How often the automatic garbage collection runs over the bin.
 
+#### `modules.mod_inbox.delete_domain_limit`
+
+* **Syntax:** non-negative integer or the string `"infinity"`
+* **Default:** `"infinity"`
+* **Example:** `modules.mod_inbox.delete_domain_limit = 10000`
+
+Domain deletion can be an expensive operation, as it requires to delete potentially many thousands of records from the DB. By default, the delete operation deletes everything in a transaction, but it might be desired, to handle timeouts and table locks more gracefully, to delete the records in batches. This limit establishes the size of the batch.
+
 ### `modules.mod_inbox.reset_markers`
 * **Syntax:** array of strings, out of `"displayed"`, `"received"`, `"acknowledged"`
 * **Default:** `["displayed"]`

src/inbox/mod_inbox.erl

@@ -131,6 +131,8 @@ config_spec() ->
                   <<"bin_ttl">> => #option{type = integer, validate = non_negative},
                   <<"bin_clean_after">> => #option{type = integer, validate = non_negative,
                                                    process = fun timer:hours/1},
+                  <<"delete_domain_limit">> => #option{type = int_or_infinity,
+                                                       validate = positive},
                   <<"aff_changes">> => #option{type = boolean},
                   <<"remove_on_kicked">> => #option{type = boolean},
                   <<"iqdisc">> => mongoose_config_spec:iqdisc()
@@ -140,6 +142,7 @@ config_spec() ->
                      <<"boxes">> => [],
                      <<"bin_ttl">> => 30, % 30 days
                      <<"bin_clean_after">> => timer:hours(1),
+                     <<"delete_domain_limit">> => infinity,
                      <<"aff_changes">> => true,
                      <<"remove_on_kicked">> => true,
                      <<"reset_markers">> => [<<"displayed">>],
@@ -298,7 +301,7 @@ remove_user(Acc, #{jid := #jid{luser = User, lserver = Server}}, _) ->
 -spec remove_domain(Acc, Params, Extra) -> {ok | stop, Acc} when
       Acc :: mongoose_domain_api:remove_domain_acc(),
       Params :: #{domain := jid:lserver()},
-      Extra :: #{host_type := mongooseim:host_type()}.
+      Extra :: gen_hook:extra().
 remove_domain(Acc, #{domain := Domain}, #{host_type := HostType}) ->
     F = fun() ->
             mod_inbox_backend:remove_domain(HostType, Domain),

src/inbox/mod_inbox_backend.erl

@@ -37,7 +37,7 @@
     LUser :: jid:luser(),
     LServer :: jid:lserver().
 
--callback remove_domain(HostType, LServer) -> ok when
+-callback remove_domain(HostType, LServer) -> term() when
     HostType :: mongooseim:host_type(),
     LServer :: jid:lserver().
 
@@ -128,7 +128,7 @@ clear_inbox(HostType, LUser, LServer) ->
     Args = [HostType, LUser, LServer],
     mongoose_backend:call_tracked(HostType, ?MAIN_MODULE, ?FUNCTION_NAME, Args).
 
--spec remove_domain(HostType, LServer) -> ok when
+-spec remove_domain(HostType, LServer) -> term() when
     HostType :: mongooseim:host_type(),
     LServer :: jid:lserver().
 remove_domain(HostType, LServer) ->

src/inbox/mod_inbox_rdbms.erl

@@ -35,7 +35,7 @@
 %% ----------------------------------------------------------------------
 
 %% TODO pools aren't multitenancy-ready yet
-init(HostType, _Options) ->
+init(HostType, Opts) ->
     RowCond = <<"WHERE luser = ? AND lserver = ? AND remote_bare_jid = ?">>,
     mongoose_rdbms:prepare(inbox_select_entry, inbox,
                            [luser, lserver, remote_bare_jid],
@@ -69,8 +69,7 @@ init(HostType, _Options) ->
     mongoose_rdbms:prepare(inbox_delete, inbox,
                            [luser, lserver],
                            <<"DELETE FROM inbox WHERE luser = ? AND lserver = ?">>),
-    mongoose_rdbms:prepare(inbox_delete_domain, inbox,
-                           [lserver], <<"DELETE FROM inbox WHERE lserver = ?">>),
+    prepare_remove_domain(Opts),
     % upserts
     BoxQuery = <<"CASE WHEN ?='bin' THEN 'bin'",
                      " WHEN inbox.box='archive' THEN 'inbox'",
@@ -95,6 +94,12 @@ init(HostType, _Options) ->
                                  UniqueKeyFields, <<"timestamp">>),
     ok.
 
+prepare_remove_domain(Opts) ->
+    {MaybeLimitSQL, MaybeLimitMSSQL} = mod_mam_utils:batch_delete_limits(Opts),
+    mongoose_rdbms:prepare(
+      inbox_delete_domain, inbox, [lserver],
+      <<"DELETE ", MaybeLimitMSSQL/binary, "FROM inbox WHERE lserver = ? ", MaybeLimitSQL/binary>>).
+
 -spec get_inbox(HostType :: mongooseim:host_type(),
                 LUser :: jid:luser(),
                 LServer :: jid:lserver(),
@@ -170,10 +175,10 @@ remove_inbox_row(HostType, {LUser, LServer, LToBareJid}) ->
     check_result(Res).
 
 -spec remove_domain(HostType :: mongooseim:host_type(),
-                    LServer :: jid:lserver()) -> ok.
+                    LServer :: jid:lserver()) -> term().
 remove_domain(HostType, LServer) ->
-    execute_delete_domain(HostType, LServer),
-    ok.
+    DeleteDomainLimit = gen_mod:get_module_opt(HostType, mod_inbox, delete_domain_limit),
+    execute_delete_domain(HostType, LServer, DeleteDomainLimit).
 
 -spec set_inbox_incr_unread(
         mongooseim:host_type(), mod_inbox:entry_key(), exml:element(), binary(), integer()) ->
@@ -471,11 +476,12 @@ execute_delete(HostType, LUser, LServer, RemBareJID) ->
 execute_delete(HostType, LUser, LServer) ->
     mongoose_rdbms:execute_successfully(HostType, inbox_delete, [LUser, LServer]).
 
--spec execute_delete_domain(HostType :: mongooseim:host_type(),
-                            LServer :: jid:lserver()) ->
+-spec execute_delete_domain(mongooseim:host_type(), jid:lserver(), infinity | non_neg_integer()) ->
     mongoose_rdbms:query_result().
-execute_delete_domain(HostType, LServer) ->
-    mongoose_rdbms:execute_successfully(HostType, inbox_delete_domain, [LServer]).
+execute_delete_domain(HostType, LServer, infinity) ->
+    mongoose_rdbms:execute_successfully(HostType, inbox_delete_domain, [LServer]);
+execute_delete_domain(HostType, LServer, Limit) ->
+    mod_mam_utils:incremental_delete_domain(HostType, LServer, Limit, [inbox_delete_domain], 0).
 
 %% DB result processing
 -type db_return() :: {RemBareJID :: jid:luser(),

src/inbox/mod_inbox_rdbms_async.erl

@@ -138,7 +138,7 @@ get_inbox(HostType, LUser, LServer, Params) ->
 get_inbox_unread(HostType, Entry) ->
     mod_inbox_rdbms:get_inbox_unread(HostType, Entry).
 
--spec remove_domain(mongooseim:host_type(), jid:lserver()) -> ok.
+-spec remove_domain(mongooseim:host_type(), jid:lserver()) -> term().
 remove_domain(HostType, LServer) ->
     mod_inbox_rdbms:remove_domain(HostType, LServer).
 

src/mam/mod_mam_utils.erl

@@ -1226,7 +1226,7 @@ db_jid_codec(HostType, Module) ->
 db_message_codec(HostType, Module) ->
     gen_mod:get_module_opt(HostType, Module, db_message_format).
 
--spec batch_delete_limits(#{delete_domain_limit := infinity | non_neg_integer()}) ->
+-spec batch_delete_limits(#{delete_domain_limit := infinity | non_neg_integer(), _ => _}) ->
     {binary(), binary()}.
 batch_delete_limits(#{delete_domain_limit := infinity}) ->
     {<<>>, <<>>};

test/common/config_parser_helper.erl

@@ -560,6 +560,7 @@ all_modules() ->
             bin_clean_after => timer:hours(1),
             iqdisc => no_queue,
             aff_changes => true,
+            delete_domain_limit => infinity,
             groupchat => [muclight],
             remove_on_kicked => true,
             reset_markers => [<<"displayed">>]},
@@ -880,6 +881,7 @@ default_mod_config(mod_inbox) ->
       bin_clean_after => timer:hours(1),
       groupchat => [muclight],
       aff_changes => true,
+      delete_domain_limit => infinity,
       remove_on_kicked => true,
       reset_markers => [<<"displayed">>],
       iqdisc => no_queue};

test/config_parser_SUITE.erl

@@ -1554,6 +1554,7 @@ mod_inbox(_Config) ->
     ?cfgh(P ++ [bin_ttl], 30, T(#{<<"bin_ttl">> => 30})),
     ?cfgh(P ++ [bin_clean_after], 43200000, T(#{<<"bin_clean_after">> => 12})),
     ?cfgh(P ++ [aff_changes], true, T(#{<<"aff_changes">> => true})),
+    ?cfgh(P ++ [delete_domain_limit], 1000, T(#{<<"delete_domain_limit">> => 1000})),
     ?cfgh(P ++ [remove_on_kicked], false, T(#{<<"remove_on_kicked">> => false})),
     ?errh(T(#{<<"backend">> => <<"nodejs">>})),
     ?errh(T(#{<<"pool_size">> => -1})),
@@ -1566,6 +1567,7 @@ mod_inbox(_Config) ->
     ?errh(T(#{<<"bin_ttl">> => true})),
     ?errh(T(#{<<"bin_clean_after">> => -1})),
     ?errh(T(#{<<"aff_changes">> => 1})),
+    ?errh(T(#{<<"delete_domain_limit">> => []})),
     ?errh(T(#{<<"remove_on_kicked">> => 1})).
 
 mod_global_distrib(_Config) ->