docs(release-notes): add #426, #427, #354 sprint cleanup entries#468
Conversation
Three small follow-ups that landed alongside the larger v26.06.1 features and bug fixes: - arc_query_client_disconnects_total Prometheus counter (PR #466, closes #426) — observability follow-up to PR #424's streaming-on- client-disconnect fix. Placed under the existing #424 entry. - Arrow streaming tests now leak-detect with CheckedAllocator (PR #465, closes #427) — test-hygiene follow-up to the same #424 work; buildArrowBatch helper now releases builders + cols defensively so CheckedAllocator + AssertSize(t,0) catches future Arrow leaks. - Line Protocol parser sub-slice indexing in splitOnDelimiter (PR #467, closes #354) — ingest hot-path perf: ParseLine -41% ns / -52% allocs, ParseBatch -31% ns / -50% allocs. Includes the bench table and notes the declined dynamic-cap-8 suggestion from Gemini review (caused a +9% regression on the full path). Doc-only; +35 lines in RELEASE_NOTES_2026.06.1.md.
|
@gemini-code-assist review please |
There was a problem hiding this comment.
Code Review
This pull request updates the release notes for the 2026.06.1 release, documenting the addition of the arc_query_client_disconnects_total Prometheus counter, memory leak detection improvements in Arrow streaming tests, and performance optimizations in the Line Protocol parser's tokenizer. Feedback is provided to correct a minor technical inaccuracy in the Prometheus terminology used in the release notes, specifically clarifying that the different paths are "label values" rather than separate "labels."
| arc_query_client_disconnects_total{path="arrow_ipc|arrow_json|sql_json"} counter | ||
| ``` | ||
|
|
||
| Three labels distinguish (a) the `/api/v1/query/arrow` IPC endpoint, (b) the `duckdb_arrow`-build path that `/api/v1/query` takes by default, and (c) the pure `database/sql` streaming JSON path. Wired at all 5 existing client-disconnect Warn-log sites in [internal/api/query_arrow.go](internal/api/query_arrow.go), [internal/api/query_arrow_json.go](internal/api/query_arrow_json.go), and [internal/api/query.go](internal/api/query.go). Guarded by the existing `isClientError()` predicate, so server-side stream failures stay in `IncQueryErrors` only — the new counter is strictly client-side noise. Typed `DisconnectPathArrowIPC` / `DisconnectPathArrowJSON` / `DisconnectPathSQLJSON` constants prevent typos at compile time; an unknown label is silently dropped by the increment switch. |
There was a problem hiding this comment.
In Prometheus terminology, path is a single label, while arrow_ipc, arrow_json, and sql_json are label values (or dimension values). Describing them as "three labels" and "an unknown label" is technically inaccurate and can confuse operators or developers configuring dashboards/alerts.
Consider updating this line to refer to them as label values:
Three label values distinguish (a) the `/api/v1/query/arrow` IPC endpoint, (b) the `duckdb_arrow`-build path that `/api/v1/query` takes by default, and (c) the pure `database/sql` streaming JSON path. Wired at all 5 existing client-disconnect Warn-log sites in [internal/api/query_arrow.go](internal/api/query_arrow.go), [internal/api/query_arrow_json.go](internal/api/query_arrow_json.go), and [internal/api/query.go](internal/api/query.go). Guarded by the existing `isClientError()` predicate, so server-side stream failures stay in `IncQueryErrors` only — the new counter is strictly client-side noise. Typed `DisconnectPathArrowIPC` / `DisconnectPathArrowJSON` / `DisconnectPathSQLJSON` constants prevent typos at compile time; an unknown label value is silently dropped by the increment switch.There was a problem hiding this comment.
Code Review
This pull request updates the release notes to document several recent changes, including the addition of the arc_query_client_disconnects_total Prometheus counter, leak detection in Arrow streaming tests using CheckedAllocator, and performance optimizations in the Line Protocol parser's tokenizer. A review comment points out a technical inaccuracy in the description of the Prometheus metric, noting that it has a single label with three possible values rather than "three labels."
| arc_query_client_disconnects_total{path="arrow_ipc|arrow_json|sql_json"} counter | ||
| ``` | ||
|
|
||
| Three labels distinguish (a) the `/api/v1/query/arrow` IPC endpoint, (b) the `duckdb_arrow`-build path that `/api/v1/query` takes by default, and (c) the pure `database/sql` streaming JSON path. Wired at all 5 existing client-disconnect Warn-log sites in [internal/api/query_arrow.go](internal/api/query_arrow.go), [internal/api/query_arrow_json.go](internal/api/query_arrow_json.go), and [internal/api/query.go](internal/api/query.go). Guarded by the existing `isClientError()` predicate, so server-side stream failures stay in `IncQueryErrors` only — the new counter is strictly client-side noise. Typed `DisconnectPathArrowIPC` / `DisconnectPathArrowJSON` / `DisconnectPathSQLJSON` constants prevent typos at compile time; an unknown label is silently dropped by the increment switch. |
There was a problem hiding this comment.
The Prometheus metric arc_query_client_disconnects_total has a single label named path with three possible values (arrow_ipc, arrow_json, and sql_json). Referring to them as 'Three labels' is technically inaccurate. It should be corrected to 'Three label values' or 'Three values for the path label' to ensure technical accuracy in the release notes.
Doc-only follow-up after the sprint's three small PRs merged. Adds release-notes entries for:
+35 lines, single file.
Test plan
🤖 Generated with Claude Code