Merge pull request #3924 from esl/inbox/incremental_domain_removal

Fix query that creates inbox and mam delete domain statement
esl · Jan 11, 2023 · 8275095 · 8275095
2 parents 759b64a + e658b06
commit 8275095
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 40 deletions.
diff --git a/big_tests/tests/domain_removal_SUITE.erl b/big_tests/tests/domain_removal_SUITE.erl
@@ -66,7 +66,16 @@ end_per_suite(Config) ->
 %%%===================================================================
 %%% Group specific setup/teardown
 %%%===================================================================
+init_per_group(Group, Config)
+  when Group =:= mam_removal_incremental; Group =:= inbox_removal_incremental ->
+    case rpc(mim(), mongoose_rdbms, db_engine, [host_type()]) of
+        odbc -> {skip, mssql_not_supported_for_incremental_removals};
+        _ -> do_init_per_group(Group, Config)
+    end;
 init_per_group(Group, Config) ->
+    do_init_per_group(Group, Config).
+
+do_init_per_group(Group, Config) ->
     case mongoose_helper:is_rdbms_enabled(host_type()) of
         true ->
             HostTypes = domain_helper:host_types(),

diff --git a/doc/modules/mod_inbox.md b/doc/modules/mod_inbox.md
@@ -56,6 +56,9 @@ How often the automatic garbage collection runs over the bin.
 
 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.
 
+!!! Note
+    Not supported by MSSQL.
+
 ### `modules.mod_inbox.reset_markers`
 * **Syntax:** array of strings, out of `"displayed"`, `"received"`, `"acknowledged"`
 * **Default:** `["displayed"]`

diff --git a/doc/modules/mod_mam.md b/doc/modules/mod_mam.md
@@ -368,6 +368,9 @@ See [Message Archive Management extensions](../open-extensions/mam.md).
 
 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.
 
+!!! Note
+    Not supported by MSSQL.
+
 #### `modules.mod_mam.db_jid_format`
 
 * **Syntax:** string, one of `"mam_jid_rfc"`, `"mam_jid_rfc_trust"`, `"mam_jid_mini"` or a module implementing `mam_jid` behaviour

diff --git a/src/inbox/mod_inbox_rdbms.erl b/src/inbox/mod_inbox_rdbms.erl
@@ -94,11 +94,21 @@ init(HostType, Opts) ->
                                  UniqueKeyFields, <<"timestamp">>),
     ok.
 
-prepare_remove_domain(Opts) ->
-    {MaybeLimitSQL, MaybeLimitMSSQL} = mod_mam_utils:batch_delete_limits(Opts),
+prepare_remove_domain(#{delete_domain_limit := infinity}) ->
     mongoose_rdbms:prepare(
-      inbox_delete_domain, inbox, [lserver],
-      <<"DELETE ", MaybeLimitMSSQL/binary, "FROM inbox WHERE lserver = ? ", MaybeLimitSQL/binary>>).
+      inbox_delete_domain, inbox, [lserver], <<"DELETE FROM inbox WHERE lserver = ?">>);
+prepare_remove_domain(#{delete_domain_limit := Limit}) ->
+    LimitSQL = case mongoose_rdbms:db_type() of
+                        mssql -> throw(delete_domain_limit_not_supported_for_mssql);
+                        _ -> {MaybeLimitSQL, _} = rdbms_queries:get_db_specific_limits_binaries(Limit),
+                             MaybeLimitSQL
+                    end,
+    ServerTable = <<"(SELECT * FROM ",
+                        "(SELECT lserver, luser, remote_bare_jid FROM inbox",
+                        " WHERE lserver = ? ", LimitSQL/binary, ") AS T)">>,
+    mongoose_rdbms:prepare(
+      inbox_incr_delete_domain, inbox, [lserver],
+      <<"DELETE FROM inbox WHERE (lserver, luser, remote_bare_jid) IN ", ServerTable/binary>>).
 
 -spec get_inbox(HostType :: mongooseim:host_type(),
                 LUser :: jid:luser(),
@@ -481,7 +491,7 @@ execute_delete(HostType, LUser, 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).
+    mod_mam_utils:incremental_delete_domain(HostType, LServer, Limit, [inbox_incr_delete_domain], 0).
 
 %% DB result processing
 -type db_return() :: {RemBareJID :: jid:luser(),

diff --git a/src/mam/mod_mam_muc_rdbms_arch.erl b/src/mam/mod_mam_muc_rdbms_arch.erl
@@ -114,17 +114,7 @@ register_prepared_queries(Opts) ->
                              "WHERE room_id = ?">>),
 
     %% Domain Removal
-    {MaybeLimitSQL, MaybeLimitMSSQL} = mod_mam_utils:batch_delete_limits(Opts),
-    IdTable = <<"(SELECT ", MaybeLimitMSSQL/binary,
-                " id from mam_server_user WHERE server = ? ", MaybeLimitSQL/binary, ")">>,
-    ServerTable = <<"(SELECT * FROM (SELECT", MaybeLimitMSSQL/binary,
-                    " server FROM mam_server_user WHERE server = ? ", MaybeLimitSQL/binary, ") as t)">>,
-    mongoose_rdbms:prepare(mam_muc_remove_domain, mam_muc_message, ['mam_server_user.server'],
-                           <<"DELETE FROM mam_muc_message "
-                             "WHERE room_id IN ", IdTable/binary>>),
-    mongoose_rdbms:prepare(mam_muc_remove_domain_users, mam_server_user, [server],
-                           <<"DELETE ", MaybeLimitMSSQL/binary,
-                             " FROM mam_server_user WHERE server IN", ServerTable/binary>>),
+    prepare_remove_domain(Opts),
 
     mongoose_rdbms:prepare(mam_muc_make_tombstone, mam_muc_message, [message, room_id, id],
                            <<"UPDATE mam_muc_message SET message = ?, search_body = '' "
@@ -148,6 +138,29 @@ register_prepared_queries(Opts) ->
                            <<"SELECT id, message FROM mam_muc_message "
                              " WHERE sender_id = ? ORDER BY id">>).
 
+prepare_remove_domain(#{delete_domain_limit := infinity}) ->
+    mongoose_rdbms:prepare(mam_muc_remove_domain, mam_muc_message, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_muc_message "
+                             "WHERE room_id IN (SELECT id FROM mam_server_user where server = ?)">>),
+    mongoose_rdbms:prepare(mam_muc_remove_domain_users, mam_server_user, [server],
+                           <<"DELETE FROM mam_server_user WHERE server = ? ">>);
+prepare_remove_domain(#{delete_domain_limit := Limit}) ->
+    LimitSQL = case mongoose_rdbms:db_type() of
+                        mssql -> throw(delete_domain_limit_not_supported_for_mssql);
+                        _ -> {MaybeLimitSQL, _} = rdbms_queries:get_db_specific_limits_binaries(Limit),
+                             MaybeLimitSQL
+                    end,
+    IdTable = <<"(SELECT * FROM ",
+                    "(SELECT msg.room_id, msg.id FROM mam_muc_message msg",
+                    " INNER JOIN mam_server_user msu ON msu.id=msg.room_id",
+                    " WHERE msu.server = ? ", LimitSQL/binary, ") AS T)">>,
+    mongoose_rdbms:prepare(mam_muc_incr_remove_domain, mam_muc_message, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_muc_message WHERE (room_id, id) IN ", IdTable/binary>>),
+    ServerTable = <<"(SELECT * FROM",
+                        "(SELECT id FROM mam_server_user WHERE server = ? ", LimitSQL/binary, ") as t)">>,
+    mongoose_rdbms:prepare(mam_muc_incr_remove_domain_users, mam_server_user, [server],
+                           <<"DELETE FROM mam_server_user WHERE id IN ", ServerTable/binary>>).
+
 %% ----------------------------------------------------------------------
 %% Declarative logic
 
@@ -341,7 +354,7 @@ remove_domain_all(HostType, Domain) ->
 -spec remove_domain_batch(host_type(), jid:lserver(), non_neg_integer()) -> any().
 remove_domain_batch(HostType, Domain, Limit) ->
     SubHosts = get_subhosts(HostType, Domain),
-    DeleteQueries = [mam_muc_remove_domain, mam_muc_remove_domain_users],
+    DeleteQueries = [mam_muc_incr_remove_domain, mam_muc_incr_remove_domain_users],
     DelSubHost = [ mod_mam_utils:incremental_delete_domain(HostType, SubHost, Limit, DeleteQueries, 0)
                    || SubHost <- SubHosts],
     TotalDeleted = lists:sum(DelSubHost),

diff --git a/src/mam/mod_mam_rdbms_arch.erl b/src/mam/mod_mam_rdbms_arch.erl
@@ -132,20 +132,7 @@ register_prepared_queries(Opts) ->
                              "WHERE user_id = ?">>),
 
     %% Domain Removal
-    {MaybeLimitSQL, MaybeLimitMSSQL} = mod_mam_utils:batch_delete_limits(Opts),
-    IdTable = <<"(SELECT ", MaybeLimitMSSQL/binary,
-                " id from mam_server_user WHERE server = ? ", MaybeLimitSQL/binary, ")">>,
-    ServerTable = <<"(SELECT * FROM (SELECT", MaybeLimitMSSQL/binary,
-                    " server FROM mam_server_user WHERE server = ? ", MaybeLimitSQL/binary, ") as t)">>,
-    mongoose_rdbms:prepare(mam_remove_domain, mam_message, ['mam_server_user.server'],
-                           <<"DELETE FROM mam_message "
-                             "WHERE user_id IN ", IdTable/binary>>),
-    mongoose_rdbms:prepare(mam_remove_domain_prefs, mam_config, ['mam_server_user.server'],
-                           <<"DELETE FROM mam_config "
-                             "WHERE user_id IN ", IdTable/binary>>),
-    mongoose_rdbms:prepare(mam_remove_domain_users, mam_server_user, [server],
-                           <<"DELETE ", MaybeLimitMSSQL/binary,
-                             " FROM mam_server_user WHERE server IN ", ServerTable/binary>>),
+    prepare_remove_domain(Opts),
 
     mongoose_rdbms:prepare(mam_make_tombstone, mam_message, [message, user_id, id],
                            <<"UPDATE mam_message SET message = ?, search_body = '' "
@@ -166,6 +153,42 @@ register_prepared_queries(Opts) ->
                              " AND id = ? AND direction = ?"
                              " ORDER BY id DESC ", LimitSQL/binary>>).
 
+prepare_remove_domain(#{delete_domain_limit := infinity}) ->
+    mongoose_rdbms:prepare(mam_remove_domain, mam_message, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_message "
+                             "WHERE user_id IN "
+                             "(SELECT id from mam_server_user WHERE server = ?)">>),
+    mongoose_rdbms:prepare(mam_remove_domain_prefs, mam_config, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_config "
+                             "WHERE user_id IN "
+                             "(SELECT id from mam_server_user WHERE server = ?)">>),
+    mongoose_rdbms:prepare(mam_remove_domain_users, mam_server_user, [server],
+                           <<"DELETE FROM mam_server_user WHERE server = ?">>);
+prepare_remove_domain(#{delete_domain_limit := Limit}) ->
+    LimitSQL = case mongoose_rdbms:db_type() of
+                        mssql -> throw(delete_domain_limit_not_supported_for_mssql);
+                        _ -> {MaybeLimitSQL, _} = rdbms_queries:get_db_specific_limits_binaries(Limit),
+                             MaybeLimitSQL
+                    end,
+    IdTable = <<"(SELECT * FROM ",
+                    "(SELECT msg.user_id, msg.id FROM mam_message msg",
+                    " INNER JOIN mam_server_user msu ON msu.id=msg.user_id",
+                    " WHERE msu.server = ? ", LimitSQL/binary, ") AS T)">>,
+    mongoose_rdbms:prepare(mam_incr_remove_domain, mam_message, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_message WHERE (user_id, id) IN ", IdTable/binary>>),
+    CfgTable = <<"(SELECT * FROM ",
+                     "(SELECT cfg.user_id, cfg.remote_jid FROM mam_config cfg",
+                     " INNER JOIN mam_server_user msu ON msu.id=cfg.user_id",
+                     " WHERE msu.server = ? ", LimitSQL/binary, ") AS T)">>,
+    mongoose_rdbms:prepare(mam_incr_remove_domain_prefs, mam_config, ['mam_server_user.server'],
+                           <<"DELETE FROM mam_config "
+                             "WHERE (user_id, remote_jid) IN ", CfgTable/binary>>),
+    ServerTable = <<"(SELECT * FROM ",
+                    "(SELECT id FROM mam_server_user WHERE server = ? ", LimitSQL/binary, ") as t)">>,
+    mongoose_rdbms:prepare(mam_incr_remove_domain_users, mam_server_user, [server],
+                           <<"DELETE FROM mam_server_user WHERE id IN ", ServerTable/binary>>).
+
+
 %% ----------------------------------------------------------------------
 %% Declarative logic
 
@@ -370,7 +393,7 @@ remove_domain_all(HostType, Domain) ->
 
 -spec remove_domain_batch(host_type(), jid:lserver(), non_neg_integer()) -> any().
 remove_domain_batch(HostType, Domain, Limit) ->
-    DeleteQueries = [mam_remove_domain, mam_remove_domain_prefs, mam_remove_domain_users],
+    DeleteQueries = [mam_incr_remove_domain, mam_incr_remove_domain_prefs, mam_incr_remove_domain_users],
     TotalDeleted = mod_mam_utils:incremental_delete_domain(HostType, Domain, Limit, DeleteQueries, 0),
     ?LOG_INFO(#{what => mam_domain_removal_completed, total_records_deleted => TotalDeleted,
                 domain => Domain, host_type => HostType}).

diff --git a/src/mam/mod_mam_utils.erl b/src/mam/mod_mam_utils.erl
@@ -90,7 +90,7 @@
 %% Shared logic
 -export([check_result_for_policy_violation/2,
          lookup/3,
-         batch_delete_limits/1, incremental_delete_domain/5,
+         incremental_delete_domain/5,
          db_message_codec/2, db_jid_codec/2]).
 
 -callback extra_fin_element(mongooseim:host_type(),
@@ -1226,13 +1226,6 @@ 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(), _ => _}) ->
-    {binary(), binary()}.
-batch_delete_limits(#{delete_domain_limit := infinity}) ->
-    {<<>>, <<>>};
-batch_delete_limits(#{delete_domain_limit := Limit}) ->
-    rdbms_queries:get_db_specific_limits_binaries(Limit).
-
 -spec incremental_delete_domain(
         mongooseim:host_type(), jid:lserver(), non_neg_integer(), [atom()], non_neg_integer()) ->
     non_neg_integer().