Skip to content

Commit d3bec66

Browse files
authored
fix(sandbox): accept extra stdout in kernel warmup probe (#52)
The warmup probe required stdout to be byte-identical to its marker, so any extra interpreter output alongside the marker discarded a healthy session and re-probed. A liveness probe only needs to prove the marker round-tripped: check containment instead. Cold/stale sessions still lack the marker, so the reset_session() retry path is preserved. Robustness cleanup per the re-scope of #51: the originally reported latency figures were withdrawn (measured against a pre-#95 sandbox image), so no performance claim is made. The docstring is also rewritten stack-agnostically; whether the cold-start race exists against the current Go sandbox agent is unverified. Closes #51
1 parent b207eb5 commit d3bec66

2 files changed

Lines changed: 74 additions & 14 deletions

File tree

src/prokube/sandbox/sandbox.py

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -405,18 +405,23 @@ def wait_until_ready(self, timeout: int = 120) -> None:
405405
)
406406

407407
def _warmup_kernel(self, deadline: float) -> None:
408-
"""Probe the Jupyter kernel until it echoes a unique marker back.
408+
"""Probe the sandbox interpreter until it echoes a unique marker back.
409409
410-
After the pod reaches Running, ipykernel still needs ~1–2s before its
411-
first ``execute_request`` produces visible iopub stdout. During that
412-
window, execd's ``exec`` sub-resource returns successfully but the
413-
stdout never reaches the SSE stream, so the first user ``run_code()``
414-
call races the cold kernel pipeline and silently returns empty output.
410+
The first execution after a pod reaches Running can race a cold
411+
interpreter: a probe may return successfully with empty stdout before
412+
the execution pipeline is fully live, in which case the first user
413+
``run_code()`` call would silently return empty output. (Whether the
414+
current sandbox agent still exhibits this race is unverified; the
415+
probe is kept as cheap insurance until warmup is re-measured against
416+
it.)
415417
416418
This method hides that race by running a tiny ``print(<marker>)``
417-
probe in a loop until the stdout matches, proving the kernel pipeline
418-
is end-to-end live. The probe is bounded by ``deadline`` (the same
419-
deadline used by :meth:`wait_until_ready`), so it can never exceed the
419+
probe in a loop until the marker appears in stdout, proving the
420+
execution pipeline is end-to-end live. The check is containment, not
421+
equality: the interpreter may append unrelated text (e.g. warnings)
422+
to the same stream, and extra output does not make the session any
423+
less live. The probe is bounded by ``deadline`` (the same deadline
424+
used by :meth:`wait_until_ready`), so it can never exceed the
420425
caller's overall timeout budget. If the deadline is reached without
421426
success, a warning is logged and the method returns without raising —
422427
the user may still get useful results, and we don't want to block
@@ -425,10 +430,10 @@ def _warmup_kernel(self, deadline: float) -> None:
425430
Notes:
426431
* The marker is per-call (``uuid4().hex``) to avoid collisions
427432
with any user code that happens to print a similar literal.
428-
* If a probe returns successfully but without the marker, discard
429-
that session before retrying. Otherwise a cold/stale Jupyter
430-
session can be reused forever and every probe keeps returning
431-
empty stdout.
433+
* If a probe returns successfully but stdout does not contain the
434+
marker, discard that session before retrying. Otherwise a
435+
cold/stale session can be reused forever and every probe keeps
436+
returning empty stdout.
432437
433438
Args:
434439
deadline: ``time.monotonic()`` value after which the probe gives
@@ -471,7 +476,7 @@ def _warmup_kernel(self, deadline: float) -> None:
471476
sleep_for = min(0.5, max(0.0, deadline - time.monotonic()))
472477
time.sleep(sleep_for)
473478
continue
474-
if result.stdout.strip() == marker:
479+
if marker in result.stdout:
475480
return
476481
self._code.reset_session()
477482
# Loop top will recompute remaining and exit if deadline passed.

tests/test_pause_resume.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,61 @@ def _probe_callback(request: httpx.Request) -> httpx.Response:
742742

743743
sbx._client.close()
744744

745+
def test_wait_until_ready_warmup_accepts_extra_stdout(
746+
self, mock_env, monkeypatch, httpx_mock: HTTPXMock
747+
):
748+
"""A probe whose stdout contains the marker plus extra text succeeds in one attempt.
749+
750+
Regression test for issue #51: the kernel may append unrelated
751+
warnings (e.g. IPython's history-thread SQLite error) to the same
752+
stdout as the marker. That session is live and must not be discarded.
753+
"""
754+
monkeypatch.setattr("time.sleep", lambda _: None)
755+
756+
_mock_version(httpx_mock)
757+
_mock_claim(httpx_mock)
758+
httpx_mock.add_response(
759+
method="GET",
760+
url=f"{BASE}/_platform/sandbox/test-ws/sandboxes/sandbox-test",
761+
json={"name": "sandbox-test", "phase": "Running"},
762+
)
763+
764+
call_counter = {"n": 0}
765+
766+
def _probe_callback(request: httpx.Request) -> httpx.Response:
767+
call_counter["n"] += 1
768+
marker = _extract_marker(request) or ""
769+
return httpx.Response(
770+
200,
771+
json={
772+
"stdout": (
773+
f"{marker}\n"
774+
"The history saving thread hit an unexpected error "
775+
"(OperationalError('attempt to write a readonly "
776+
"database')). History will not be written to the "
777+
"database.\n"
778+
),
779+
"stderr": "",
780+
"success": True,
781+
"execution_time_ms": 1,
782+
},
783+
)
784+
785+
httpx_mock.add_callback(
786+
_probe_callback,
787+
method="POST",
788+
url=f"{BASE}/_platform/sandbox/test-ws/sandboxes/sandbox-test/exec",
789+
is_reusable=True,
790+
)
791+
792+
sbx = Sandbox.from_pool("python-pool")
793+
sbx.wait_until_ready(timeout=5)
794+
795+
assert sbx.status == "Running"
796+
assert call_counter["n"] == 1
797+
798+
sbx._client.close()
799+
745800
def test_wait_until_ready_warmup_caps_per_probe_timeout(
746801
self, mock_env, monkeypatch, httpx_mock: HTTPXMock
747802
):

0 commit comments

Comments
 (0)