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

[Core] Fixed a potential buggy code where create_task is not invoked #31018

Merged
merged 3 commits into from
Dec 16, 2022

Conversation

rkooo567
Copy link
Contributor

@rkooo567 rkooo567 commented Dec 12, 2022

Signed-off-by: SangBin Cho rkooo567@gmail.com

Why are these changes needed?

When using create_task in a "fire and forget" way, we should keep the references
alive for the reliable execution. This API is used to fire and forget
asynchronous execution.

Details:
Screen Shot 2022-12-11 at 11 31 03 PM

This PR fixes the code that has the problem specified above ^. Also it implements a utility method to avoid such cases.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

Signed-off-by: SangBin Cho <rkooo567@gmail.com>
@rkooo567 rkooo567 added the @author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer. label Dec 12, 2022
Copy link
Contributor

@architkulkarni architkulkarni left a comment

Choose a reason for hiding this comment

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

Did the end of the PR description get cut off accidentally? It ends with "Q: How should we"

Signed-off-by: SangBin Cho <rkooo567@gmail.com>
@rkooo567
Copy link
Contributor Author

@architkulkarni deleted that part. I don't remember what I was thinking to write lol

@rkooo567
Copy link
Contributor Author

also added an unit test for run_backgroun_task

@rkooo567
Copy link
Contributor Author

running a mac test now

@rkooo567 rkooo567 merged commit 0d5a90c into ray-project:master Dec 16, 2022
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this pull request Dec 19, 2022
…ay-project#31018)

Signed-off-by: SangBin Cho <rkooo567@gmail.com>

When using create_task in a "fire and forget" way, we should keep the references
alive for the reliable execution. This API is used to fire and forget
asynchronous execution.

Signed-off-by: Weichen Xu <weichen.xu@databricks.com>
AmeerHajAli pushed a commit that referenced this pull request Jan 12, 2023
…31018)

Signed-off-by: SangBin Cho <rkooo567@gmail.com>

When using create_task in a "fire and forget" way, we should keep the references
alive for the reliable execution. This API is used to fire and forget
asynchronous execution.
tamohannes pushed a commit to ju2ez/ray that referenced this pull request Jan 25, 2023
…ay-project#31018)

Signed-off-by: SangBin Cho <rkooo567@gmail.com>

When using create_task in a "fire and forget" way, we should keep the references
alive for the reliable execution. This API is used to fire and forget
asynchronous execution.

Signed-off-by: tmynn <hovhannes.tamoyan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@author-action-required The PR author is responsible for the next step. Remove tag to send back to the reviewer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants