Skip to content

fix(bench): warm every app before judging, and relay the daemon's output live - #18

Merged
proggeramlug merged 1 commit into
mainfrom
fix/bench-report-every-app
Aug 28, 2026
Merged

fix(bench): warm every app before judging, and relay the daemon's output live#18
proggeramlug merged 1 commit into
mainfrom
fix/bench-report-every-app

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Two harness defects, both found while reproducing perry#8546 and both of which cost real conclusions.

1. The warm phase stopped at the first non-200

warm_every_app asserted per app in index order, so a run reported bench-000: 500 and nothing about the rest. The question under #8546 is which apps fail, and the answer turned out to be: every application except the one whose init finished last, regardless of dispatch order (serial 3 apps: 000 and 001 fail, 002 serves; concurrent: whichever finished init last serves). That rule was invisible while the harness stopped at the first failure.

Now every app is warmed, each status and a body excerpt are printed, and the assertion carries the full failure list.

2. The daemon's output was discarded on the failure paths that need it

  • stderr went into a bounded sink that only the ready-timeout branches printed. A daemon that dies mid-request — the exact outcome PERRY_GC_PROTECT_FROMSPACE=1 produces on a hit — surfaced as reqwest::Error(IncompleteMessage) with the fault report thrown away.
  • stdout was read only until HTTP listener ready. After that the deployment dispatch failed … handler promise rejected: <reason> line — the one fix(worker): never report an empty handler rejection reason #14 exists to make useful — was never read, and a chatty daemon would eventually block on a full pipe.

COOP_BENCH_TRACE_DAEMON=1 relays both streams live; the stdout reader keeps draining regardless. Default behaviour is unchanged.

Verified

With this in place, on Perry 0.5.1516, the from-space fault report from the pre-#15 worker printed in full (obj_type=5, backtrace in invoke_buffer_handler), and post-#15 the per-app table above was produced in four configurations. Results are on perry#8546.

https://claude.ai/code/session_01UZJbhb2FTuakurTHPAKQgd

Summary by CodeRabbit

  • Enhancements
    • Added optional live daemon output streaming during benchmark runs for easier diagnostics.
    • Improved startup failure reporting with retained error output.
    • Benchmark warm-up now runs across all applications instead of stopping after the first failure.
    • Warm-up results include request statuses and response excerpts, with failures reported together for easier review.

…put live

The density harness stopped at the first non-200 warm response, so a run
said 'bench-000: 500' and nothing about the others. Under perry#8546 that
hid the actual rule: every application except the one whose init finished
LAST fails, whatever the dispatch order. Warm all of them, print each
status and body excerpt, then assert with the full list.

It also dropped the daemon's output on the paths that matter. stderr was
captured into a bounded sink that only the ready-timeout branches printed,
so a daemon that died mid-request (a from-space fault under
PERRY_GC_PROTECT_FROMSPACE) surfaced as a bare reqwest error with the fault
report discarded. stdout was read only until 'HTTP listener ready', which
discards the 'deployment dispatch failed' line that names a 500's cause and
lets a chatty daemon block on a full pipe. COOP_BENCH_TRACE_DAEMON=1 now
relays both streams live; the stdout reader drains regardless.

Claude-Session: https://claude.ai/code/session_01UZJbhb2FTuakurTHPAKQgd
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Free

Run ID: 2e880b5e-c148-40ba-af86-a27cdc5334c8

📥 Commits

Reviewing files that changed from the base of the PR and between 3e7180e and 00a80e4.

📒 Files selected for processing (1)
  • crates/coop-daemon/tests/resource_benchmark.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The resource benchmark can stream daemon output when tracing is enabled. It continues draining daemon output after readiness. Warm-up requests now run for every app and report all failures together.

Changes

Resource benchmark

Layer / File(s) Summary
Daemon output handling
crates/coop-daemon/tests/resource_benchmark.rs
When COOP_BENCH_TRACE_DAEMON is set, the benchmark forwards daemon stderr and stdout live. It retains bounded stderr capture and drains stdout after readiness.
Warm-up failure reporting
crates/coop-daemon/tests/resource_benchmark.rs
The benchmark logs each warm-up response, collects status and expected-body failures for every app, and reports the complete failure list after all requests finish.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 00a80

This PR improves benchmark warm-up reporting and preserves live daemon diagnostics without changing product behavior; no actionable merge-blocking risk remains beyond normal checks and review.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing.

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant