Antithesis scenario demonstrates termination consistency - #1923
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. |
82f962d to
f02e786
Compare
9323066 to
3030ba8
Compare
This comment has been minimized.
This comment has been minimized.
3030ba8 to
911b945
Compare
f02e786 to
ec3c3ba
Compare
911b945 to
a6d68de
Compare
ec3c3ba to
bd9cd93
Compare
a6d68de to
c14e5cd
Compare
bd9cd93 to
4132c2e
Compare
This commit introduces a check to assert that the capture is consistent when lading shuts down, or is shut down. This is check for both variations of the capture file.
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: 5ec6a09645
ℹ️ 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".
| --capture-path /capture/capture.jsonl \ | ||
| --capture-flush-seconds 1 \ |
There was a problem hiding this comment.
The general scenario hard-codes a .jsonl capture path and leaves --capture-format at its JSONL default; the sampled configuration only varies generators, and a repo-wide search found no other setting of the capture format in this scenario. Consequently the new Parquet and Multi branches are unreachable, so Antithesis can report this termination-consistency scenario healthy without testing two of the three formats the checker explicitly claims to cover.
Useful? React with 👍 / 👎.
| // A parse failure is real corruption only if it is not the final | ||
| // line; a torn final line is the tolerated interrupted write. | ||
| if idx + 1 != line_count { | ||
| torn_before_final += 1; |
There was a problem hiding this comment.
Only tolerate the final parse failure when the file does not end in \n. Rust's str::lines() omits the trailing empty segment, so for "{bad json}\n" the malformed record is still considered the final line and is silently accepted. This creates a false negative for any fully written but corrupt last record; if lading is restarted afterward, File::create truncates the capture before a later record can turn it into detectable mid-file corruption.
Useful? React with 👍 / 👎.
| # lading writes its capture here (rw); it is a named volume so it survives a | ||
| # node_termination of this container for the checker to validate. | ||
| - capture:/capture |
There was a problem hiding this comment.
When Antithesis restarts the lading service after a node termination, the persisted ready sentinel lets the entrypoint immediately launch lading against the same capture path, and CaptureManager::new_jsonl opens that path with fs::File::create, truncating the killed run's artifact. The named volume therefore preserves the file only during the narrow interval before restart; an anytime_ check scheduled after restart usually sees a new empty or healthy capture and can miss the corruption this scenario is intended to detect. Use per-run paths or move/snapshot the prior artifact before reopening it.
Useful? React with 👍 / 👎.
| Err(_) => ParquetReport { | ||
| readable: false, | ||
| invariants_hold: false, | ||
| records: 0, |
There was a problem hiding this comment.
validate_parquet returns Err not only for the expected missing-footer case but also for I/O failures, Arrow decoding failures, missing columns, and invalid column types. Collapsing every error into readable: false makes the caller's !r.readable || r.invariants_hold assertion pass even for a footer-complete capture with a corrupt or incompatible schema, hiding exactly the kind of malformed output this oracle should report. Only the specific incomplete-footer condition should be tolerated; other validation errors need to fail the assertion.
Useful? React with 👍 / 👎.
1 participant
What does this PR do?
This commit introduces a check to assert that the capture is consistent
when lading shuts down, or is shut down. This is check for both variations
of the capture file.