perf: optimize feature gates query and improve loading UX#3284
perf: optimize feature gates query and improve loading UX#3284openshift-merge-bot[bot] merged 1 commit intoopenshift:mainfrom
Conversation
Add LIKE pre-filter before regexp_matches to eliminate non-matching rows early, replace window function with DISTINCT ON for the first-seen subquery to avoid join fan-out, enable 4h API response caching, and keep the DataGrid mounted during reloads so it shows a loading overlay instead of unmounting. Benchmarked against production: response time dropped from ~70s to ~19s (3.7x speedup). Results verified identical across all 39 feature gates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
WalkthroughThe changes optimize the feature gates database query by replacing window functions with DISTINCT ON for efficiency, add API response caching, and refactor the frontend FeatureGates component to improve loading state management and toolbar functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
openshift-ci
bot
added
the
approved
| Select(`name, release, regexp_matches(name, '\[(FeatureGate|OCPFeatureGate):([^\]]+)\]|install should succeed') AS match`). | ||
| Where("release = ?", release) | ||
| Where("release = ?", release). | ||
| Where("name LIKE '%FeatureGate:%' OR name LIKE '%install should succeed%'") |
There was a problem hiding this comment.
The regexp match is expensive, but LIKE can use index so its much faster, 4x faster
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sippy-ng/src/tests/FeatureGates.js (1)
276-314:⚠️ Potential issue | 🟡 MinorEnsure the loading overlay stops on fetch errors.
Line 276 sets
isLoadedfalse and the grid now usesloading={!isLoaded}; in the error path (Line 309–313)isLoadednever flips back, so the overlay can stick after a failure.✅ Suggested fix (always end loading)
const fetchData = () => { setLoaded(false) setRows([]) + setFetchError('') let queryString = '' if (filterModel && filterModel.items.length > 0) { @@ - .then((json) => { + .then((json) => { if (json != null) { setRows(json) } else { setRows([]) } - setLoaded(true) }) .catch((error) => { setFetchError( 'Could not retrieve feature gates ' + props.release + ', ' + error ) }) + .finally(() => setLoaded(true)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sippy-ng/src/tests/FeatureGates.js` around lines 276 - 314, The fetchData function currently sets setLoaded(false) before the fetch but never resets it on errors; update fetchData (inside the Promise chain for the fetch in fetchData) to ensure setLoaded(true) is called on both success and failure — either add setLoaded(true) in the .catch handler alongside setFetchError, or use a finally-equivalent (Promise.finally or an async/await try/catch/finally) to call setLoaded(true) so the loading overlay controlled by isLoaded is always cleared after the request.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/db/query/feature_gates.go`:
- Around line 19-30: The query in firstSeenQuery (pkg/db/query/feature_gates.go)
naively casts parts of release to INT/INT[] and will fail for non-numeric
releases like "Presubmits"; modify the SQL to extract and cast only the leading
numeric components (or skip non-numeric releases) before casting—for example use
regexp to pull the leading digit groups (e.g., (regexp_matches(release,
'^(\\d+)'))[1]::int for major and similarly for minor, or use regexp_replace to
strip non-digits) and/or add a WHERE clause to exclude releases without numeric
prefixes so the CASTs in firstSeenQuery succeed safely.
---
Outside diff comments:
In `@sippy-ng/src/tests/FeatureGates.js`:
- Around line 276-314: The fetchData function currently sets setLoaded(false)
before the fetch but never resets it on errors; update fetchData (inside the
Promise chain for the fetch in fetchData) to ensure setLoaded(true) is called on
both success and failure — either add setLoaded(true) in the .catch handler
alongside setFetchError, or use a finally-equivalent (Promise.finally or an
async/await try/catch/finally) to call setLoaded(true) so the loading overlay
controlled by isLoaded is always cleared after the request.
|
Scheduling required tests: |
|
/lgtm |
openshift-ci
bot
added
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neisw, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel Thanks :) |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@stbenjam: all tests passed! 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. |
3 participants
Add LIKE pre-filter before regexp_matches to eliminate non-matching rows early, replace window function with DISTINCT ON for the first-seen subquery to avoid join fan-out, enable 4h API response caching, and keep the DataGrid mounted during reloads so it shows a loading overlay instead of unmounting.
Benchmarked against production: response time dropped from ~70s to ~19s (3.7x speedup). Results verified identical across all 39 feature gates.
Summary by CodeRabbit
Performance
Improvements