[core] Dynamic generators that error return partial ObjectRefs followed by exception ObjectRef #28864
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
jjyao
approved these changes
Sep 28, 2022
| @@ -69,18 +69,28 @@ We can also pass the ``ObjectRef`` returned by a task with ``num_returns="dynami | |||
| :start-after: __dynamic_generator_pass_start__ | |||
| :end-before: __dynamic_generator_pass_end__ | |||
|
|
|||
| Limitations | |||
| ----------- | |||
| Exception handling and limitations | |||
There was a problem hiding this comment.
Should we separate "Exception handling" and "Limitations"
There was a problem hiding this comment.
Ah yeah, good point.
|
|
||
| Although a generator function creates ``ObjectRefs`` one at a time, currently Ray will not schedule dependent tasks until the entire task is complete and all values have been created. This is similar to the semantics used by tasks that return multiple values as a list. | ||
|
|
||
| ``num_returns="dynamic"`` is not yet supported for actor tasks. | ||
|
|
||
| If a generator function raises an exception before yielding all its values, all values returned by the generator will be replaced by the exception traceback, including values that were already successfully yielded. | ||
| If the task was called with ``num_returns="dynamic"``, the exception will be stored in the ``ObjectRef`` returned by the task instead of the usual ``ObjectRefGenerator``. | ||
| If a generator function raises an exception before yielding all its values, the values that it already stored will still be accessible through their ``ObjectRefs``. |
There was a problem hiding this comment.
If a static generator function?
There was a problem hiding this comment.
Actually for both static and dynamic ones. I can make this clearer.
ericl
approved these changes
Sep 30, 2022
WeichenXu123
pushed a commit
to WeichenXu123/ray
that referenced
this pull request
Dec 19, 2022
…ed by exception ObjectRef (ray-project#28864) Previously if a dynamic generator task errored, we would store the error as the top-level ObjectRef, which could lead to a leak of any objects already stored, since the ObjectRefs are never returned to Python. This PR improves usability by storing the exception as an additional final ObjectRef returned by the generator (see included examples in docs code). All successfully stored objects will still be returned by the generator as usual. Updated some tests to match this new behavior. Signed-off-by: Weichen Xu <weichen.xu@databricks.com>
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?
Previously if a dynamic generator task errored, we would store the error as the top-level ObjectRef, which could lead to a leak of any objects already stored, since the ObjectRefs are never returned to Python.
This PR improves usability by storing the exception as an additional final ObjectRef returned by the generator (see included examples in docs code). All successfully stored objects will still be returned by the generator as usual. Updated some tests to match this new behavior.
Related issue number
Part 1 of #28686:
Handles application-level errors: we now make the partially stored ObjectRefs visible to the application.
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.