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.

Sign up for GitHub

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

Jump to bottom

[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does not catch correct ObjectLostError). #47940

Merged
merged 30 commits into from
Oct 9, 2024
Merged

[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does not catch correct ObjectLostError). #47940

merged 30 commits into from
Oct 9, 2024

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Oct 8, 2024 •
edited
Loading

Fix spot node preemption problem (RLlib does not catch correct ObjectLostError).

If EnvRunners are located on a SPOT instance and the node pre-empts, sometimes an ObjectLostError is thrown from the ray.get call inside RLlib's FaultTolerantActorManager. This error wouldn't be properly handled and thus the main process (the owner of the FaultTolerantActorManager) would crash.

This would happen even if all proper fault-tolerance settings have been applied:

config.fault_tolerance(ignore_env_runner_failures=True OR recreate_failed_env_runners=True)

Why are these changes needed?

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 added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • 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 :(

sven1977 added 21 commits October 5, 2024 01:46
@sven1977
wip
6fd97ba 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into rl_m…
69b2d6c 
…odule_do_over_bc_default_module_04_refactor_rl_module_and_multi_rl_module
@sven1977
wip
416b622 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
face4a3 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
901ade5 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
ac2acd9 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into rl_m…
28db530 
…odule_do_over_bc_default_module_04_refactor_rl_module_and_multi_rl_module

Signed-off-by: sven1977 <svenmika1977@gmail.com>

# Conflicts:
#	rllib/algorithms/ppo/tests/test_ppo_rl_module.py
#	rllib/algorithms/ppo/torch/ppo_torch_rl_module.py
#	rllib/core/rl_module/multi_rl_module.py
#	rllib/core/rl_module/rl_module.py
#	rllib/tuned_examples/dqn/multi_agent_cartpole_dqn.py
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into rl_m…
ab59679 
…odule_do_over_bc_default_module_04_refactor_rl_module_and_multi_rl_module

Signed-off-by: sven1977 <svenmika1977@gmail.com>

# Conflicts:
#	rllib/algorithms/ppo/tests/test_ppo_rl_module.py
#	rllib/algorithms/ppo/torch/ppo_torch_rl_module.py
#	rllib/core/rl_module/multi_rl_module.py
#	rllib/core/rl_module/rl_module.py
#	rllib/tuned_examples/dqn/multi_agent_cartpole_dqn.py
@sven1977
fixes
5a00666 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
4cd530e 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
2a012f7 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
e4cb737 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
053832a 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fixes
a178c74 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
merge
9f6f391 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
0b2f87b 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
6bb5c1a 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
7c1e0c0 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
05222ad 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
25ed553 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
c1cda51 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 requested review from maxpumperla, simonsays1980 and a team as code owners October 8, 2024 18:24
@sven1977
wip
1037c7d 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Merge branch 'master' of https://github.com/ray-project/ray into fix_…
953c3dc 
…spot_preemption_problem_with_not_catching_right_ray_error
@sven1977
wip
a878caf 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
d50b08a 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
wip
9eaa260 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
fix
4e53ef9 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@github-actions github-actions bot disabled auto-merge October 9, 2024 09:21
@sven1977
fixes
e3ed6f0 
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 enabled auto-merge (squash) October 9, 2024 10:41
@sven1977 sven1977 added tests-ok The tagger certifies test failures are unrelated and assumes personal liability. rllib RLlib related issues rllib-system system issues, runtime env, oom, etc rllib-newstack rllib-oldstack-cleanup Issues related to cleaning up classes, utilities on the old API stack labels Oct 9, 2024
@sven1977 sven1977 merged commit e0bcdd4 into ray-project:master Oct 9, 2024
6 checks passed
@sven1977 sven1977 deleted the fix_spot_preemption_problem_with_not_catching_right_ray_error branch October 9, 2024 12:56
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
a508bd7 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
e5a9a3f 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
4a35f0b 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
5aca74d 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
0170ec7 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
670c8a8 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
fb34398 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
fe2aea0 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
ujjawal-khare pushed a commit to ujjawal-khare-27/ray that referenced this pull request Oct 15, 2024
@sven1977 @ujjawal-khare
[RLlib; fault-tolerance] Fix spot node preemption problem (RLlib does…
a994eec 
… not catch correct `ObjectLostError`). (ray-project#47940)

Signed-off-by: ujjawal-khare <ujjawal.khare@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonsays1980 simonsays1980 simonsays1980 approved these changes

@maxpumperla maxpumperla Awaiting requested review from maxpumperla

Assignees

@simonsays1980 simonsays1980

Labels
go add ONLY when ready to merge, run all tests rllib RLlib related issues rllib-newstack rllib-oldstack-cleanup Issues related to cleaning up classes, utilities on the old API stack rllib-system system issues, runtime env, oom, etc tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sven1977 @simonsays1980