Skip to content

Conversation

@andybalaam
Copy link
Member

@andybalaam andybalaam commented Mar 22, 2024

NOT READY FOR REVIEW. Marked as such to encourage GH to run the relevant job.

This is just a test to see whether the errors we were seeing here: https://github.com/matrix-org/matrix-rust-sdk/actions/runs/8388525873/job/22972906570?pr=3252#step:9:1818 are fixed by setting pool_max_idle_per_host to 0 on the hyper client.

It's a total guess, but here are the issues I found that suggest it might help:

@andybalaam andybalaam changed the title crypto: Formatting for integration_tests client: Set pool_max_idle_per_host to 0 on hyper::Client Mar 22, 2024
@andybalaam andybalaam marked this pull request as ready for review March 22, 2024 16:24
@andybalaam andybalaam requested a review from a team as a code owner March 22, 2024 16:24
@andybalaam andybalaam requested review from bnjbvr and removed request for a team March 22, 2024 16:24
@andybalaam andybalaam changed the title client: Set pool_max_idle_per_host to 0 on hyper::Client WIP: client: Set pool_max_idle_per_host to 0 on hyper::Client Mar 22, 2024
@andybalaam andybalaam removed the request for review from bnjbvr March 22, 2024 16:25
@codecov
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.59%. Comparing base (97959bb) to head (d7466e3).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3269      +/-   ##
==========================================
- Coverage   83.63%   83.59%   -0.05%     
==========================================
  Files         239      238       -1     
  Lines       24686    24575     -111     
==========================================
- Hits        20647    20543     -104     
+ Misses       4039     4032       -7     

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

@bnjbvr
Copy link
Member

bnjbvr commented Mar 26, 2024

Thanks for digging into it. I think the runtime dropped was a sign that another task timed out; so it's really the reporting that was misleading here, and as long as we all collectively remember that a timeout elsewhere in the test may cause this "dropped runtime blah blah" errors, we should be fine.

In the meanwhile, I suppose we can avoid this, and revive it if we figure out with utter confidence that it will fix the testing issue we were observing.

@bnjbvr bnjbvr closed this Mar 26, 2024
@andybalaam
Copy link
Member Author

Great, thank you for figuring this out.

@poljar poljar deleted the andybalaam/work-around-hyper-2312 branch November 5, 2024 11:41
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