Fix query that creates inbox and mam delete domain statement #3924

NelsonVides · 2023-01-09T16:00:17Z

This was passing because both with and without increments, the query created was under the same name, so then in tests the non-incremental test was running first and creating the prepared query that would remove everything. So then when the group with incremental delete was running, it would fail to create the right query but it would reuse the old one. In production though, the query created would fail because it is not valid SQL and we wouldn't know that until after the query is ran.

By ensuring that the used query has a different name, we can at least be sure we are not actually just using the plain one.

Unfortunately, I don't know of a way to make this work for mssql easily, syntax is entirely different. Already the current select * over a select is a compromise between pgsql and mysql.

codecov · 2023-01-09T16:10:45Z

Codecov Report

Base: 83.15% // Head: 83.13% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (e658b06) compared to base (759b64a).
Patch coverage: 61.76% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3924      +/-   ##
==========================================
- Coverage   83.15%   83.13%   -0.03%     
==========================================
  Files         535      535              
  Lines       34110    34127      +17     
==========================================
+ Hits        28364    28371       +7     
- Misses       5746     5756      +10

Impacted Files	Coverage Δ
src/mam/mod_mam_utils.erl	`89.87% <ø> (-0.07%)`	⬇️
src/mam/mod_mam_rdbms_arch.erl	`41.17% <26.66%> (-4.77%)`	⬇️
src/inbox/mod_inbox_rdbms.erl	`92.02% <85.71%> (-0.33%)`	⬇️
src/mam/mod_mam_muc_rdbms_arch.erl	`95.04% <91.66%> (-0.62%)`	⬇️
src/async_pools/mongoose_aggregator_worker.erl	`63.33% <0.00%> (-5.01%)`	⬇️
src/inbox/mod_inbox_rdbms_async.erl	`72.05% <0.00%> (-1.48%)`	⬇️
src/ejabberd_c2s.erl	`88.95% <0.00%> (-0.08%)`	⬇️
src/mod_muc_log.erl	`62.82% <0.00%> (ø)`
src/pubsub/mod_pubsub_db_rdbms.erl	`95.34% <0.00%> (+0.25%)`	⬆️
src/mod_roster.erl	`79.37% <0.00%> (+0.47%)`	⬆️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

mongoose-im · 2023-01-10T14:58:44Z

small_tests_24 / small_tests / 4ab9ef1
Reports root / small

small_tests_25 / small_tests / 4ab9ef1
Reports root / small

ldap_mnesia_24 / ldap_mnesia / 4ab9ef1
Reports root/ big
OK: 2223 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 4ab9ef1
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / 4ab9ef1
Reports root/ big
OK: 2223 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / 4ab9ef1
Reports root/ big
OK: 4151 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 4ab9ef1
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / 4ab9ef1
Reports root/ big
OK: 2365 / Failed: 0 / User-skipped: 681 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 4ab9ef1
Reports root/ big
OK: 2723 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 4ab9ef1
Reports root/ big
OK: 4174 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / 4ab9ef1
Reports root/ big
OK: 4551 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / 4ab9ef1
Reports root/ big
OK: 2561 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0

mysql_redis_25 / mysql_redis / 4ab9ef1
Reports root/ big
OK: 4537 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / 4ab9ef1
Reports root/ big
OK: 4551 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

mssql_mnesia_25 / odbc_mssql_mnesia / 4ab9ef1
Reports root/ big
OK: 4548 / Failed: 0 / User-skipped: 100 / Auto-skipped: 0

mongoose-im · 2023-01-10T15:06:41Z

small_tests_24 / small_tests / e658b06
Reports root / small

small_tests_25 / small_tests / e658b06
Reports root / small

ldap_mnesia_24 / ldap_mnesia / e658b06
Reports root/ big
OK: 2223 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / e658b06
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

ldap_mnesia_25 / ldap_mnesia / e658b06
Reports root/ big
OK: 2223 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0

dynamic_domains_mysql_redis_25 / mysql_redis / e658b06
Reports root/ big
OK: 4151 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0

dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / e658b06
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0

internal_mnesia_25 / internal_mnesia / e658b06
Reports root/ big
OK: 2365 / Failed: 0 / User-skipped: 681 / Auto-skipped: 0

pgsql_mnesia_24 / pgsql_mnesia / e658b06
Reports root/ big
OK: 4551 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0

dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / e658b06
Reports root/ big
OK: 4174 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0

elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / e658b06
Reports root/ big
OK: 2723 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0

riak_mnesia_24 / riak_mnesia / e658b06
Reports root/ big
OK: 2561 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0

mysql_redis_25 / mysql_redis / e658b06
Reports root/ big
OK: 4537 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0

pgsql_mnesia_25 / pgsql_mnesia / e658b06
Reports root/ big
OK: 4562 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription

{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_2699@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_2699@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"ZFHie0+jf5M9zTtWHz8E4g==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log

mssql_mnesia_25 / odbc_mssql_mnesia / e658b06
Reports root/ big
OK: 4570 / Failed: 2 / User-skipped: 100 / Auto-skipped: 0

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription

{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_2702@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_2702@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"emhZVbBEsuZASjAEAsVkmw==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription

{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_2713@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_2713@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"yRFECnRdX4+4JeseA9o6Mw==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {escalus_story,story,4,
      [{file,
         "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
       {line,72}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log

mssql_mnesia_25 / odbc_mssql_mnesia / e658b06
Reports root/ big
OK: 4545 / Failed: 3 / User-skipped: 100 / Auto-skipped: 0

inbox_extensions_SUITE:regular:one_to_one:pagination:pagination_overrides_form

{error,
  {timeout_when_waiting_for_stanza,
    [{escalus_client,wait_for_stanza,
       [{client,
          <<"mike_pagination_overrides_form_1580@localhost/res1">>,
          escalus_tcp,<0.11214.1>,
          [{event_manager,<0.11162.1>},
           {server,<<"localhost">>},
           {username,<<"mike_pagination_overrides_form_1580">>},
           {resource,<<"res1">>}],
          [{event_client,
             [{event_manager,<0.11162.1>},
            {server,<<"localhost">>},
            {username,<<"mike_pagination_overrides_form_1580">>},
            {resource,<<"res1">>}]},
           {resource,<<"res1">>},
           {username,<<"mike_pagination_overrides_form_1580">>},
           {server,<<"localhost">>},
           {host,<<"localhost">>},
           {port,5222},
           {auth,{escalus_auth,auth_plain}},
           {wspath,undefined},
           {username,<<"mike_pagination_overrides_form_1580">>},
           {server,<<"localhost">>},
           {password,<<"nicniema">>},
           {stream_id,<<"eeb8ede2371cf584">>}]},
        5000],
       [{file,
          "/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_client.erl"},
        {line,136}]},
     {inbox_helper,'-given_conversations_between/2-fun-1-',4,
       [{file,"/home/circleci/project/big_tests/tests/inbox_helper.erl"},
        {line,543}]},
     {lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
     {inbox_extensions_SUITE,'-pagination_overrides_form/1-fun-0-',4,
       [{file,
  ...

Report log

inbox_extensions_SUITE:regular:one_to_one:pagination:max_queries_can_be_limited

{error,
 {{inbox_size,3,
   [{times,1,
   {error,
    #{check_params => #{},error => function_clause,
    expected_items =>
     [#{content => <<"Msg 3">>,
      from => <<"alice_max_queries_can_be_limited_1583@localhost/res1">>,
      to => <<"mike_max_queries_can_be_limited_1583@localhost/res1">>,
      unread => 0,verify => #Fun<inbox_helper.28.35890797>},
      #{content => <<"Msg 2">>,
      from => <<"alice_max_queries_can_be_limited_1583@localhost/res1">>,
      to => <<"kate_max_queries_can_be_limited_1583@localhost/res1">>,
      unread => 0,verify => #Fun<inbox_helper.28.35890797>},
      #{content => <<"Msg 1">>,
      from => <<"alice_max_queries_can_be_limited_1583@localhost/res1">>,
      to => <<"bob_max_queries_can_be_limited_1583@localhost/res1">>,
      unread => 0,verify => #Fun<inbox_helper.28.35890797>}],
    inbox_items =>
     [<<"<message from='alice_max_queries_can_be_limited_1583@localhost' to='alice_max_queries_can_be_limited_1583@localhost/res1' id='1673-365176-392063'><result xmlns='erlang-solutions.com:xmpp:inbox:0' unread='0' queryid='6fa8dfc8cd3c25119176deea64ceb691'><forwarded xmlns='urn:xmpp:forward:0'><delay xmlns='urn:xmpp:delay' stamp='2023-01-10T15:39:31.422027Z'/><message from='alice_max_queries_can_be_limited_1583@localhost/res1' xml:lang='en' to='mike_max_queries_can_be_limited_1583@localhost/res1' type='chat' xmlns='jabber:client'><body>Msg 3</body></message></forwarded><read>true</read><box>inbox</box><archive>false</archive><mute>0<...

Report log

inbox_extensions_SUITE:regular:one_to_one:pagination:max_queries_can_fetch_ahead

{error,
 {{inbox_size,2,
   [{times,1,
   {error,
    #{check_params => #{},error => function_clause,
    expected_items =>
     [#{content => <<"Msg 2">>,
      from =>
       <<"alice_max_queries_can_fetch_ahead_1585@localhost/res1">>,
      to => <<"kate_max_queries_can_fetch_ahead_1585@localhost/res1">>,
      unread => 0,verify => #Fun<inbox_helper.28.35890797>},
      #{content => <<"Msg 1">>,
      from =>
       <<"alice_max_queries_can_fetch_ahead_1585@localhost/res1">>,
      to => <<"bob_max_queries_can_fetch_ahead_1585@localhost/res1">>,
      unread => 0,verify => #Fun<inbox_helper.28.35890797>}],
    inbox_items =>
     [<<"<message from='alice_max_queries_can_fetch_ahead_1585@localhost' to='alice_max_queries_can_fetch_ahead_1585@localhost/res1' id='1673-365176-402419'><result xmlns='erlang-solutions.com:xmpp:inbox:0' unread='0' queryid='5ba295857cbe86c48613f5de0e0510de'><forwarded xmlns='urn:xmpp:forward:0'><delay xmlns='urn:xmpp:delay' stamp='2023-01-10T15:39:27.005185Z'/><message from='alice_max_queries_can_fetch_ahead_1585@localhost/res1' xml:lang='en' to='bob_max_queries_can_fetch_ahead_1585@localhost/res1' type='chat' xmlns='jabber:client'><body>Msg 1</body></message></forwarded><read>true</read><box>inbox</box><archive>false</archive><mute>0</mute></result></message>">>],
    query_params =>
     #{box => inbox,'end' => <<"2023-01-10T15:39:36.392449Z">>,limit => 2},
    reason => inbox_mismatch,
    stacktrace =>
     [{lists,zip,
       [[],
      [{conv...

Report log

mssql_mnesia_25 / odbc_mssql_mnesia / e658b06
Reports root/ big
OK: 4548 / Failed: 0 / User-skipped: 100 / Auto-skipped: 0

chrzaszcz

Looks good 👍 It would be good to add tests for MAM incremental deletion, it's just a bit sad to see the coverage go down.

NelsonVides · 2023-01-11T08:26:51Z

Looks good 👍 It would be good to add tests for MAM incremental deletion, it's just a bit sad to see the coverage go down.

Oh but there are tests for it, I don't know why here codecov reports bad coverage but locally my tests report to hit that path correctly 🤷🏽

MongooseIM/big_tests/tests/domain_removal_SUITE.erl

Line 40 in e658b06

{mam_removal_incremental, [], [mam_pm_removal,

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 95dc3b2 to 414a617 Compare January 9, 2023 16:20

NelsonVides changed the title ~~Fix query that creates inbox delete domain statement~~ Fix query that creates inbox and mam delete domain statement Jan 9, 2023

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 414a617 to 3aab37f Compare January 9, 2023 16:49

This comment was marked as outdated.

Sign in to view

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 3aab37f to efe3671 Compare January 9, 2023 17:39

This comment was marked as outdated.

Sign in to view

Fix query that creates inbox delete domain statements

11b9377

NelsonVides force-pushed the inbox/incremental_domain_removal branch from ea68968 to 9feb307 Compare January 9, 2023 23:03

NelsonVides added 2 commits January 10, 2023 00:07

Fix query that creates mam muc delete domain statements

bd4708d

Fix query that creates mam pm delete domain statements

68aeff8

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 9feb307 to 68aeff8 Compare January 9, 2023 23:07

This comment was marked as outdated.

Sign in to view

Incremental removal is not really supported for mssql

42a246d

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 7381c56 to 42a246d Compare January 10, 2023 09:14

This comment was marked as outdated.

Sign in to view

NelsonVides marked this pull request as ready for review January 10, 2023 12:03

Fail to start on MSSQL with delete_domain_limit

e658b06

NelsonVides force-pushed the inbox/incremental_domain_removal branch from 4ab9ef1 to e658b06 Compare January 10, 2023 14:58

chrzaszcz approved these changes Jan 11, 2023

View reviewed changes

chrzaszcz merged commit 8275095 into master Jan 11, 2023

chrzaszcz deleted the inbox/incremental_domain_removal branch January 11, 2023 08:51

jacekwegr added this to the 6.1.0 milestone Apr 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix query that creates inbox and mam delete domain statement #3924

Fix query that creates inbox and mam delete domain statement #3924

NelsonVides commented Jan 9, 2023 •

edited

Loading

This comment was marked as outdated.

codecov bot commented Jan 9, 2023 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Jan 10, 2023 •

edited

Loading

mongoose-im commented Jan 10, 2023 •

edited

Loading

chrzaszcz left a comment

NelsonVides commented Jan 11, 2023 •

edited

Loading

Fix query that creates inbox and mam delete domain statement #3924

Fix query that creates inbox and mam delete domain statement #3924

Conversation

NelsonVides commented Jan 9, 2023 • edited Loading

This comment was marked as outdated.

codecov bot commented Jan 9, 2023 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

mongoose-im commented Jan 10, 2023 • edited Loading

mongoose-im commented Jan 10, 2023 • edited Loading

chrzaszcz left a comment

Choose a reason for hiding this comment

NelsonVides commented Jan 11, 2023 • edited Loading

NelsonVides commented Jan 9, 2023 •

edited

Loading

codecov bot commented Jan 9, 2023 •

edited

Loading

mongoose-im commented Jan 10, 2023 •

edited

Loading

mongoose-im commented Jan 10, 2023 •

edited

Loading

NelsonVides commented Jan 11, 2023 •

edited

Loading