diff --git a/TODO.md b/TODO.md index 1c2be9a..758a163 100644 --- a/TODO.md +++ b/TODO.md @@ -196,3 +196,11 @@ Session-start and session-end records with model, goal, outcome. Written to `--l **STATUS: SHIPPED 2026-07-06** — `has_running_marker` is now liveness-aware. Observed live: a session teardown killed the API mid-execution of `209fc5df`, leaving an orphaned `running.json`; `check_registry` would have answered "already running — poll instead" to every resubmission of that config, permanently, and the marker had to be removed by hand. Fix (single choke point — `running_marker.has_running_marker`, inherited by `check_registry` and the API's `is_running`): a marker only counts as running if it is readable, younger than `MAX_MARKER_AGE_SECONDS` (6h — also caps the PID-reuse window), and (same-host) its recorded PID is alive. Stale markers are removed on detection (self-heal) with a loud `[WARN]`. Different-host markers within the age window are conservatively treated as alive (can't probe a remote PID). 6 regression tests pin each rule, including the exact 209fc5df scenario (dead PID → not running → marker gone). + +## 13. No fail-closed resource cap — a granted 16GB memory_limit froze a 16GB host + +**STATUS: SHIPPED 2026-07-06 (validation-level guard); OS-level sandbox still open.** An agent-built sort_spill config swept `duckdb.memory_limit: [512MB, 16GB]` on the 16GB dev machine; DuckDB took what it was granted and the host froze hard (not a clean OOM — full freeze). The config was *well-designed for the question* — the danger was purely host-relative. + +Guard shipped at the single validation choke point (`_check_memory_limits_fit_host`): any engine memory-limit (matrix lane or engine_params) above 50% of physical RAM is rejected at submission, with `meta.allow_high_memory: true` as the explicit, loud override. 6 tests incl. the exact freeze config. The agent coaching loop turns the 422 into a smaller re-submission automatically. + +Still open (harness-tenets gap, deliberate): OS-level enforcement (sandbox/cgroup/ulimit) for defense-in-depth below the validation layer — validation can't stop a workload that grows past its declared limit. diff --git a/sql_benchmarks/validation.py b/sql_benchmarks/validation.py index 8d0a9f0..2d80108 100644 --- a/sql_benchmarks/validation.py +++ b/sql_benchmarks/validation.py @@ -28,6 +28,7 @@ def validate_experiment_config(config: dict, source_label: str = "config") -> No _check_matrix_present(config, source_label) _check_matrix_aliases_resolvable(config, source_label) _check_table_rows_are_aliases(config, source_label) + _check_memory_limits_fit_host(config, source_label) def _check_matrix_present(config: dict, source_label: str) -> None: @@ -93,3 +94,78 @@ def _check_table_rows_are_aliases(config: dict, source_label: str) -> None: f"Literals are rejected because they don't feed the SQL " f"template substitution pipeline correctly." ) + + +# --- Host-memory guard (fail-closed) ----------------------------------------- +# +# Observed live 2026-07-06: an agent-built sort_spill config swept +# `duckdb.memory_limit: [512MB, 16GB]` on a 16GB machine. DuckDB takes the +# memory it is granted; the 16GB lane froze the host (hard freeze, not a +# clean OOM). No OS-level sandbox exists (harness-tenets gap), so the +# validation surface — the single choke point every submission passes +# through — is where the lab fails closed. +# +# Rule: any engine memory-limit value (matrix lane or engine_params) above +# MEMORY_CAP_FRACTION of physical RAM is rejected at submission with an +# explicit override (`meta.allow_high_memory: true`) for machines/operators +# that know what they're doing. + +MEMORY_CAP_FRACTION = 0.5 + +_MEMORY_UNIT_BYTES = { + "kb": 1024, "kib": 1024, + "mb": 1024 ** 2, "mib": 1024 ** 2, + "gb": 1024 ** 3, "gib": 1024 ** 3, + "tb": 1024 ** 4, "tib": 1024 ** 4, +} + + +def _parse_memory_bytes(value) -> "int | None": + """'512MB' / '16GB' / '1.5GiB' -> bytes. None if not a memory string.""" + import re + if not isinstance(value, str): + return None + m = re.fullmatch(r"\s*(\d+(?:\.\d+)?)\s*([KMGT]i?B)\s*", value, re.IGNORECASE) + if not m: + return None + return int(float(m.group(1)) * _MEMORY_UNIT_BYTES[m.group(2).lower()]) + + +def _host_memory_bytes() -> int: + import os + return os.sysconf("SC_PAGE_SIZE") * os.sysconf("SC_PHYS_PAGES") + + +def _iter_memory_limit_values(config: dict): + """Yield (path, value) for every engine memory-limit setting: matrix + lanes like `execution.matrix.'duckdb.memory_limit'` and static + `engine_params..memory_limit`.""" + matrix = (config.get("execution") or {}).get("matrix") or {} + for dim, values in matrix.items(): + if str(dim).endswith(".memory_limit"): + for v in values or []: + yield f"execution.matrix.{dim}", v + engine_params = config.get("engine_params") or {} + if isinstance(engine_params, dict): + for engine, params in engine_params.items(): + if isinstance(params, dict) and "memory_limit" in params: + yield f"engine_params.{engine}.memory_limit", params["memory_limit"] + + +def _check_memory_limits_fit_host(config: dict, source_label: str) -> None: + meta = config.get("meta") or {} + if meta.get("allow_high_memory") is True: + return # explicit operator override — on their head, loudly opted in + host = _host_memory_bytes() + cap = int(host * MEMORY_CAP_FRACTION) + for path, value in _iter_memory_limit_values(config): + limit = _parse_memory_bytes(value) + if limit is not None and limit > cap: + raise ValueError( + f"SEMANTIC ERROR in {source_label}: {path} = '{value}' exceeds " + f"{int(MEMORY_CAP_FRACTION * 100)}% of this host's physical RAM " + f"({host / 1024**3:.0f}GB). Granting an engine that much memory " + f"can freeze the machine (observed live). Use a value at or " + f"below {cap / 1024**3:.0f}GB, or set 'meta.allow_high_memory: " + f"true' if this host can genuinely afford it." + ) diff --git a/sql_benchmarks_tests/test_experiment_validation.py b/sql_benchmarks_tests/test_experiment_validation.py index db92da1..c82db02 100644 --- a/sql_benchmarks_tests/test_experiment_validation.py +++ b/sql_benchmarks_tests/test_experiment_validation.py @@ -155,3 +155,68 @@ def test_boolean_rows_not_treated_as_int(): # If it does raise, it should NOT be with our "literal 'rows: True'" # message — the check explicitly filters out booleans. assert "literal 'rows: True'" not in str(e) + + +# --------------------------------------------------------------------------- +# Host-memory guard (fail-closed) — observed live 2026-07-06: a 16GB +# duckdb.memory_limit lane on a 16GB machine froze the host hard. +# --------------------------------------------------------------------------- + +import pytest as _pytest + +from sql_benchmarks import validation as _validation +from sql_benchmarks.validation import ( + _check_memory_limits_fit_host, _parse_memory_bytes, +) + + +def test_parse_memory_bytes_units(): + assert _parse_memory_bytes("512MB") == 512 * 1024**2 + assert _parse_memory_bytes("16GB") == 16 * 1024**3 + assert _parse_memory_bytes("1.5GiB") == int(1.5 * 1024**3) + assert _parse_memory_bytes("fast") is None + assert _parse_memory_bytes(4) is None + + +def _cfg(matrix_limits=None, engine_params=None, allow=False): + cfg = {"execution": {"matrix": {}}, "meta": {}} + if matrix_limits: + cfg["execution"]["matrix"]["duckdb.memory_limit"] = matrix_limits + if engine_params: + cfg["engine_params"] = engine_params + if allow: + cfg["meta"]["allow_high_memory"] = True + return cfg + + +def test_memory_limit_above_half_host_ram_rejected(monkeypatch): + """The exact freeze config: 16GB lane on a 16GB host.""" + monkeypatch.setattr(_validation, "_host_memory_bytes", lambda: 16 * 1024**3) + with _pytest.raises(ValueError, match="exceeds 50%"): + _check_memory_limits_fit_host(_cfg(matrix_limits=["512MB", "16GB"]), "t") + + +def test_memory_limit_within_cap_accepted(monkeypatch): + monkeypatch.setattr(_validation, "_host_memory_bytes", lambda: 16 * 1024**3) + _check_memory_limits_fit_host(_cfg(matrix_limits=["512MB", "8GB"]), "t") + + +def test_engine_params_memory_limit_also_guarded(monkeypatch): + monkeypatch.setattr(_validation, "_host_memory_bytes", lambda: 16 * 1024**3) + with _pytest.raises(ValueError, match="engine_params.duckdb.memory_limit"): + _check_memory_limits_fit_host( + _cfg(engine_params={"duckdb": {"memory_limit": "12GB"}}), "t") + + +def test_explicit_override_allows_high_memory(monkeypatch): + """Fail-closed with a loud, explicit opt-in — not a hidden default.""" + monkeypatch.setattr(_validation, "_host_memory_bytes", lambda: 16 * 1024**3) + _check_memory_limits_fit_host( + _cfg(matrix_limits=["16GB"], allow=True), "t") + + +def test_non_memory_matrix_dimensions_ignored(monkeypatch): + monkeypatch.setattr(_validation, "_host_memory_bytes", lambda: 16 * 1024**3) + cfg = {"execution": {"matrix": {"rows": ["small", "large"], + "duckdb.threads": [1, 8]}}, "meta": {}} + _check_memory_limits_fit_host(cfg, "t") # no raise