fix: load balancer page review followups - #723
Conversation
…fault probe, retry 0, serialized saves, estimated shares)
|
Warning Review limit reachedNext included review available in 32 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (13)
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 |
github-actions
Bot
added
area/frontend
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| queue.current = queue.current | ||
| .catch(() => undefined) | ||
| .then(async () => { | ||
| const before = | ||
| queryClient.getQueryData<LoadBalancerResource>( | ||
| appLoadBalancerKeys.detail(appName), | ||
| )?.config ?? {} | ||
| try { | ||
| await save.mutateAsync(apply(before)) |
There was a problem hiding this comment.
The queue orders suggestion applies, but form saves and Undo still send full-config PUTs outside it. If an operator saves the form while a suggestion is applying, whichever request finishes last can replace the other's settings. Clicking an earlier suggestion's Undo can also restore its old snapshot and discard later changes. These writes need the same ordering as suggestion applies.
Knowledge Base Used: Web console
| const all = [...history.values()].map((h) => h.series.latency_ms) | ||
| const len = Math.min(...all.map((s) => s.length)) | ||
| if (all.length === 0 || len < 2) return undefined |
There was a problem hiding this comment.
This takes the shortest latency series across every upstream. A new upstream, or one without active probe samples, can have fewer than two points, making the entire trend disappear even when other upstreams have enough history to show it.
| return Array.from({ length: len }, (_, i) => { | ||
| const vals = all.map((s) => s[s.length - len + i]?.value ?? 0) | ||
| return Math.round(vals.reduce((a, b) => a + b, 0) / vals.length) |
There was a problem hiding this comment.
The aggregate pairs samples by their position in each series, not by their available timestamps. When an upstream misses a probe, its remaining values can be averaged with values recorded at different times on other upstreams. The resulting trend can mislead operators about when latency changed.
1 participant
What this does
Fixes the open review findings from the load balancer page redesign (#721):
What this does not do
No backend or API changes. No change to the export generator output itself.
Verification
tsc -b, eslint and prettier on the touched files, vitest for components/loadbalancer and lib/loadBalancer (66 pass), new tests for retries 0, flap idempotence, shell quoting and share labels. Not verified: a live browser run, the full web test suite, or concurrent saves against a real server.