[core] Post _on_completed callbacks to io_service_ instead of running them synchronously
#39112
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
Signed-off-by: Edward Oakes <[email protected]>
Signed-off-by: Edward Oakes <[email protected]>
|
Kicked off release tests for core and serve: https://buildkite.com/ray-project/release-tests-pr/builds/51486 |
8 tasks
…-asyncio-callback-to-loop
…-asyncio-callback-to-loop
|
Test test seems pretty promising |
|
previous release tests are here; https://buildkite.com/ray-project/release-tests-pr/builds/51486 (note: I didn't finish some of long running tests to run core tests) |
|
@edoakes can you take a look at failures from https://buildkite.com/ray-project/release-tests-pr/builds/51486#018a485f-94f3-4afa-96a5-3f3ea7cab789? (from logs its failures are pretty mysterious) |
|
Rerunning it with core https://buildkite.com/ray-project/release-tests-pr/builds/51571. I hope the wheel is correct this time... |
|
Test result so far seems pretty promising |
rkooo567
approved these changes
Aug 31, 2023
_on_completed callbacks to io_service_ instead of running them synchronously_on_completed callbacks to io_service_ instead of running them synchronously
Signed-off-by: Edward Oakes <[email protected]>
|
@rkooo567 there are a lot of failures that have tracebacks like this: Looks sort of suspicious (some others had this raised in |
|
Same tracebacks are happening on the release branch, so I'm not concerned about them: https://buildkite.com/ray-project/release-tests-branch/builds/2103#018a44b2-6415-495b-8823-7824101415f8 Note that this test also forcibly kills replicas constantly, so seeing some tracebacks is expected. |
|
All tests also passed on the serve cancellation PR with this change merged in: https://buildkite.com/ray-project/oss-ci-build-pr/builds/34426 |
edoakes
added a commit
to edoakes/ray
that referenced
this pull request
Aug 31, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager).
8 tasks
edoakes
added a commit
to edoakes/ray
that referenced
this pull request
Aug 31, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager). Signed-off-by: Edward Oakes <[email protected]>
GeneDer
pushed a commit
that referenced
this pull request
Sep 1, 2023
* [core] Decrement `ObjectRef._on_completed` callback when callback errors (#38673) We manually increment and decrement the Python refcount for the callback to avoid Python GC'ing it while it's stored in the core worker. However, the decrement logic is skipped if the callback raises an exception. This change adds a `try: finally` block to always decrement the ref count. The added test case fails without the change and passes w/ it. Signed-off-by: Edward Oakes <[email protected]> * [core] Post `_on_completed` callbacks to `io_service_` instead of running them synchronously (#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager). Signed-off-by: Edward Oakes <[email protected]> --------- Signed-off-by: Edward Oakes <[email protected]>
LeonLuttenberger
pushed a commit
to jaidisido/ray
that referenced
this pull request
Sep 5, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager).
harborn
pushed a commit
to harborn/ray
that referenced
this pull request
Sep 8, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager).
jimthompson5802
pushed a commit
to jimthompson5802/ray
that referenced
this pull request
Sep 12, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager). Signed-off-by: Jim Thompson <[email protected]>
vymao
pushed a commit
to vymao/ray
that referenced
this pull request
Oct 11, 2023
…ning them synchronously (ray-project#39112) Fixes the deadlock described in the linked issue by posting callbacks to the `io_service_`. This avoids running them while holding locks (such as from the task manager). Signed-off-by: Victor <[email protected]>
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?
Attempts to fix the deadlock described in the linked issue by posting callbacks to the
io_service_. This avoids running them while holding locks (such as from the task manager).Related issue number
#39113 (will be closed once cherry-picked).
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.