Repository navigation
Conversation
25ad3fe to
2feb8b3
Compare
ea9e4b3 to
9a74524
Compare
9a74524 to
b54dacb
Compare
QA LLM Review1. MEDIUM -- Swap sizes get a "Disk" chart that plots swap
The disk chart and the tooltip's Disk row appear whenever Details
Fix: decide whether to show the chart and the tooltip's Disk row from the size's 2. LOW -- A transient null heap limit relabels the memory chart until the page remounts
DetailsThe orchestrator records an all-null row when the metrics API call fails ( |
b54dacb to
82652de
Compare
| const timelinesById = new Map<string, ReplicaTimeline>(); | ||
| if (!history.data) return timelinesById; | ||
| const { startDate, endDate } = history.data; | ||
| for (const { id, data } of history.data.graphData) { |
There was a problem hiding this comment.
Timelines are only built for replicas in graphData, i.e. ones that reported metrics in the window. A replica that's been offline the whole window has no samples, so it gets a toggle but no status row, though an offline stretch is exactly what the strip is for. If every replica is down, the card shows "No resource usage in this time period" and hides the strip entirely, which reads as idle, not broken. Status history already covers the current replicas, and transformReplicaTimeline already carries a pre-window offline transition across the window. Could the rows come from replicasById, and the empty state keep the status strip?
There was a problem hiding this comment.
Agreed. Rows now come from every replica, so one offline the whole window gets an offline row from status history, and a window with no usage keeps the status rows under the empty message. Both have tests.
There was a problem hiding this comment.
Follow-up: my premise here was incomplete. In cloud a down replica still writes an all-null metrics row every minute (get_metrics falls back to ServiceProcessMetrics::default() when the metrics API call fails). So when every replica is down for the whole window, the card takes the hasNoUtilizationMetrics branch. It shows the metrics-collection alert and no Replica Status. That's the QA bot's Oct 8 finding.
A missing metrics-server produces the same all-null rows, so the readings alone can't tell the two apart. Status history can, and it's already loaded. Proposal, for when every reading in the window is empty:
- Some replica was online in the window: something was running and wasn't measured. "Your replicas were running, but no resource usage was recorded for this period." Then the deployment-specific advice below.
- Every replica was offline the whole window, and the last offline reason was
oom-killed: "All replicas were offline for this period. They last went offline after running out of memory. See Troubleshoot memory usage." - Every replica was offline the whole window, with no reason recorded: "All replicas were offline for this period, and Materialize didn't record why. If they keep restarting, see Troubleshoot hydration failures."
- Status unknown (history failed to load, or has no rows): "No resource usage was recorded for this period. The replicas may have been offline, or metrics may be unavailable." Then the deployment-specific advice below.
Deployment-specific advice, by appConfig.mode:
- Cloud: "If this continues, contact support." Customers can't install or fix metrics-server in cloud, so the current advice isn't actionable there.
- Self-managed: "Check that metrics-server is installed in your Kubernetes cluster."
In every case, show the Replica Status section below the message.
The memory and hydration guides each need a one-line entry in mz-doc-urls.json, as #36479 did. Support is already there. I'd avoid "increase the cluster size" for OOMs, since the memory guide lists several causes where a resize isn't the fix.
There was a problem hiding this comment.
Thanks for the follow-up. Implemented your proposal in 2488ee4: when no sample in the window has a reading, the card picks the message from replica status (ran unmeasured, all offline after an OOM, all offline with no reason, or unknown), adds the cloud or self-managed advice, and always shows Replica Status below. The two guides are in mz-doc-urls.json.
d4e204d to
de5b9a5
Compare
|
Thanks. For 1, the Disk chart and the tooltip's Disk row are gone; the new page doesn't chart disk. For 2, the memory label now comes from the utilization rows: a window is labelled as a share of the memory allocation only when none of its samples has a heap limit. The heap limits query also keeps polling while a replica's latest limit is null, so the legend's byte sizes recover after a failed fetch. |
QA LLM Review1. MEDIUM -- A cluster whose replicas are all down still loses its status strip
The new status rows assume an offline replica reports no samples, but in cloud a provisioned replica that is down still writes an all-null metrics row every minute. So when every replica has been down for the whole window, for example a single-replica cluster stuck Pending or crashing at boot, DetailsWhy the rows exist: the per-replica metrics task runs every 60s for as long as the replica is provisioned and sends whatever the orchestrator returns ( The new To fix this, keep the status strip in the |
### Motivation Part of moving the Console to React 19. visx 3.12 caps React at 18 in its peer range. Main also mixes `@visx/grid` and `@visx/shape` 4.0 with the other visx packages on 3.12, which installs two copies of the shared visx internals. Supersedes #39496. ### Description - All `@visx/*` packages move to 4.0.0, deduped to one copy each. - visx 4 adds an `exports` map, so deep imports such as `@visx/tooltip/lib/hooks/useTooltipInPortal` and `@visx/responsive/lib/components/ParentSize` no longer resolve. They now import from the package roots. This includes the `vi.mock` in `ThresholdLineGraph.test.tsx`. - visx 4's `ParentSize` puts its children in an absolutely positioned box inside a `height: 100%` container, so it collapses to 0px tall when its parent has no fixed height. Two charts relied on the old behavior: - **Cluster overview utilization graphs:** they set their own height, so they now measure only their width with `useParentSize`. - **Source and sink error graph:** its `ParentSize` gets the graph's fixed height. The other five `ParentSize` call sites are already in fixed-height boxes. The redesigned cluster page's resource usage chart (#39508) has the same width-only `ParentSize`, so whichever PR lands second needs this change there. ### Verification Existing tests. jsdom doesn't do layout, so the tests can't catch the collapse. In headless Chromium, the utilization graph renders 0px tall on visx 4 without this change and at full height with it. Worth eyeballing on the preview: - the cluster overview CPU, memory and disk graphs; - a source's and a sink's error graph; - the freshness graph; - the billing spend chart. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
QA LLM Review1. HIGH -- On visx 4 the resource usage card renders only its header
The base now includes visx 4, whose DetailsThe I rendered the real component tree in headless Chromium, with Chakra styles and visx 4's real The tests mock Fix: measure only the width, as #39611 did for - <ParentSize debounceTime={10} style={{ width: "100%", minWidth: 0 }}>
- {({ width }) =>
- width > 0 && (
- <ResourceUsageCharts
+ <Box ref={parentRef} width="100%" minWidth={0}>
+ {width > 0 && (
+ <ResourceUsageChartsCall |
jdonelson
left a comment
There was a problem hiding this comment.
On the memory graph, I would add a space or a tick mark or some way to indicate that each pair represents one point in time. As it is, it's not clear that each pair represents one single point in time.
At one point I think you had overlapping bars? I liked that as a solution.
8d2ede6 to
7ea4cc0
Compare
e81d5d4 to
5e350cc
Compare
5e350cc to
6bc0626
Compare
6bc0626 to
2a22188
Compare
2a22188 to
2488ee4
Compare
|
Thanks. The all-down case is fixed in 2488ee4 along the lines of the review thread: empty readings no longer hide Replica Status. The memory chart now overlaps each time's replica bars by half a bar, with a surface ring between them, so each group reads as one point in time. |
2488ee4 to
7a0f846
Compare
7a0f846 to
69e9986
Compare
jdonelson
left a comment
There was a problem hiding this comment.
I think the current bar chart is struggling to display processes per replica. It worked well with 2 or 3 plots (1 per replica) but for hefty cluster with 2 replicas and 8 processes each it starts to lose visual fidelity. The colors and line styles will cycle and different processes will have the same color/style. And as the bars increase in number, we won't be able to fit them all in the bounds and the time window will have to shrink.
rendered example:
Some possible alternatives could be:
- Display only the max process size per replica.
- Display the max, min, and average using bounded range or candlestick graphs. Or the average could be a line while the max is shown via bars.
- Drill-downs or toggles so that we're not showing everything at once.
Here are illustrations of these ideas along with some others:
https://data.europa.eu/apps/data-visualisation-guide/differences-in-time-series
https://observablehq.com/@observablehq/whats-different-timeseries-forecast-performance?collection=@observablehq/analyzing-time-series-data
("Perceiving the vertical distance or the shortest path?" section)
203c05e to
97778e6
Compare
QA LLM Review1. MEDIUM -- Clusters with
|
97778e6 to
0bb33c9
Compare
|
Agreed, thanks. The card charts one series per replica again, and per-process detail moves to the workers drawer in #39889. |
|
Thanks, fixed in 0bb33c9: bucket merging and bar slots count only the replicas reporting at the same time, and replicas that share a name share one toggle, color and status row. |
Third in the cluster details redesign stack. The Metrics tab gets a resource usage card: CPU, memory and replica status share one time axis and one hover tooltip, with a toggle per replica. - CPU: a line per replica, as a share of the replica's CPU allocation. Readings past 100% extend the axis, with a dashed line marking the allocation. - Memory: a bar per replica as a share of the heap limit (RAM plus swap), on a fixed 0 to 100% axis so swap-heavy sizes don't stretch it. On 26.44 and later the bar splits into RAM and swap, and the legend gives the RAM and heap limits in bytes. Earlier environments show heap alone. Where no sample has a heap limit, the labels say the bars are a share of the memory allocation. - Replica status: a row per replica, including one offline the whole window. Running, finished hydration and offline stretches, a marker for each out-of-memory kill, and a badge with the count of objects still hydrating. A window with no usage still shows the status rows. - Dotted markers where an index or materialized view was created on the cluster. - Replicas that share a name, such as a scheduled cluster's refreshes, share one toggle, color and status row. A time's memory bars split only among the replicas reporting then. - Processes: when a replica's size runs more than one process, a Processes button on the CPU chart opens a drawer with a tab per such replica. It compares the busiest process with the average one for CPU and memory, as shares of one process's allocation, over the card's window up to 24h. It reads each process's samples from a SUBSCRIBE that is open only while the drawer is. Wide windows merge neighboring buckets, keeping each metric's peak, so bars stay at least 10px wide for as many replicas as report at once. The time axis matches the classic graphs. The mark layers are memoized, so hovering only redraws the cursor and tooltip.
0bb33c9 to
e0e0670
Compare
|
Correction to my last note: per-process detail now lives in a Processes drawer on this card, opened from the CPU chart for multi-process sizes. It compares each replica's busiest process with the average one, in e0e0670. |
QA LLM Review1. MEDIUM -- The tooltip lists every same-name replica at every hovered time
DetailsOn the PR's own scheduled-cluster fixture (27 refreshes, 7-day window), hovering inside refresh 10 produced a tooltip with 27 "r1 100cc Running" headers and one set of CPU/Memory/Swap rows. As a positive control, skipping members that have no point and aren't the group's representative brought it down to 2 headers. Before this commit, each dropped replica's timeline covered only its own lifetime, so 2. MEDIUM -- A past blue-green deployment's replicas merge with the current ones and inherit their OOMs
DetailsProbe: I gave the current
3. MEDIUM -- The scheduled-cluster test fails one hour in three
"charts a scheduled cluster's refreshes as one replica" depends on the wall clock. It fails whenever the window's first bucket falls on a UTC hour divisible by 3, which is 8 hours of every day, so it will flake on main and on unrelated console PRs. DetailsAt the mocked 900px width the plot is 828px, which gives 4.9px per hour. With one bar at a time, I ran it in a clean copy at 20:34Z and it failed, with all 27 paths single points. Shifting Pinning the clock ( |
Motivation
Third PR in the cluster details redesign stack, on top of #39507. It replaces the Metrics tab's overview charts with the resource usage card from the design. CPU, memory and replica status share one time axis and one tooltip, so a spike, an out-of-memory kill and the restart after it line up. Fixes CNS-174
Description
useChartLegend's toggle now takes any click event, since it only reads the modifier keys.useClusterObjectsLive, so DDL elsewhere in the environment doesn't redraw the charts. The cluster page mounts the objects collection bridge, as Object Explorer does.mz_cluster_replica_metrics_history(a lookup on itsreplica_idindex) through a SUBSCRIBE that is open only while the drawer is, starting at console: add data for the redesigned cluster page's resource usage #39507's rolling literal start.Loading and deriving live in
useResourceUsageData. The chart marks are memoized, so hovering only redraws the cursor and tooltip.The classic page's check for a cluster reporting no metrics moves into
hasNoUtilizationMetrics, which both pages now use. Its behavior is unchanged.Differences from the classic graphs: the CPU axis extends past 100% instead of clamping, there's no disk chart, replica toggles replace the replica dropdown, and windows of 3h or less don't poll offline events.
Not yet checked in Safari or Firefox.
Verification
Verified using teleport to customer environments
ResourceUsage.test.tsxrenders the card against MSW handlers and covers:r1, a line per refresh with more than one point, and bars as wide as their own bucket;processSpreadModel.test.tscovers each bucket's busiest process and the average of the processes that reported, leaving out a process whose fetch failed, and memory against the heap limit with the allocation fallback. The drawer test answers its SUBSCRIBE through a newmockSubscribeSockethelper, which sends a completed snapshot over the WebSocket.ClusterMetrics.test.tsxcovers the error shown when the clusters subscribe fails before its first snapshot.ClusterDetail.test.tsxmocks the new Metrics tab.🤖 Generated with Claude Code