Deduplicate in-flight Prometheus polls and add interval jitter#16747
Deduplicate in-flight Prometheus polls and add interval jitter#16747alimobrem wants to merge 2 commits into
Conversation
Problem: Each usePrometheusPoll hook creates an independent HTTP request every 15 seconds via useURLPoll. The cluster dashboard fires ~11 simultaneous Prometheus requests per cycle, all at the same instant. When multiple components poll the same query URL, duplicate requests are issued. Solution: Part A — In-flight request deduplication: Add a module-level cache (url-fetch-cache.ts) keyed by URL. When a fetch for a URL is already in-flight, return the existing Promise instead of issuing a duplicate request. The cache entry is cleared when the Promise settles, so the next poll cycle gets fresh data. Part B — Poll interval jitter: Add a per-instance random offset of 0-20% to each usePoll interval. For the default 15-second delay, this spreads requests across a 0-3 second window, preventing the thundering herd where all dashboard cards fire simultaneously. SDK compatibility: usePrometheusPoll (SDK-exported) and useURLPoll (internal SDK) have unchanged signatures. The dedup and jitter are internal implementation details. External plugins benefit automatically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: alimobrem The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Walkthrough
ChangesURL polling behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant useURLPoll
participant dedupedFetch
participant safeFetch
useURLPoll->>dedupedFetch: request URL
dedupedFetch->>safeFetch: start one in-flight request
safeFetch-->>dedupedFetch: resolve or reject
dedupedFetch-->>useURLPoll: return shared promise
dedupedFetch->>dedupedFetch: clear settled URL entry
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@alimobrem: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
…onsumers usePoll is used by 5 consumers (URL polling, console updates, helm detection, catalog items, query browser). Jitter should only apply to URL-based polling, not to all interval-based polling. Move the jitter ref into useURLPoll where it's scoped correctly. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/public/components/utils/url-poll-hook.ts`:
- Around line 19-22: The polling flow in the hook’s tick callback must not let
one unmount abort the shared dedupedFetch request. Update the AbortController
and cleanup handling used by dedupedFetch so cancellation is scoped to the
individual subscriber, or avoid aborting the shared request during unmount,
while preserving normal polling and AbortError handling for genuinely canceled
requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c0d4709-b42e-4f8e-8030-af78178ee26e
📒 Files selected for processing (2)
frontend/packages/console-shared/src/hooks/__tests__/usePoll.spec.tsfrontend/public/components/utils/url-poll-hook.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/packages/console-shared/src/hooks/tests/usePoll.spec.ts
| const jitterRef = useRef(delay ? Math.floor(Math.random() * delay * 0.2) : 0); | ||
| const tick = useCallback(() => { | ||
| if (url) { | ||
| safeFetch(url) | ||
| dedupedFetch(url, safeFetch) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect dedupedFetch implementation for abort handling
cat -n frontend/public/components/utils/url-fetch-cache.ts
# Inspect useSafeFetch abort lifecycle
cat -n frontend/public/components/utils/safe-fetch-hook.tsRepository: openshift/console
Length of output: 1144
Avoid aborting the shared poll request on unmount. dedupedFetch reuses the first caller’s promise, so when that hook unmounts its AbortController cancels the request for every subscriber. The others then swallow AbortError and miss a poll cycle.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/public/components/utils/url-poll-hook.ts` around lines 19 - 22, The
polling flow in the hook’s tick callback must not let one unmount abort the
shared dedupedFetch request. Update the AbortController and cleanup handling
used by dedupedFetch so cancellation is scoped to the individual subscriber, or
avoid aborting the shared request during unmount, while preserving normal
polling and AbortError handling for genuinely canceled requests.
1 participant
Summary
useURLPoll— when multiple hooks poll the same URL simultaneously, they share one HTTP request instead of issuing duplicatesusePoll— spreads ~11 simultaneous dashboard requests across a 0-3 second window instead of thundering-herding at t=0usePrometheusPolloruseURLPollsignatures — SDK compatibility preserved, external plugins benefit automaticallyDetails
Part A — In-flight dedup (
url-fetch-cache.ts):A module-level
Map<string, Promise>keyed by URL. On cache hit, returns the existing in-flight Promise. On settlement (success or error), the entry is cleared so the next poll cycle fetches fresh data. 16 lines, zero dependencies.Part B — Poll jitter (
usePoll.ts):Each hook instance gets a stable random offset of 0 to
delay * 0.2(0-3 seconds for the default 15-second interval). The first tick still fires immediately for fast initial data load. After the jittered start, polls run at consistent intervals. The jitter value is stored in a ref so it's stable across re-renders.Test plan
url-fetch-cachetests passing (5 tests: same-URL dedup, different-URL separation, post-settlement new fetch, error propagation, cache cleanup)usePolltests passing (4 tests: immediate fire, interval ticks, no interval when delay=0, cleanup)/api/prometheus/→ verify requests are staggered🤖 Generated with Claude Code
Summary by CodeRabbit