Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix slow MAM muc_prefs_cases tests #4286

Merged
merged 1 commit into from
May 22, 2024

Conversation

gustawlippa
Copy link
Contributor

@gustawlippa gustawlippa commented May 22, 2024

This PR fixes three testcase added recently. I'm having issues with this case in my other PR, and noticed that it's surprisingly slow.
Time before fix:
obraz
Time after fix:
obraz

Explanation:
Since the room is reused across muc_run_prefs_case calls, there is no need to resend presence every time - the users are room participants all the time. The issue is that when (re)entering, Alice would wait for 4 stanzas - 3 presences and 1 room subject message. However, only in the beginning, as a new user would she receive the room subject - subsequent waits for 4 stanzas failed silently after only 3 presences, but only after waiting the 5s timeout. This has happend 6 times per testcase, and there were 3 tescases using this mechanism, so there is 3 * 6 * 5=90s less pointless waiting now.

I think maybe Escalus should fail loudly when less then N stanzas arrive when waiting for N stanzas.

This has a huge impact on the testing times for these cases.

Since the room is reused across `muc_run_prefs_case` calls, there is no need to
resend presence every time - the users are room participants all the time. The
issue is that when (re)entering, Alice would wait for 4 stanzas - 3 presences
and 1 room subject message. However, only in the beginning, as a new user would
she receive the room subject - subsequent waits for 4 stanzas failed silently
after only 3 presences, but only after waiting the 5s timeout. This has happend
6 times per testcase, and there were 3 tescases using this mechanism, so there
is 3*6*5=90s less pointless waiting now.
@gustawlippa gustawlippa changed the title Send initial presence to room only once Fix slow MAM muc_prefs_cases tests May 22, 2024
@mongoose-im
Copy link
Collaborator

mongoose-im commented May 22, 2024

elasticsearch_and_cassandra_26 / elasticsearch_and_cassandra_mnesia / 41863b7
Reports root/ big
OK: 457 / Failed: 0 / User-skipped: 41 / Auto-skipped: 0


small_tests_25 / small_tests / 41863b7
Reports root / small


small_tests_26 / small_tests / 41863b7
Reports root / small


small_tests_26_arm64 / small_tests / 41863b7
Reports root / small


ldap_mnesia_25 / ldap_mnesia / 41863b7
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


dynamic_domains_mysql_redis_26 / mysql_redis / 41863b7
Reports root/ big
OK: 4589 / Failed: 0 / User-skipped: 138 / Auto-skipped: 0


ldap_mnesia_26 / ldap_mnesia / 41863b7
Reports root/ big
OK: 2288 / Failed: 0 / User-skipped: 905 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 41863b7
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_26 / pgsql_mnesia / 41863b7
Reports root/ big
OK: 4622 / Failed: 0 / User-skipped: 105 / Auto-skipped: 0


pgsql_cets_26 / pgsql_cets / 41863b7
Reports root/ big
OK: 4517 / Failed: 0 / User-skipped: 174 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_26 / odbc_mssql_mnesia / 41863b7
Reports root/ big
OK: 4619 / Failed: 0 / User-skipped: 108 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 41863b7
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


internal_mnesia_26 / internal_mnesia / 41863b7
Reports root/ big
OK: 2428 / Failed: 0 / User-skipped: 765 / Auto-skipped: 0


pgsql_mnesia_26 / pgsql_mnesia / 41863b7
Reports root/ big
OK: 5011 / Failed: 0 / User-skipped: 112 / Auto-skipped: 0


mysql_redis_26 / mysql_redis / 41863b7
Reports root/ big
OK: 4990 / Failed: 0 / User-skipped: 133 / Auto-skipped: 0


mssql_mnesia_26 / odbc_mssql_mnesia / 41863b7
Reports root/ big
OK: 5008 / Failed: 0 / User-skipped: 115 / Auto-skipped: 0

Copy link

codecov bot commented May 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.67%. Comparing base (cd92518) to head (41863b7).

Additional details and impacted files
@@                  Coverage Diff                   @@
##           feature/instrument    #4286      +/-   ##
======================================================
- Coverage               84.70%   84.67%   -0.04%     
======================================================
  Files                     556      556              
  Lines                   33890    33890              
======================================================
- Hits                    28708    28697      -11     
- Misses                   5182     5193      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gustawlippa gustawlippa marked this pull request as ready for review May 22, 2024 11:14
Copy link
Contributor

@jacekwegr jacekwegr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the fix!

@jacekwegr jacekwegr merged commit 02359b1 into feature/instrument May 22, 2024
4 checks passed
@jacekwegr jacekwegr deleted the instrument/fix_slow_mam_test branch May 22, 2024 12:05
@jacekwegr jacekwegr added this to the 6.3.0 milestone Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants