Skip to content

fix: TableStrategy currently hides panic in spawned column writer tasks.#8672

Merged
robert3005 merged 1 commit into
developfrom
adamg/fix-panic-propegation-async-spawn
Jul 7, 2026
Merged

fix: TableStrategy currently hides panic in spawned column writer tasks.#8672
robert3005 merged 1 commit into
developfrom
adamg/fix-panic-propegation-async-spawn

Conversation

@AdamGS

@AdamGS AdamGS commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

TableStrategy currently hides panics that happen in the individual column streams.

What changes are included in this PR?

Instead of detaching the fanout work, we await it and make sure everything finishes successfully.

What APIs are changed? Are there any user-facing changes?

No API change, added test to verify panic propagation.

Signed-off-by: Adam Gutglick <adam@spiraldb.com>
@AdamGS AdamGS force-pushed the adamg/fix-panic-propegation-async-spawn branch from 4c33887 to 9824901 Compare July 7, 2026 15:50
@AdamGS AdamGS changed the title table strategy fix: TableStrategy currently hides panic in spawned column writer tasks. Jul 7, 2026
@AdamGS AdamGS requested a review from robert3005 July 7, 2026 15:50
@AdamGS AdamGS added the changelog/fix A bug fix label Jul 7, 2026
@AdamGS AdamGS marked this pull request as ready for review July 7, 2026 15:51
@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

⚡ 4 improved benchmarks
❌ 1 regressed benchmark
✅ 1602 untouched benchmarks
⏩ 42 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation chunked_varbinview_into_canonical[(1000, 10)] 170.1 µs 206.4 µs -17.56%
Simulation chunked_varbinview_canonical_into[(1000, 10)] 190.5 µs 154 µs +23.7%
Simulation rebuild_naive 105.5 µs 91.4 µs +15.49%
Simulation chunked_varbinview_into_canonical[(100, 100)] 307.7 µs 272.5 µs +12.94%
Simulation bitwise_not_vortex_buffer_mut[128] 273.6 ns 244.4 ns +11.93%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing adamg/fix-panic-propegation-async-spawn (9824901) with develop (0d50342)

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@robert3005 robert3005 merged commit 4abe3d0 into develop Jul 7, 2026
77 of 80 checks passed
@robert3005 robert3005 deleted the adamg/fix-panic-propegation-async-spawn branch July 7, 2026 16:29
robert3005 pushed a commit that referenced this pull request Jul 8, 2026
## Rationale for this change

Following #8672, I've audited a few other suspicous code paths that had
similar shape, mostly on the read side.

## What changes are included in this PR?

1. Propagate panics and shutdown correctly for `FileSegmentSource`.
2. Handle panics correctly in the shared iterator used by DuckDB
3. Misc panic handling around our runtime abstraction. 

## What APIs are changed? Are there any user-facing changes?

None

---------

Signed-off-by: Adam Gutglick <adam@spiraldb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants