Fix connection reaper deadlock. #150
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
January 10, 2024 11:27
Added explicitly running test to detect deadlock. Use ```go test -tags=TestConnReaperDeadlockLoop -run ^TestConnReaperDeadlockLoop$```. Be aware the test generate a lot of sockets in TIME_WAIT. Without patch to socket.go it breaks pretty quick on fast and slow machines. With the patch it runs way longer but gets slower when there are plenty of sockets in TIME_WAIT.
|
Dont get whats wrong with TestXPubSub - I ran it only in loop for almost 24h - no single failure |
sbinet
reviewed
Jan 16, 2024
Co-authored-by: Sebastien Binet <binet@cern.ch>
|
I will try to find way to make test with single pass to hit the deadlock. Please give me couple of days |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #150 +/- ##
==========================================
+ Coverage 68.19% 68.22% +0.03%
==========================================
Files 30 30
Lines 2600 2603 +3
==========================================
+ Hits 1773 1776 +3
Misses 724 724
Partials 103 103 ☔ View full report in Codecov by Sentry. |
The test TestConnReaperDeadlock2 designed after TestConnReaperDeadlock but uses internals to ensure problematic sequnence, which in case of TestConnReaperDeadlock requires number of runs.
|
I think the test is now ready but it might looks more ugly even can repro problem each run. |
sbinet
reviewed
Jan 22, 2024
Additional test simplification. Now it needs only two connections, where 1st fails asap and 2nd fails with delay - giving connection reaper time to meet deadlock
sbinet
approved these changes
Jan 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added explicitly running test to detect deadlock.
Use
go test -tags=TestConnReaperDeadlockLoop -run ^TestConnReaperDeadlockLoop$. Be aware the test generate a lot of sockets in TIME_WAIT. Without patch to socket.go it breaks pretty quick on fast and slow machines. With the patch it runs up to default test timeout (10m) but gets slower when there are plenty of sockets in TIME_WAIT.Fixes #149