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

Cleanup connection reuse logic #885

Merged
merged 1 commit into from
Jul 17, 2022
Merged

Cleanup connection reuse logic #885

merged 1 commit into from
Jul 17, 2022

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Jul 17, 2022

Fixes #875. The problem in the original issue here was that if you tried
to do a websocket request, then very quickly tried another request,
the connection pool would try to reuse the same underlying Connection
object for the 2nd request, even while the 1st was still in the shutdown
and close sequence. This resulted in the 2nd connection having sporadic
behavior, usually in the form of an EOFError.

The fix here is cleaning up our ConnectionRequest logic around which
connections should be reused or closed prior to being released back
to the connection pool.

Fixes #875. The problem in the original issue here was that if you tried
to do a websocket request, then very quickly tried another request,
the connection pool would try to reuse the same underlying Connection
object for the 2nd request, even while the 1st was still in the shutdown
and close sequence. This resulted in the 2nd connection having sporadic
behavior, usually in the form of an EOFError.

The fix here is cleaning up our ConnectionRequest logic around which
connections should be reused or closed prior to being released back
to the connection pool.
@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2022

Codecov Report

Merging #885 (bb4bc2f) into master (cf706bc) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #885      +/-   ##
==========================================
+ Coverage   80.12%   80.22%   +0.09%     
==========================================
  Files          36       36              
  Lines        2878     2892      +14     
==========================================
+ Hits         2306     2320      +14     
  Misses        572      572              
Impacted Files Coverage Δ
src/ConnectionPool.jl 88.94% <100.00%> (ø)
src/HTTP.jl 86.48% <100.00%> (+3.43%) ⬆️
src/clientlayers/ConnectionRequest.jl 78.94% <100.00%> (+2.32%) ⬆️
src/clientlayers/RetryRequest.jl 74.41% <0.00%> (-2.33%) ⬇️
src/Messages.jl 87.03% <0.00%> (-0.62%) ⬇️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@quinnj quinnj merged commit f70a03d into master Jul 17, 2022
@quinnj quinnj deleted the jq/875 branch July 17, 2022 07:32
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.

Creating two WebSocket connections in sequence causes an EOFError
2 participants