Skip to content

Commit 7ca7c99

Browse files
committed
stderr handling
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent b6312d1 commit 7ca7c99

2 files changed

Lines changed: 70 additions & 12 deletions

File tree

bench-orchestrator/bench_orchestrator/runner/executor.py

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33

44
"""Benchmark binary execution."""
55

6+
import selectors
67
import subprocess
8+
from collections import deque
79
from collections.abc import Callable
810
from pathlib import Path
911
from typing import final
@@ -126,7 +128,8 @@ def run(
126128
if self.verbose:
127129
console.print(f"[dim]$ {' '.join(cmd)}[/dim]")
128130

129-
results = []
131+
results: list[str] = []
132+
diagnostic_lines: deque[str] = deque(maxlen=200)
130133

131134
with Progress(
132135
SpinnerColumn(),
@@ -136,28 +139,51 @@ def run(
136139
) as progress:
137140
_task = progress.add_task(f"Running {self.backend.value} {benchmark.value}...", total=None)
138141

142+
# Merge stderr into stdout so verbose benchmark logs cannot fill a separate pipe and
143+
# block the child process before it emits JSON results.
139144
process = subprocess.Popen(
140145
cmd,
141146
stdout=subprocess.PIPE,
142-
stderr=subprocess.PIPE,
147+
stderr=subprocess.STDOUT,
143148
text=True,
149+
bufsize=1,
144150
)
145151

146152
assert process.stdout is not None
147-
for line in iter(process.stdout.readline, ""):
148-
line = line.strip()
149-
if line:
150-
results.append(line)
151-
if on_result:
152-
on_result(line)
153+
selector = selectors.DefaultSelector()
154+
selector.register(process.stdout, selectors.EVENT_READ)
155+
156+
try:
157+
while selector.get_map():
158+
for key, _mask in selector.select(timeout=0.1):
159+
line = key.fileobj.readline()
160+
if line == "":
161+
selector.unregister(key.fileobj)
162+
continue
163+
164+
line = line.rstrip()
165+
if not line:
166+
continue
167+
168+
if line.startswith("{"):
169+
results.append(line)
170+
if on_result:
171+
on_result(line)
172+
else:
173+
diagnostic_lines.append(line)
174+
console.print(line, markup=False)
175+
finally:
176+
selector.close()
153177

154178
ret_code = process.wait()
155179

156180
if ret_code != 0:
157-
stderr = process.stderr.read() if process.stderr else ""
158181
console.print(f"[red]Benchmark failed with code {process.returncode}[/red]")
159-
if stderr:
160-
console.print(f"[red]{stderr}[/red]")
161-
raise RuntimeError(f"Benchmark {self.backend.value} {benchmark.value} failed: {stderr}")
182+
diagnostics = "\n".join(diagnostic_lines)
183+
if diagnostics:
184+
console.print(f"[red]{diagnostics}[/red]")
185+
raise RuntimeError(
186+
f"Benchmark {self.backend.value} {benchmark.value} failed: {diagnostics}"
187+
)
162188

163189
return results

bench-orchestrator/tests/test_executor.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# SPDX-License-Identifier: Apache-2.0
22
# SPDX-FileCopyrightText: Copyright the Vortex contributors
33

4+
import sys
5+
import textwrap
46
from pathlib import Path
57

68
from bench_orchestrator.config import Benchmark, ExecutionBackend, Format
@@ -44,3 +46,33 @@ def test_build_command_omits_formats_for_lance_backend() -> None:
4446
assert "--formats" not in cmd
4547
assert "--queries" in cmd
4648
assert "1,3" in cmd
49+
50+
51+
def test_run_streams_logs_without_counting_them(tmp_path: Path) -> None:
52+
script = tmp_path / "fake-bench.py"
53+
script.write_text(
54+
textwrap.dedent(
55+
f"""\
56+
#!{sys.executable}
57+
import sys
58+
59+
print("preparing duckdb tables", file=sys.stderr, flush=True)
60+
print('{{"engine":"duckdb","format":"parquet","query":0}}', flush=True)
61+
print("finished query 0", file=sys.stderr, flush=True)
62+
"""
63+
)
64+
)
65+
script.chmod(0o755)
66+
67+
executor = BenchmarkExecutor(script, ExecutionBackend.DUCKDB)
68+
streamed: list[str] = []
69+
70+
results = executor.run(
71+
benchmark=Benchmark.CLICKBENCH,
72+
formats=[Format.PARQUET],
73+
iterations=1,
74+
on_result=streamed.append,
75+
)
76+
77+
assert results == ['{"engine":"duckdb","format":"parquet","query":0}']
78+
assert streamed == results

0 commit comments

Comments
 (0)