Lading shutdown safety in Antithesis scenario - #1924
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
9323066 to
3030ba8
Compare
9e4cd52 to
646c635
Compare
646c635 to
37b9127
Compare
3030ba8 to
911b945
Compare
37b9127 to
4f9a4c5
Compare
a6d68de to
c14e5cd
Compare
4f9a4c5 to
d0eceb6
Compare
Shudown safety has been a regular source of bugs in lading. This commit adds a new scenario 'shutdown-safety' to demonstrate that lading does in fact shut down clealy. The SUT is now instrumented with four 'breadcrumbs' demonstrating clean exit and I have corrected: * TCP generator no hangs if the other side is unreachable during shutdown `anytime_lading_drained_bounded` workload checker asserts that lading's return code is always 0.
d0eceb6 to
b038ae4
Compare
c14e5cd to
5ec6a09
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b038ae486d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A connected worker honors shutdown here via select. The | ||
| // connect-loop branch above does not select on shutdown, so a | ||
| // worker stuck reconnecting to an unreachable destination | ||
| // never reaches this point. The contrast localizes that gap. |
There was a problem hiding this comment.
In the shutdown-safety scenario's fixed 127.0.0.1:1 configuration, the worker remains in the connect/retry branch at lines 233-250 and never polls this shutdown arm; Tcp::spin then waits indefinitely for that worker, so runtime.shutdown_timeout(60s) outlasts the new 35-second watchdog and every ordinary run records rc == 124 rather than validating a clean exit. GNU timeout --help confirms that it sends TERM after the duration and returns 124 when the command times out. The connect attempt and retry sleep need to select on the shutdown watcher before this scenario can assert rc == 0.
Useful? React with 👍 / 👎.
| `docker-compose.yaml` snouty consumes as `--config`, `launch.env`, the | ||
| hard-wired `lading.yaml`, and the `workload/` build inputs. Config variation is | ||
| deferred to the workload seam. | ||
| - `harness/` — the shared harness crate. Holds the per-timeline config sampler |
There was a problem hiding this comment.
The three newly added bullets for harness/, scenarios/general/, and scenarios/shutdown-safety/ use Unicode em dashes, although repository documentation is required to contain US-ASCII only; replace these separators with ASCII punctuation.
AGENTS.md reference: AGENTS.md:L151-L158
Useful? React with 👍 / 👎.
| EXPERIMENT=15 | ||
|
|
||
| rc=0 | ||
| timeout $((EXPERIMENT + 20)) /usr/local/bin/lading \ |
There was a problem hiding this comment.
In the inspected shutdown-safety pipeline, launch.sh enables global cpu_mod and clock_jitter, but this 35-second watchdog starts at process launch rather than when the experiment timer signals shutdown. Under those configured faults, startup, the 8 MiB payload precomputation, or timer scheduling can consume the 20-second margin before graceful shutdown even begins, causing timeout to record 124 and the checker to misdiagnose slow startup or clock interference as a hung drain. Arm the bound from an externally observable shutdown-start signal, or disable the interfering faults for this timing assertion.
AGENTS.md reference: AGENTS.md:L193-L204
Useful? React with 👍 / 👎.
| // Fourth breadcrumb. The runtime drained within the shutdown-delay backstop | ||
| // and lading is about to return its exit status. | ||
| lading_antithesis::reachable!( | ||
| "lading drained its runtime within the shutdown-delay backstop and exited cleanly" |
There was a problem hiding this comment.
Runtime::shutdown_timeout returns both when all tasks finish and when its deadline expires and the remaining tasks are forcibly dropped, but this breadcrumb fires unconditionally afterward and claims the runtime drained cleanly. Any configuration without the scenario's shorter external watchdog can therefore satisfy this Antithesis reachability assertion after waiting out the backstop with hung tasks, obscuring the exact shutdown regression these breadcrumbs are intended to localize; rename it to describe only backstop completion or separately establish that all runtime tasks joined.
Useful? React with 👍 / 👎.
| # Shutdown-safety scenario | ||
|
|
||
| This scenario tests that lading performs a clean, prompt graceful shutdown when | ||
| its own experiment timer elapses, even while a generator is stuck unable to reach | ||
| its destination. Shutdown correctness matters because lading owns the clock in |
There was a problem hiding this comment.
This new scenario introduces a distinct two-container subsystem and non-obvious architectural choices around an external watchdog, intentionally unreachable transport, fault exclusions, and exit-code oracles, but records those decisions only in a standalone scenario README. Move the durable rationale into a new ADR (including alternatives and consequences) and update the ADR index, leaving this README as operational usage documentation.
AGENTS.md reference: AGENTS.md:L31-L49
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
Shudown safety has been a regular source of bugs in lading. This commit
adds a new scenario 'shutdown-safety' to demonstrate that lading does
in fact shut down clealy. The SUT is now instrumented with four 'breadcrumbs'
demonstrating clean exit and I have corrected:
Additional Notes
anytime_lading_drained_boundedworkload checker asserts that lading's returncode is always 0.