observability: log the ic-persist thread panic cause (closes #585)#604
Merged
Conversation
A panic in the off-thread snapshot dump (encode bug, filesystem edge) previously
surfaced to the shard only as an opaque oneshot RecvError ("save failed"), with the
panic cause lost. Wrap the encode+write under catch_unwind and tracing::error! the
downcast panic payload before the thread exits, then turn the panic into an io::Error
so the save fails cleanly (the durable prior snapshot is intact, manifest-last).
AssertUnwindSafe is sound: the caught closure only READS the frozen Arcs (immutable
for the save) + the filesystem.
The backpressure-wait-metric half of #585 is moot (the save throttle was measured to
worsen the tail and is deprecated for tail use, see #589).
Closes #585.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Zeke <ezequiel.lares@outlook.com>
perf-gate (A5)Same-runner ratchet of HEAD against the merge-base (both rebuilt and measured in this job).
Overall: PASS
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Closes #585. A panic in the off-thread snapshot dump (
ic-persist-<n>, from #588) previously surfaced to the serving shard only as an opaque oneshotRecvError("save failed"), with the panic CAUSE lost. Now the encode+write runs undercatch_unwind, and on a panic the downcast payload (&str/String) istracing::error!-logged with the shard index, then turned into anio::Errorso the save fails cleanly. The DURABLE prior snapshot is intact (manifest-last), so this is a failed save, never data loss.AssertUnwindSafeis sound: the caught closure only READS the frozenArcs (immutable for the save) + the filesystem, and the frozen slots are dropped regardless.The backpressure-wait-metric half of #585 is MOOT: the save throttle (#577/#578) was measured to WORSEN the concurrent-snapshot tail and is deprecated for tail use (see #589), so a metric for it is not worth adding.
Tests
persistence suite (10, incl.
bgsave_cow_serves_inplace_writes_and_reloads_consistently,bgsave_off_thread_keeps_datapath_low_latency_during_dump) passes; clippy-D warnings(incl.io_other_error->Error::other) + io_uring build + fmt clean; no dashes. The panic path is a pure add around the existing dump; the normal save path is behavior-unchanged.Closes #585.
🤖 Generated with Claude Code