Basekick-Labs / Basekick-Labs/arc

arcx: the Arrow IPC stream writer recovers from a panic without the #729 poison marker and truncation trailer

Open
#846 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
677
Forks
53
Avg merge
9h 14m
Merged PRs (30d)
164

Description

Follow-up from the #717 review of PR #845, arcx-only (builds with the `arcx_engine` tag; no release ships it).

PR #845 wraps the three body-stream writers in `internal/api/arcx_hook.go` in `safeStream`, so a panic inside one fails the request instead of crashing the process. For the msgpack and JSON writers that is the same disposition the stock writers have. The Arrow IPC writer (`tryArcxRouterArrow`) is not quite the same: the stock Arrow IPC writer in `query_arrow.go` additionally poisons the stream (`poisonArrowStream`) and sets the `arrowStreamTruncatedTrailer` on a panic (#729), so an Arrow reader cannot mistake a stream cut at a batch boundary for a complete one. The arcx site has neither, so after #845 a panic there leaves the client with a stream that ends cleanly at the last written batch and no end-of-stream marker or trailer.

Before #845 the same panic took the process down, so this is strictly an improvement, but the arcx Arrow path should adopt the #729 poison + trailer before the arcx engine becomes the default engine. The site is also still uncapped (see the #702 note above the hook call in `query_arrow.go`).

Fix shape: route the arcx Arrow IPC site through the same `setBodyStreamWithTrailers` + `poisonArrowStream` path the stock writer uses, or factor that pair into a helper both call.

Contributor guide

Open the contributing guide

Research direction

Read internal/api/arcx_hook.go and compare tryArcxRouterArrow with the stock Arrow path in query_arrow.go, including the #702 note. Trace how setBodyStreamWithTrailers and poisonArrowStream are used, then verify the arcx path marks a panic-truncated stream with the poison marker and arrowStreamTruncatedTrailer without crashing the process.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.