Skip to content

tcp: free up unsuccessful connection attempts#165

Merged
rcschrg merged 1 commit intodevelopmentfrom
tcp_connection_count
Apr 8, 2025
Merged

tcp: free up unsuccessful connection attempts#165
rcschrg merged 1 commit intodevelopmentfrom
tcp_connection_count

Conversation

@maurerle
Copy link
Copy Markdown
Collaborator

@maurerle maurerle commented Apr 8, 2025

unsuccessful connection attempts are never freed from connection count

if a connection attempt throws an error in create_connection, we do not decrease the counter in _connection_counts, as the list stays empty. Therefore, maximum 10 connection attempts are made if the other container is not ready yet.

This was the case when using the wait_all_online from the DistributedClockManager.
The manager did send 10 requests to check if all respond, but stops afterwards, as all connection_counts are allocated.

This is fixed by defering the increment of the connection count right after we successfully created a connection.
A similar solution would be to decrease it again on failure, which would decrease the probability of creating too many connections, while the last connection is created - though I would go for less lines of code on this one.

Fix this by decrementing the counter if an error is thrown

@maurerle maurerle requested a review from rcschrg April 8, 2025 14:08
… count

if a connection attempt throws an error in `create_connection`, we do not decrease the counter in _connection_counts. Therefore, maximum 10 connection attempts are made if the other container is not ready yet
This is fixed by decreasing if an error is thrown during create_connection
@maurerle maurerle force-pushed the tcp_connection_count branch from d0e2527 to 6326813 Compare April 8, 2025 14:23
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 8, 2025

Codecov Report

Attention: Patch coverage is 40.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 89.13%. Comparing base (dd5b074) to head (6326813).
Report is 4 commits behind head on development.

Files with missing lines Patch % Lines
mango/container/tcp.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           development     #165      +/-   ##
===============================================
- Coverage        89.23%   89.13%   -0.10%     
===============================================
  Files               22       22              
  Lines             2507     2512       +5     
===============================================
+ Hits              2237     2239       +2     
- Misses             270      273       +3     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rcschrg rcschrg merged commit b91ae35 into development Apr 8, 2025
8 of 10 checks passed
@maurerle maurerle deleted the tcp_connection_count branch April 8, 2025 17:16
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.

2 participants