Skip to content

[fix](ray) Publish completion after stream retirement - #237

Merged
kaka11chen merged 1 commit into
AstroVela:mainfrom
kaka11chen:fix/issue-217-terminal-retirement
Jul 28, 2026
Merged

[fix](ray) Publish completion after stream retirement#237
kaka11chen merged 1 commit into
AstroVela:mainfrom
kaka11chen:fix/issue-217-terminal-retirement

Conversation

@kaka11chen

Copy link
Copy Markdown
Contributor

Summary

  • keep a terminal stream record pending while RayStreamAdapter.retire() releases task/source ownership and deletes the ObjectRef stream
  • publish the collector complete event only after that local retirement finishes
  • replace the GC-timing assertion with a deterministic retirement gate that proves completion cannot overtake cleanup

Root cause

AsyncResultCollector._maybe_complete_record() removed the record and published complete before calling record.adapter.retire(). Consumers could therefore observe an empty collector while the adapter still held strong references to TaskLeaseObjectRefGenerator, making the weakref assertion scheduler-dependent in the full fast suite.

Testing

  • python -m pytest tests/fast/test_udf_stream_result_collector.py (21 passed)
  • related stream adapter and collector tests (33 passed)
  • python -m pytest tests/fast/test_udf_process.py (6 passed)
  • scripts/run_release_tests.sh (418 passed, 1 skipped; 7 real-Ray tests passed)
  • scripts/run_fast_tests.sh non-ray (5895 passed, 28 skipped, 8 xfailed, 1 xpassed)
  • pre-commit run --from-ref upstream/main --to-ref HEAD

Closes #217

Copilot AI review requested due to automatic review settings July 28, 2026 10:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@kaka11chen

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 190ef100e3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@QuakeWang QuakeWang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

+1

@kaka11chen
kaka11chen merged commit 420c8cf into AstroVela:main Jul 28, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2][Flaky] Terminal stream weakref can outlive collector retirement in the full fast suite

4 participants