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 deadlock caused by non-responsive ZK servers #80

Merged
merged 2 commits into from
Mar 23, 2022

Conversation

tobio
Copy link

@tobio tobio commented Mar 21, 2022

Fixes #79

Instead of blindly waiting for a response from queueRequest, wait for a message on either the requests response channel or an indication that the client shouldQuit. This ensures that any ongoing requests are completed when a client is closed.

Copy link

@nemith nemith left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@pmazzini pmazzini left a comment

Choose a reason for hiding this comment

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

Tests are stuck / not working

@tobio
Copy link
Author

tobio commented Mar 23, 2022

@pmazzini thanks for taking a look.

The latest push has fixed the test workflow in my fork, are you able to approve the checks against this PR?

@pmazzini pmazzini dismissed their stale review March 23, 2022 21:06

request review

@codecov
Copy link

codecov bot commented Mar 23, 2022

Codecov Report

Merging #80 (5e66dae) into master (21b4929) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
- Coverage   76.86%   76.78%   -0.09%     
==========================================
  Files           7        7              
  Lines        1193     1193              
==========================================
- Hits          917      916       -1     
- Misses        188      189       +1     
  Partials       88       88              
Impacted Files Coverage Δ
conn.go 74.37% <100.00%> (-0.16%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 21b4929...5e66dae. Read the comment docs.

@nemith nemith merged commit abd6db4 into go-zookeeper:master Mar 23, 2022
@tobio tobio deleted the request-deadlock branch March 24, 2022 00:17
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.

Connection deadlock when connecting to unresponsive ZK ensemble.
3 participants