Skip to content

fix: propagate panics from TableStrategy parallel column writers (upstream #8672)#73

Merged
1 commit merged into
spiceai-54from
lukim/spiceai-54-tablestrategy-fix
Jul 20, 2026
Merged

fix: propagate panics from TableStrategy parallel column writers (upstream #8672)#73
1 commit merged into
spiceai-54from
lukim/spiceai-54-tablestrategy-fix

Conversation

@lukekim

@lukekim lukekim commented Jul 20, 2026

Copy link
Copy Markdown

Cherry-pick of upstream vortex-data#8672 (4abe3d04f0) onto spiceai-54.

Upstream yanked 0.76.0 (the release this branch tracks) over this bug: TableStrategy runs its column-chunk fan-out task detached and discards channel-send results, so a panic/death in the parallel write pipeline lets surviving column writers see a clean EOF and finalize columns at different lengths silently — the written file still reports full row count. With the fix, the fan-out future is joined with the column writers and any producer panic/error fails the write loudly.

Context and evidence chain: spiceai/spiceai#11942 (likely root cause of the SF1000 chbench_q10 c_city divergence behind spiceai/spiceai#11910). Consumed by spiceai/spiceai#11943, which currently pins this branch's tip 68badd689 — after this merges, that pin moves to the spiceai-54 merge SHA.

Upstream's regression test table_fanout_panic_propagates passes on this branch (cargo test -p vortex-layout table_fanout).

…ks. (vortex-data#8672)

## 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>
(cherry picked from commit 4abe3d0)

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR cherry-picks an upstream fix to vortex-layout’s TableStrategy writer to ensure failures in the “fan-out/transposition” stage (including panics) cannot be silently converted into channel EOF, which previously allowed columns to finalize at different lengths without failing the write.

Changes:

  • Replace the detached fan-out task with a fanout_fut that is joined with the column-writer futures via futures::future::try_join, so producer errors/panics surface to the caller.
  • Add an explicit error when a column writer finishes early (send fails), preventing silent truncation.
  • Add a regression test (table_fanout_panic_propagates) that validates a panic in the transposition stream propagates (i.e., fails loudly).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lukekim lukekim added the bug Something isn't working label Jul 20, 2026
@lukekim lukekim closed this pull request by merging all changes into spiceai-54 in 1b0f1d2 Jul 20, 2026
@lukekim
lukekim deleted the lukim/spiceai-54-tablestrategy-fix branch July 20, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants