-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Fix two types of eviction hangs #5225
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll have to modify the .cc test, too. Usually, I run something like this to check the tests:
bazel test --test_output=errors //:reconstruction_policy_test
import ray | ||
|
||
|
||
class TestUnreconstructableErrors(unittest.TestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to put these in a new file? Maybe they should go in test_failure.py
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that test file too large already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'd prefer we just put it there now and reorganize that whole file later, but up to you.
Oops, fixed the test. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test FAILed. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
Test PASSed. |
Test PASSed. |
Test FAILed. |
Test PASSed. |
Test FAILed. |
Test FAILed. |
What do these changes do?
While writing tests for memory limits I ran into these unhandled eviction hangs. This PR fixes them and add tests for them.
Related issue number
I don't think these have an issue associated.
Linter
scripts/format.sh
to lint the changes in this PR.