Skip to content

Commit c998e8d

Browse files
committed
feat(sandbox): expire failed Ray submissions immediately
1 parent 14242df commit c998e8d

5 files changed

Lines changed: 58 additions & 14 deletions

File tree

rock/actions/sandbox/sandbox_info.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ class SandboxInfo(TypedDict, total=False):
2929
archive_time: str
3030
auto_transition_state: State
3131
auto_transition_time: str
32-
auto_archive_seconds: int
33-
auto_delete_seconds: int
32+
auto_archive_seconds: int | None
33+
auto_delete_seconds: int | None
3434
archive_prefix: str
3535
registry_namespace: str
3636
extended_params: dict[str, str]

rock/sandbox/sandbox_manager.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from fastapi import UploadFile
88

9-
from rock import env_vars
9+
from rock import InternalServerRockError, env_vars
1010
from rock.actions import (
1111
BashObservation,
1212
CloseBashSessionResponse,
@@ -182,8 +182,21 @@ async def start_async(
182182
with StageTimer("startup_timing", f"[{sandbox_id}] Meta store policy update", logger):
183183
await self._meta_store.update(sandbox_id, sandbox_info)
184184

185-
with StageTimer("startup_timing", f"[{sandbox_id}] Operator submit", logger):
186-
submitted_info = await self._operator.submit(docker_deployment_config, user_info)
185+
try:
186+
with StageTimer("startup_timing", f"[{sandbox_id}] Operator submit", logger):
187+
submitted_info = await self._operator.submit(docker_deployment_config, user_info)
188+
except Exception as e:
189+
immediate_timeout_info = SandboxTimeoutHelper.make_timeout_info(0)
190+
sandbox_info["auto_archive_seconds"] = None
191+
sandbox_info["auto_delete_seconds"] = 0
192+
try:
193+
await self._meta_store.update(sandbox_id, sandbox_info)
194+
await self._meta_store.update_timeout(sandbox_id, immediate_timeout_info)
195+
except Exception:
196+
logger.exception("[%s] failed to update immediate cleanup metadata", sandbox_id)
197+
raise InternalServerRockError(
198+
f"Sandbox {sandbox_id} submission failed and has been marked for immediate deletion: {e}"
199+
) from e
187200
create_time = sandbox_info.get("create_time")
188201
sandbox_info.update(submitted_info)
189202
await self._build_sandbox_info_metadata(sandbox_info, user_info, cluster_info)

rock/sandbox/sandbox_statemachine.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ def apply_effective_auto_transition_policy(info: dict[str, Any], config: DockerD
6767

6868
@staticmethod
6969
def _resolve_user_seconds(info: dict[str, Any], field: str) -> int | None:
70-
raw = info.get(field)
71-
if raw is None:
70+
if field in info:
71+
raw = info[field]
72+
else:
7273
spec = info.get("spec") or {}
7374
raw = spec.get(field)
7475
return SandboxLifecycleHelper._coerce_seconds(raw)

tests/unit/sandbox/test_sandbox_statemachine.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,25 @@
1818
from rock.config import AutoTransitionConfig
1919
from rock.sandbox.sandbox_statemachine import SandboxLifecycleHelper, SandboxStateMachine
2020

21+
22+
class TestLifecyclePolicyResolution:
23+
def test_missing_metadata_field_falls_back_to_spec(self):
24+
info = {"spec": {"auto_archive_seconds": 600, "auto_delete_seconds": 1200}}
25+
26+
assert SandboxLifecycleHelper.resolve_auto_archive_seconds(info) == 600
27+
assert SandboxLifecycleHelper.resolve_auto_delete_seconds(info) == 1200
28+
29+
def test_explicit_none_disables_spec_policy(self):
30+
info = {
31+
"auto_archive_seconds": None,
32+
"auto_delete_seconds": None,
33+
"spec": {"auto_archive_seconds": 600, "auto_delete_seconds": 1200},
34+
}
35+
36+
assert SandboxLifecycleHelper.resolve_auto_archive_seconds(info) is None
37+
assert SandboxLifecycleHelper.resolve_auto_delete_seconds(info) is None
38+
39+
2140
# ---------------------------------------------------------------------------
2241
# Transitions
2342
# ---------------------------------------------------------------------------

tests/unit/sandbox/test_sandbox_transitions.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88

99
import pytest
1010

11+
from rock import InternalServerRockError
1112
from rock.actions.sandbox.response import State
1213
from rock.admin.proto.response import SandboxStartResponse
1314
from rock.common.constants import StopReason
1415
from rock.config import AutoTransitionConfig, SandboxLifecycleConfig
1516
from rock.sandbox.sandbox_manager import SandboxManager
17+
from rock.sandbox.sandbox_statemachine import SandboxLifecycleHelper
1618
from rock.sdk.common.exceptions import BadRequestRockError
1719

1820

@@ -428,22 +430,31 @@ async def assert_pending_exists(*args, **kwargs):
428430
assert mock_meta_store.update.await_count == 2
429431

430432
@pytest.mark.asyncio
431-
async def test_submit_failure_keeps_pending_record_with_spec_timeout_and_effective_policy(
433+
async def test_submit_failure_marks_pending_record_for_immediate_deletion(
432434
self, mgr_start, mock_meta_store, mock_operator, mock_docker_config
433435
):
434-
mock_docker_config.auto_delete_seconds = 7200
436+
mock_docker_config.auto_archive_seconds = 600
435437
mock_operator.submit.side_effect = RuntimeError("ray submit failed")
436438

437-
with pytest.raises(RuntimeError, match="ray submit failed"):
439+
with pytest.raises(
440+
InternalServerRockError,
441+
match="Sandbox sb-1 submission failed and has been marked for immediate deletion: ray submit failed",
442+
):
438443
await mgr_start.start_async(MagicMock(image="python:3.11"))
439444

440445
mock_meta_store.create.assert_awaited_once()
441446
assert mock_meta_store.create.await_args.kwargs["timeout_info"]
442-
assert mgr_start.created_spec["auto_delete_seconds"] == 7200
443-
mock_meta_store.update.assert_awaited_once()
444-
pending_info = mock_meta_store.update.await_args.args[1]
447+
assert mgr_start.created_spec["auto_archive_seconds"] == 600
448+
assert mock_meta_store.update.await_count == 2
449+
pending_info = mock_meta_store.update.await_args_list[-1].args[1]
445450
assert pending_info["state"] == State.PENDING
446-
assert pending_info["auto_delete_seconds"] == 3600
451+
assert pending_info["auto_delete_seconds"] == 0
452+
assert pending_info["auto_archive_seconds"] is None
453+
assert SandboxLifecycleHelper.resolve_auto_archive_seconds(pending_info) is None
454+
assert SandboxLifecycleHelper.resolve_auto_delete_seconds(pending_info) == 0
455+
mock_meta_store.update_timeout.assert_awaited_once()
456+
timeout_info = mock_meta_store.update_timeout.await_args.args[1]
457+
assert timeout_info["auto_clear_time"] == "0"
447458

448459
@pytest.mark.asyncio
449460
async def test_uses_cluster_auto_delete_when_user_policy_is_unspecified(

0 commit comments

Comments
 (0)