[Client] Disconnect on dataclient error #16588
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
ckw017
force-pushed
the
disconnect_on_dataclient_error
branch
2 times, most recently
from
3f57979
to
502d0ac
Compare
ijrsvt
reviewed
Jun 21, 2021
ckw017
force-pushed
the
disconnect_on_dataclient_error
branch
from
8817342
to
6f56625
Compare
ijrsvt
approved these changes
Jun 21, 2021
|
@DmitriGekhtman , if this is not too much work, it would be nice to include this PR as well in 1.4.1 |
ckw017
force-pushed
the
disconnect_on_dataclient_error
branch
from
b3bbec6
to
c26ab22
Compare
|
Rebased with master to fix failing java test, reviewers can merge at their discretion once everything is passing. |
sure, np -- any additional release testing required? |
No. Thanks! |
|
test_array, test_async are both known flakey on windows -- travis build failure is widespread |
DmitriGekhtman
pushed a commit
that referenced
this pull request
Jun 22, 2021
* disconnect when main thread finds dataclient shut down, update error messages * Add test_dataclient_disconnect to small tests * drop unused var * add __main__ section to test * avoid direct ray import * rerun
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.
Why are these changes needed?
When the data client's grpc channel disconnects (either for connection issues, or for a bad request) client is left in a state where it can't perform any remote functionality, but is still technically connected (client_ray.is_connected() is True).Since is_connected() still returns True, this causes errors if the user tries to start a new connection. This PR disconnects the client when the data client is found to be shut down by the main thread so that the user can cleanly try to reconnect.
Related issue number
Checks
scripts/format.shto lint the changes in this PR.