[core][accelerated DAGs] Make DAG teardown blocking and fix bug during close #45099
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: Your Name <[email protected]>
Signed-off-by: Your Name <[email protected]>
Signed-off-by: Stephanie Wang <[email protected]>
Signed-off-by: Stephanie Wang <[email protected]>
rkooo567
approved these changes
May 2, 2024
| self.teardown() | ||
| monitor = getattr(self, "_monitor", None) | ||
| if monitor is not None: | ||
| # Teardown asynchronously. |
There was a problem hiding this comment.
I think it is because del call ordering is not guaranteed when python interpreter exits
| if self.in_teardown: | ||
| if wait: |
There was a problem hiding this comment.
Do we need this? Since we are already in teardown, isn't there supposed to be other caller that is waiting?
There was a problem hiding this comment.
It's to make sure that explicitly calling .teardown() will always wait until the DAG is torn down.
Signed-off-by: Stephanie Wang <[email protected]>
shaikhismail
pushed a commit
to shaikhismail/ray
that referenced
this pull request
May 4, 2024
…g close (ray-project#45099) ## Why are these changes needed? Contains a few fixes related to DAG teardown: - Removes an unnecessary `.close()` call that would error if the DAG has a single output (instead of a MultiOutputNode) - Makes `dag.teardown()` blocking to ensure that actors can be reused after the teardown call returns. - Makes DAG teardown in `__del__` asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down before `dag.teardown()` is complete. --------- Signed-off-by: Stephanie Wang <[email protected]>
harborn
pushed a commit
to harborn/ray
that referenced
this pull request
May 8, 2024
…g close (ray-project#45099) ## Why are these changes needed? Contains a few fixes related to DAG teardown: - Removes an unnecessary `.close()` call that would error if the DAG has a single output (instead of a MultiOutputNode) - Makes `dag.teardown()` blocking to ensure that actors can be reused after the teardown call returns. - Makes DAG teardown in `__del__` asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down before `dag.teardown()` is complete. --------- Signed-off-by: Stephanie Wang <[email protected]>
peytondmurray
pushed a commit
to peytondmurray/ray
that referenced
this pull request
May 13, 2024
…g close (ray-project#45099) ## Why are these changes needed? Contains a few fixes related to DAG teardown: - Removes an unnecessary `.close()` call that would error if the DAG has a single output (instead of a MultiOutputNode) - Makes `dag.teardown()` blocking to ensure that actors can be reused after the teardown call returns. - Makes DAG teardown in `__del__` asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down before `dag.teardown()` is complete. --------- Signed-off-by: Stephanie Wang <[email protected]>
ryanaoleary
pushed a commit
to ryanaoleary/ray
that referenced
this pull request
Jun 6, 2024
…g close (ray-project#45099) ## Why are these changes needed? Contains a few fixes related to DAG teardown: - Removes an unnecessary `.close()` call that would error if the DAG has a single output (instead of a MultiOutputNode) - Makes `dag.teardown()` blocking to ensure that actors can be reused after the teardown call returns. - Makes DAG teardown in `__del__` asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down before `dag.teardown()` is complete. --------- Signed-off-by: Stephanie Wang <[email protected]> Signed-off-by: Ryan O'Leary <[email protected]>
ryanaoleary
pushed a commit
to ryanaoleary/ray
that referenced
this pull request
Jun 7, 2024
…g close (ray-project#45099) ## Why are these changes needed? Contains a few fixes related to DAG teardown: - Removes an unnecessary `.close()` call that would error if the DAG has a single output (instead of a MultiOutputNode) - Makes `dag.teardown()` blocking to ensure that actors can be reused after the teardown call returns. - Makes DAG teardown in `__del__` asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down before `dag.teardown()` is complete. --------- Signed-off-by: Stephanie Wang <[email protected]>
8 tasks
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?
Contains a few fixes related to DAG teardown:
.close()call that would error if the DAG has a single output (instead of a MultiOutputNode)dag.teardown()blocking to ensure that actors can be reused after the teardown call returns.__del__asynchronous. if synchronous, this can hang the driver upon shutdown. I'm not exactly sure why but I believe this happens if the CoreWorker is shut down beforedag.teardown()is complete.