Skip to content

ui/db-console: fix duplicate TS query requests on dashboard switch#162612

Open
dhartunian wants to merge 1 commit intocockroachdb:masterfrom
dhartunian:davidh/push-wxnzznsstwsm
Open

ui/db-console: fix duplicate TS query requests on dashboard switch#162612
dhartunian wants to merge 1 commit intocockroachdb:masterfrom
dhartunian:davidh/push-wxnzznsstwsm

Conversation

@dhartunian
Copy link
Collaborator

Note(davidh): this investigation, conclusions, and fix was done by Claude. Keeping a detailed transcript here for posterity. I believe it's correct after reviewing.

When switching dashboards via the dropdown on the Metrics page, time series queries were dispatched multiple times with different time windows, resulting in duplicate HTTP requests to /ts/query. This doubled the load on the time series infrastructure.

The root cause was a chain of events triggered by the dashboard switch:

  1. ErrorBoundary uses key={pathname}, so the entire subtree unmounts and remounts when the dashboard path changes.

  2. setClusterPath called history.push(path) without preserving query params, so ?preset=past-hour was lost from the URL.

  3. On remount, TimeScaleDropdownWithSearchParams's useEffect([]) fired. With no URL params, the else branch called onTimeScaleChange(props.currentScale), which dispatched SET_SCALE with the already-current scale.

  4. The SET_SCALE reducer set shouldUpdateMetricsWindowFromScale = true even though the scale hadn't changed.

  5. MetricsTimeManager (mounted above the ErrorBoundary, stays alive) saw the flag, called setWindow with a new "now" timestamp T2 (slightly different from the existing T1).

  6. MetricsDataProviders that already dispatched requests with T1 in componentDidMount now saw changed timeInfo (T2) and dispatched a second round of requests in componentDidUpdate.

  7. The saga's delay(0) batching window collected both T1 and T2 requests, grouped them into separate batches by timespan, and sent multiple HTTP requests.

Three changes fix this:

  • Preserve query params in setClusterPath by passing { pathname, search } to history.push instead of just the path string. This keeps ?preset=past-hour in the URL across dashboard switches.

  • Skip the SET_SCALE dispatch in TimeScaleDropdownWithSearchParams when the URL has ?preset=X and the redux store already has the matching preset with a moving window.

  • In the else branch (no URL params), call updateUrlParams instead of onTimeScaleChange to sync the URL without dispatching SET_SCALE.

Also removes a dead requests = [] assignment that cleared a function parameter after it was already consumed.

Fixes: #158507

Release note: None

Epic: None

Note(davidh): this investigation, conclusions, and fix was done by
Claude. Keeping a detailed transcript here for posterity. I believe it's
correct after reviewing.

When switching dashboards via the dropdown on the Metrics page, time
series queries were dispatched multiple times with different time
windows, resulting in duplicate HTTP requests to `/ts/query`. This
doubled the load on the time series infrastructure.

The root cause was a chain of events triggered by the dashboard switch:

1. `ErrorBoundary` uses `key={pathname}`, so the entire subtree
   unmounts and remounts when the dashboard path changes.

2. `setClusterPath` called `history.push(path)` without preserving
   query params, so `?preset=past-hour` was lost from the URL.

3. On remount, `TimeScaleDropdownWithSearchParams`'s `useEffect([])`
   fired. With no URL params, the else branch called
   `onTimeScaleChange(props.currentScale)`, which dispatched
   `SET_SCALE` with the already-current scale.

4. The `SET_SCALE` reducer set `shouldUpdateMetricsWindowFromScale =
   true` even though the scale hadn't changed.

5. `MetricsTimeManager` (mounted above the ErrorBoundary, stays alive)
   saw the flag, called `setWindow` with a new "now" timestamp T2
   (slightly different from the existing T1).

6. `MetricsDataProviders` that already dispatched requests with T1 in
   `componentDidMount` now saw changed `timeInfo` (T2) and dispatched
   a second round of requests in `componentDidUpdate`.

7. The saga's `delay(0)` batching window collected both T1 and T2
   requests, grouped them into separate batches by timespan, and sent
   multiple HTTP requests.

Three changes fix this:

- Preserve query params in `setClusterPath` by passing
  `{ pathname, search }` to `history.push` instead of just the path
  string. This keeps `?preset=past-hour` in the URL across dashboard
  switches.

- Skip the `SET_SCALE` dispatch in `TimeScaleDropdownWithSearchParams`
  when the URL has `?preset=X` and the redux store already has the
  matching preset with a moving window.

- In the else branch (no URL params), call `updateUrlParams` instead
  of `onTimeScaleChange` to sync the URL without dispatching
  `SET_SCALE`.

Also removes a dead `requests = []` assignment that cleared a
function parameter after it was already consumed.

Fixes: cockroachdb#158507

Release note: None

Epic: None
@dhartunian dhartunian requested a review from a team as a code owner February 6, 2026 21:36
@trunk-io
Copy link
Contributor

trunk-io bot commented Feb 6, 2026

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ui: time series query batching race condition causes duplicate requests on time window refresh

2 participants