release-26.2: stmtdiagnostics: add continuous bundle collection with COUNT-based limiting#166159
Conversation
|
This backport targets 26.2, which is an End-of-Life (EOL) version. Please verify that backporting to this EOL version is intentional and appropriate. EOL versions no longer receive maintenance updates according to our support policy. Thanks for opening a backport. Before merging, please confirm that the change does not break backwards compatibility and otherwise complies with the backport policy. Include a brief release justification in the PR description explaining why the backport is appropriate. All backports must be reviewed by the TL for the owning area. While the stricter LTS policy does not yet apply, please exercise judgment and consider gating non-critical changes behind a disabled-by-default feature flag when appropriate. |
themavik
left a comment
There was a problem hiding this comment.
What it does: Adds continuous statement diagnostic bundle collection with COUNT-based per-request limiting, plus request_id schema migration.
Done well: Clear migration path (V26_2_StmtDiagnosticsRequestID) with version gating. The mixed-version behavior (no limit until migration completes, NULL request_id for pre-migration bundles) is sensible. Good test coverage in statement_diagnostics_test.go. Cluster setting sql.stmt_diagnostics.max_bundles_per_request gives operators control.
Suggestion: The SELECT COUNT(*) at insertion time could become a hotspot under heavy continuous collection. Consider whether a cached counter or batched cleanup would help at scale—or document that operators should tune max_bundles_per_request for high-throughput scenarios.
Add the `request_id` column and index to `system.statement_diagnostics` via a new cluster version gate `V26_2_StmtDiagnosticsRequestID`. The `request_id` column links diagnostic bundles to their originating request, enabling multiple bundles per request for continuous collection. The migration adds: - `request_id INT8` column to `system.statement_diagnostics` - `request_id_idx` index for efficient lookup by request Resolves: cockroachdb#161001 Release note: None Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…miting Implement continuous diagnostic bundle collection for requests that have both `sampling_probability` and `expires_at` set. At bundle insertion time, enforce the per-request limit by counting existing bundles via `SELECT COUNT(*) FROM system.statement_diagnostics WHERE request_id = ?` and comparing against the `sql.stmt_diagnostics.max_bundles_per_request` cluster setting (default 10). This replaces a previous design that used an in-memory counter and a `collections_remaining` DB column. The `max_bundles_per_request` setting now requires a positive integer (minimum 1); unlimited collection is no longer supported. During mixed-version rolling upgrades, continuous requests collect without limit enforcement until the migration completes and `request_id` becomes available. Pre-migration bundles have NULL `request_id`, so the post-migration counter effectively resets to 0. Resolves: cockroachdb#161001 Release note (ops change): Statement diagnostics requests with sampling_probability and expires_at now collect up to 10 bundles (configurable via sql.stmt_diagnostics.max_bundles_per_request) instead of a single bundle. Set the cluster setting to 1 to restore single-bundle behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address feedback from dhartunian's review:
- Update copyright year from 2025 to 2026 in new migration file
- Refactor duplicate UPDATE statement in statement_diagnostics.go
to extract SQL execution after if/else
- Remove unused hasMatchingIndex() function and export
- Test improvements:
- Remove unnecessary time.Sleep in test
- Remove redundant count re-query after SucceedsSoon
- Replace two require calls with single require.Equal
- Extract SQL execution outside SucceedsSoon loop to prevent
false positives from accumulated statements (lines 1610, 1887)
Informs: cockroachdb#161001
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
c3b0ba9 to
79de19e
Compare
Backport 3/3 commits from #162154 on behalf of @isaactwong.
Summary
request_idcolumn and index tosystem.statement_diagnosticsvia schema migration (V26_2_StmtDiagnosticsRequestID), linking bundles to their originating diagnostic request.sampling_probabilityandexpires_atset. At insertion time, enforces per-request limits by counting existing bundles (SELECT COUNT(*) WHERE request_id = ?) against thesql.stmt_diagnostics.max_bundles_per_requestcluster setting (default 10).request_id, so the counter resets to 0 post-migration.Commit breakdown
🤖 Generated with Claude Code
Release justification: missed the branch cut yesterday due to merge conflicts