Skip to content

events: turn panics in cold-read goroutines into errors - #1126

Draft
tamirms wants to merge 2 commits into
tamirms/hot-tier-window-indexfrom
tamirms/cold-read-panics-to-errors
Draft

tamirms wants to merge 2 commits into
tamirms/hot-tier-window-indexfrom
tamirms/cold-read-panics-to-errors

Conversation

@tamirms

@tamirms tamirms commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

What

A panic on one of the cold read path's background goroutines now fails the request instead of ending the process.

  • A ReadItems worker recovers a panic into a packfile.PanicError and keeps its record workspace out of the pool, since the workspace may be half written.
  • The MPHF load in OpenColdReader sends a panic the same way, as its load error.
  • The packfile open goroutine only swallows a panic, because sync.OnceValue repeats it in the request's own waitOpen call, where the bridge recovers it.

A PanicError's message is only the operation and "panicked", because queryEvents and getEvents copy internal error messages into client responses. The panic value and stack ride on the error, and both handlers log them as fields when they log the failure.

Tests: TestReadItemsWorkerPanicIsAnError makes a worker panic and requires a plain error that carries the value and stack, and a reader that keeps serving; TestWithPanicLogsValueAndStack requires the handlers' log entry to carry both.

Why

The bridge turns a handler panic into a failed request and logs its stack, but these three goroutines run outside its recover, so a panic in any of them exits the process. The ReadItems workers run on the request path since #1086 reads cold data 8 at a time; the other two predate this stack. No input is known to make them panic: records carry CRC32C, zstd checks its frames, and counts are checked at open. A bug that does panic now fails the requests that reach it and logs its stack, the same trade the bridge makes for handler panics.

🤖 Generated with Claude Code

@tamirms
tamirms added this pull request to stack #1102 October 19, 2026 13:00
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 514857a to 80fbd4e Compare October 21, 2026 08:48
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 80fbd4e to 9c5401e Compare October 21, 2026 09:12
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 9c5401e to 0fab37e Compare October 21, 2026 11:16
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 0fab37e to 8f6a81d Compare October 21, 2026 14:04
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 8f6a81d to 89eafaf Compare October 21, 2026 14:07
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch 2 times, most recently from 20d0029 to 5aefee6 Compare October 21, 2026 17:05
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 5aefee6 to cc9c643 Compare October 21, 2026 17:15
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch 2 times, most recently from 06facb1 to f5786df Compare October 22, 2026 19:46
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch 6 times, most recently from 32cdb16 to 0b548b7 Compare October 26, 2026 09:12
tamirms and others added 2 commits October 28, 2026 08:05
Three goroutines on the cold read path ran outside the bridge's
recover, so a panic in any of them ended the process. A ReadItems
worker now recovers a panic into a packfile.PanicError and keeps its
record workspace out of the pool. The MPHF load sends a panic the same
way as its load error. The error's message is plain, because
queryEvents and getEvents copy internal error messages into client
responses; the panic value and stack ride on the error instead, and
both handlers log them as fields when they log the failure. The
packfile open goroutine only swallows a panic: sync.OnceValue repeats
it in the request's own waitOpen, where the bridge recovers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wwsd8qTEGeSb7poFEYcgg9
doOpen closed the file on each error return, but a panic in openFile
skipped those closes. With the open goroutine now recovering the panic,
the file stayed open until the garbage collector closed it. A deferred
close now covers every path that does not hand the file to the Reader.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tamirms
tamirms force-pushed the tamirms/cold-read-panics-to-errors branch from 7992107 to 53c961e Compare October 28, 2026 08:05
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