mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-28 14:10:37 +00:00
fix(benchmark): validate sandbox timing options (#7651)
* fix(benchmark): validate sandbox timing options * fix(benchmark): guard infinite idle reaping --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
c58c71f3fa
commit
966aa9496a
@ -1402,6 +1402,14 @@ As of 2026-10-21, the milestone includes **829 merged pull requests**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **benchmarks:** Reject negative and NaN sandbox idle-timeout values before
|
||||
constructing any provider, and apply the same validation to BoxLite/AIO
|
||||
health-check skip windows. Valid zero, positive, and positive-infinity values
|
||||
retain their existing behavior, OpenSandbox keeps rejecting nonzero skip
|
||||
windows, and AIO benchmark output now records accepted values without implying
|
||||
that its provider consumes them. The idle-reaping scenario rejects infinity
|
||||
before provisioning because it must sleep past the configured timeout. ([#7651])
|
||||
|
||||
- **tui:** The `/threads` and `/model` pickers show thread titles and model
|
||||
names exactly as written. Labels were parsed as Textual markup, so a title
|
||||
such as `Fix [/bold] in parser` exited the TUI with `MarkupError` whenever the
|
||||
@ -13237,3 +13245,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#7597]: https://github.com/bytedance/deer-flow/pull/7597
|
||||
[#7606]: https://github.com/bytedance/deer-flow/pull/7606
|
||||
[#7631]: https://github.com/bytedance/deer-flow/pull/7631
|
||||
[#7651]: https://github.com/bytedance/deer-flow/pull/7651
|
||||
|
||||
@ -10555,3 +10555,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#7597]: https://github.com/bytedance/deer-flow/pull/7597
|
||||
[#7606]: https://github.com/bytedance/deer-flow/pull/7606
|
||||
[#7631]: https://github.com/bytedance/deer-flow/pull/7631
|
||||
[#7651]: https://github.com/bytedance/deer-flow/pull/7651
|
||||
|
||||
@ -46,9 +46,9 @@ vars; `summarize` and pytest are offline. See its README for the protocol.
|
||||
Benchmarks in `scripts/benchmark/` must be standalone and reproducible. Import
|
||||
production functions; never duplicate them or introduce an alternative runtime.
|
||||
|
||||
OpenSandbox benchmarks reuse the production lifecycle with scoped config overrides.
|
||||
Export only non-secret dimensions; final cleanup failures must fail both the runner
|
||||
and JSONL summary. See the sandbox benchmark README for supported options and tests.
|
||||
Sandbox benchmarks reuse production lifecycle. Reject negative/NaN timing values
|
||||
before provider creation, export no secrets, and fail on final cleanup errors.
|
||||
See the sandbox benchmark README for supported options and tests.
|
||||
|
||||
- Pin every external dataset by immutable revision and SHA-256. Callers provide
|
||||
the local dataset path; evaluation commands must not silently download data.
|
||||
|
||||
@ -75,13 +75,20 @@ Python and POSIX utilities required by the selected workload and output sync.
|
||||
The normal warm-up creates and destroys a separate remote before measurement.
|
||||
|
||||
CLI `--replicas` and `--idle-timeout` override their YAML values, including their
|
||||
CLI defaults (3 and 600). All providers reject negative `--idle-timeout` values
|
||||
before provider creation. Zero disables idle reaping, except that the
|
||||
`idle_timeout` scenario requires a positive value. `--image` overrides YAML only
|
||||
when supplied. Connection settings, including HTTPS and `use_server_proxy`, are retained. The exported
|
||||
configuration dimensions contain only image, replicas and idle timeout; credentials,
|
||||
endpoint and environment are not copied into result rows. Record non-secret SDK,
|
||||
server and deployment versions separately for reproducibility.
|
||||
CLI defaults (3 and 600). All providers reject negative and NaN
|
||||
`--idle-timeout` values before provider creation. BoxLite and AIO likewise reject
|
||||
negative and NaN `--health-check-skip-seconds`; zero, positive finite values,
|
||||
and positive infinity remain accepted. OpenSandbox still rejects every nonzero
|
||||
health-check skip value as unsupported. Zero idle timeout disables idle reaping,
|
||||
except that the `idle_timeout` scenario requires a finite positive value because
|
||||
it sleeps past the timeout. `--image` overrides YAML only when supplied.
|
||||
Connection settings, including HTTPS and `use_server_proxy`, are retained. The
|
||||
exported configuration dimensions contain only image, replicas, idle timeout,
|
||||
and the health-check skip window; credentials, endpoint and environment are not
|
||||
copied into result rows. BoxLite consumes the skip window during warm reclaim;
|
||||
AIO accepts and records it for comparison but does not currently alter provider
|
||||
behavior. Record non-secret SDK, server and deployment versions separately for
|
||||
reproducibility.
|
||||
|
||||
`release_ms` includes production best-effort output synchronization. Successful
|
||||
command/release timing does not prove that every artifact was mirrored. Warm hits
|
||||
|
||||
@ -50,6 +50,7 @@ import argparse
|
||||
import importlib
|
||||
import importlib.metadata
|
||||
import json
|
||||
import math
|
||||
import os
|
||||
import re
|
||||
import statistics
|
||||
@ -241,6 +242,7 @@ def _make_aio_provider(config: dict[str, Any]) -> tuple[Any, dict[str, Any]]:
|
||||
"container_prefix": config.get("container_prefix"),
|
||||
"replicas": config.get("replicas", 3),
|
||||
"idle_timeout": config.get("idle_timeout", 600),
|
||||
"health_check_skip_seconds": config.get("health_check_skip_seconds", 0.0),
|
||||
"mounts": config.get("mounts", []),
|
||||
"environment": config.get("environment", {}),
|
||||
"provisioner_url": config.get("provisioner_url", ""),
|
||||
@ -751,18 +753,20 @@ def _parse_args(argv: list[str] | None = None) -> argparse.Namespace:
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
args = _parse_args(argv)
|
||||
if math.isnan(args.idle_timeout) or args.idle_timeout < 0:
|
||||
raise SystemExit("idle timeout must be nonnegative and not NaN")
|
||||
if math.isnan(args.health_check_skip_seconds) or args.health_check_skip_seconds < 0:
|
||||
raise SystemExit("health-check skip seconds must be nonnegative and not NaN")
|
||||
if args.provider == "opensandbox" and (args.fault_inject is not None or args.health_check_skip_seconds != 0):
|
||||
raise SystemExit("opensandbox benchmarks do not support --fault-inject or nonzero --health-check-skip-seconds")
|
||||
if min(args.concurrency, args.users, args.replicas) < 1 or min(args.iterations, args.warmup_iterations) < 0:
|
||||
raise SystemExit("concurrency, users and replicas must be positive; iteration counts must be nonnegative")
|
||||
if args.idle_timeout < 0:
|
||||
raise SystemExit("idle timeout must be nonnegative")
|
||||
if args.fault_inject is not None and (args.concurrency != 1 or args.no_warmpool or args.scenario != "warm_same_thread"):
|
||||
raise SystemExit("fault injection requires warm_same_thread, concurrency=1 and an enabled warm pool")
|
||||
if args.fault_inject is not None and not 0 <= args.fault_inject < args.iterations:
|
||||
raise SystemExit("fault injection iteration must be inside the measured batch")
|
||||
if args.scenario == "idle_timeout" and (args.concurrency != 1 or args.idle_timeout <= 0):
|
||||
raise SystemExit("idle_timeout requires concurrency=1 and a positive idle timeout")
|
||||
if args.scenario == "idle_timeout" and (args.concurrency != 1 or not math.isfinite(args.idle_timeout) or args.idle_timeout <= 0):
|
||||
raise SystemExit("idle_timeout requires concurrency=1 and a finite positive idle timeout")
|
||||
if args.workload == "state_reuse" and (args.scenario != "warm_same_thread" or args.concurrency != 1):
|
||||
raise SystemExit("state_reuse requires --scenario warm_same_thread --concurrency 1")
|
||||
args.run_id = args.run_id or uuid.uuid4().hex[:12]
|
||||
|
||||
@ -31,6 +31,12 @@ event waiters during teardown.
|
||||
benchmark module's clock binding. Keep asyncio's clock real, initialize
|
||||
`Trace.started` from the controlled clock, and advance ticks at event gates.
|
||||
|
||||
`test_bench_opensandbox.py` owns provider-independent sandbox benchmark CLI
|
||||
validation. Invalid timing values must be rejected through real `main()` before
|
||||
fail-loud factories run; accepted controls must preserve factory inputs and JSONL
|
||||
dimensions. Keep live providers stubbed and OpenSandbox's unsupported-option
|
||||
checks distinct from shared timing validation.
|
||||
|
||||
`test_doctor_script.py` runs a copied doctor from a temporary checkout with an
|
||||
empty-model config. Drop inherited `DEER_FLOW_*`, `PYTHONPATH` and `PYTHONHOME`
|
||||
from child processes; cover help, invalid flags, normal reporting and an
|
||||
|
||||
@ -291,7 +291,8 @@ def test_unsupported_options_fail_before_creating_resources(transport, extra):
|
||||
|
||||
|
||||
@pytest.mark.parametrize("provider", ["opensandbox", "boxlite", "aio-docker"])
|
||||
def test_negative_idle_timeout_fails_before_provider_creation(monkeypatch, provider):
|
||||
@pytest.mark.parametrize("value", ["-5", "nan", "NaN", "-inf"])
|
||||
def test_invalid_idle_timeout_fails_before_provider_creation(monkeypatch, provider, value):
|
||||
assert provider in bench.PROVIDER_FACTORIES
|
||||
|
||||
def unexpected_provider(*args, **kwargs):
|
||||
@ -299,4 +300,98 @@ def test_negative_idle_timeout_fails_before_provider_creation(monkeypatch, provi
|
||||
|
||||
monkeypatch.setitem(bench.PROVIDER_FACTORIES, provider, unexpected_provider)
|
||||
with pytest.raises(SystemExit, match="idle timeout must be nonnegative"):
|
||||
bench.main(["--provider", provider, "--scenario", "warm_same_thread", "--idle-timeout", "-5"])
|
||||
bench.main(["--provider", provider, "--scenario", "warm_same_thread", f"--idle-timeout={value}"])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("provider", ["boxlite", "aio-docker"])
|
||||
@pytest.mark.parametrize("value", ["-5", "nan", "NaN", "-inf"])
|
||||
def test_invalid_health_check_skip_fails_before_provider_creation(monkeypatch, provider, value):
|
||||
assert provider in bench.PROVIDER_FACTORIES
|
||||
|
||||
def unexpected_provider(*args, **kwargs):
|
||||
pytest.fail("invalid health-check skip must be rejected before provisioning")
|
||||
|
||||
monkeypatch.setitem(bench.PROVIDER_FACTORIES, provider, unexpected_provider)
|
||||
with pytest.raises(SystemExit, match="health-check skip seconds must be nonnegative"):
|
||||
bench.main(["--provider", provider, f"--health-check-skip-seconds={value}"])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("provider", ["opensandbox", "boxlite", "aio-docker"])
|
||||
def test_idle_timeout_scenario_rejects_infinite_timeout_before_provider_creation(monkeypatch, provider):
|
||||
def unexpected_provider(*args, **kwargs):
|
||||
pytest.fail("an infinite idle_timeout scenario must be rejected before provisioning")
|
||||
|
||||
monkeypatch.setitem(bench.PROVIDER_FACTORIES, provider, unexpected_provider)
|
||||
|
||||
with pytest.raises(SystemExit, match="finite positive idle timeout"):
|
||||
bench.main(["--provider", provider, "--scenario", "idle_timeout", "--idle-timeout=inf"])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("provider", ["boxlite", "aio-docker"])
|
||||
@pytest.mark.parametrize("option", ["--idle-timeout", "--health-check-skip-seconds"])
|
||||
@pytest.mark.parametrize("value", [None, "0", "2.5", "inf"])
|
||||
def test_nonnegative_timing_values_reach_provider_factory(monkeypatch, tmp_path, provider, option, value):
|
||||
captured = []
|
||||
|
||||
class FakeSandbox:
|
||||
def execute_command(self, command, timeout=None):
|
||||
return "ok"
|
||||
|
||||
class FakeProvider:
|
||||
def __init__(self):
|
||||
self._lock = threading.Lock()
|
||||
self._warm_pool = {}
|
||||
|
||||
def acquire(self, thread_id, *, user_id=None):
|
||||
return "sandbox-id"
|
||||
|
||||
def get(self, sandbox_id):
|
||||
return FakeSandbox()
|
||||
|
||||
def release(self, sandbox_id):
|
||||
return None
|
||||
|
||||
def shutdown(self):
|
||||
return None
|
||||
|
||||
def factory(config):
|
||||
captured.append(config)
|
||||
return FakeProvider(), {
|
||||
"image": config["image"],
|
||||
"replicas": config["replicas"],
|
||||
"idle_timeout": config["idle_timeout"],
|
||||
"health_check_skip_seconds": config["health_check_skip_seconds"],
|
||||
}
|
||||
|
||||
monkeypatch.setitem(bench.PROVIDER_FACTORIES, provider, factory)
|
||||
output = tmp_path / f"{provider}-{option[2:]}-{value or 'default'}.jsonl"
|
||||
args = ["--provider", provider, "--iterations", "1", "--warmup-iterations", "0", "--output", str(output)]
|
||||
if value is not None:
|
||||
args.append(f"{option}={value}")
|
||||
|
||||
assert bench.main(args) == 0
|
||||
key = option[2:].replace("-", "_")
|
||||
expected = float(value) if value is not None else {"idle_timeout": 600.0, "health_check_skip_seconds": 0.0}[key]
|
||||
assert captured[0][key] == expected
|
||||
assert json.loads(output.read_text(encoding="utf-8"))[key] == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize("value", [0.0, 7.5, float("inf")])
|
||||
def test_aio_factory_forwards_health_check_skip_to_config_and_snapshot(monkeypatch, value):
|
||||
from deerflow.community.aio_sandbox import aio_sandbox_provider as aio_module
|
||||
|
||||
captured = {}
|
||||
|
||||
class FakeAioProvider:
|
||||
def __init__(self):
|
||||
captured.update(vars(aio_module.get_app_config().sandbox))
|
||||
self._config = {"image": "resolved-aio-image"}
|
||||
|
||||
monkeypatch.setattr(aio_module, "AioSandboxProvider", FakeAioProvider)
|
||||
|
||||
provider, snapshot = bench._make_aio_provider({"health_check_skip_seconds": value})
|
||||
|
||||
assert isinstance(provider, FakeAioProvider)
|
||||
assert captured["health_check_skip_seconds"] == value
|
||||
assert snapshot["health_check_skip_seconds"] == value
|
||||
assert snapshot["image"] == "resolved-aio-image"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user