Caused by [https://github.com/MaterializeInc/materialize/pull/37407:](<https://github.com/MaterializeInc/materialize/pull/37407:>) 1. Tier switch → chart spins forever. Both subscribe hooks mount up front; the inactive one connects its websocket with an undefined request, so the socket opens and passes ReadyForQuery idle. Switching tiers enables that hook (request undefined → defined), but useAutomaticallyConnectSocket calls setRequest and then returns without reconnect() when previousRequest === undefined. Nothing re-triggers ReadyForQuery, so the query is never sent, snapshotComplete never flips, and the spinner never resolves. (It also leaks the old subscription on disable.) 2. "Last hour" / "Last 3 hours" hide crashes. The new <=3h path reads the un-binned 3h view, which has no status columns, and the client binner hardcodes offlineEvents: null. So the two windows you'd reach for during an incident silently omit replica offline/OOM events — while the old ad-hoc path (still used for >14d) joins mz_cluster_replica_status_history and shows them. 3. "Last 6 hours" keeps growing. The 6h→24h path subscribes with a bucket_start >= minDate bound computed once at socket open, the view retains 24h, and the render path never re-clips the streamed rows to the current window (the axis widens to the oldest bucket). So the window keeps accreting old buckets, ~7h after one hour, approaching 24h over a long session, while still labeled "Last 6 hours". The >=3h path avoids this by clipping samples to the window start. Some extra tests: ```diff diff --git a/console/src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts b/console/src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts index 30c495130d..aafc73d7e4 100644 --- a/console/src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts +++ b/console/src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts @@ -7,6 +7,7 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. +import { subMinutes } from "date-fns"; import { CompiledQuery } from "kysely"; import { SEARCH_PATH } from "~/api/materialize"; @@ -17,6 +18,13 @@ import { } from "~/test/sql/materializeSqlClient"; import { testdrive } from "~/test/sql/mzcompose"; +import { + bucketRowsToBucketsByReplicaId, + rebucketUtilizationSamples, + toReplicaUtilizationGraphData, + UtilizationBucketRow, + UtilizationSample, +} from "./replicaUtilizationBinning"; import { buildConsoleClusterUtilizationOverviewQuery, buildConsoleClusterUtilizationUnbinned3hQuery, @@ -507,4 +515,188 @@ describe("console cluster utilization indexed views", () => { "2030-01-01T01:00:00.000Z", ); }); + + // Regression test: the ≤3h "Last hour"/"Last 3 hours" tier reads `_overview_3h` + // (utilization only, no status columns) and `rebucketUtilizationSamples` hardcodes + // `offlineEvents: null`, so crashes/OOMs vanish from the chart. The ad-hoc query + // (still used for >14d) joins `mz_cluster_replica_status_history` and surfaces them. + // FAILS until the bug is fixed: the ≤3h path should surface the OOM too. + it("surfaces OOM/offline events in the ≤3h unbinned path", async () => { + const client = await getMaterializeClient(); + + await testdrive(` + > CREATE CLUSTER c REPLICAS (r1 (SIZE 'scale=1,workers=1')); + > CREATE SCHEMA IF NOT EXISTS internal_test; + > SET schema = internal_test; + > CREATE TABLE mz_cluster_replica_metrics_history ( + occurred_at TIMESTAMP NOT NULL, replica_id TEXT NOT NULL, process_id uint8 NOT NULL, + cpu_nano_cores double, memory_bytes double, disk_bytes double, heap_bytes double, heap_limit double); + > CREATE TABLE mz_cluster_replica_status_history ( + replica_id TEXT NOT NULL, process_id uint8 NOT NULL, occurred_at TIMESTAMP NOT NULL, + status TEXT NOT NULL, reason TEXT); + > CREATE TABLE mz_cluster_replica_sizes ( + size TEXT NOT NULL, processes uint8 NOT NULL, cpu_nano_cores uint8 NOT NULL, + memory_bytes uint8 NOT NULL, disk_bytes uint8); + > INSERT INTO internal_test.mz_cluster_replica_sizes VALUES + ('scale=1,workers=1', 1, ${size25cc.cpuNanoCores}, ${size25cc.memoryBytes}, ${size25cc.diskBytes}); + # The un-binned 3h view has no status/offline columns. + > CREATE TABLE mz_console_cluster_utilization_overview_3h ( + replica_id TEXT, cluster_id TEXT, size TEXT, name TEXT, occurred_at TIMESTAMPTZ, cpu_percent DOUBLE, + memory_percent DOUBLE, disk_percent DOUBLE, heap_percent DOUBLE, memory_and_disk_percent DOUBLE); + `); + + await client.query(`SET search_path TO ${mockedSearchPath};`); + + const { + rows: [cluster], + } = await client.query("select id from mz_clusters where name = 'c'"); + const { + rows: [replica], + } = await client.query( + `SELECT id, name FROM mz_cluster_replicas WHERE cluster_id = '${cluster.id}' ORDER BY id`, + ); + + const startTime = "2030-01-01T00:00:00Z"; + const ts = "2030-01-01T00:00:30.000Z"; + + // A utilization sample and a coincident OOM for the same replica and bucket. + await client.query(`INSERT INTO internal_test.mz_cluster_replica_metrics_history VALUES + (TIMESTAMP '${ts}', '${replica.id}', 0, 5789441, 46788608, 937984, NULL, NULL)`); + await client.query(`INSERT INTO internal_test.mz_cluster_replica_status_history VALUES + ('${replica.id}', 0, TIMESTAMP '${ts}', 'offline', 'oom-killed')`); + await client.query(`INSERT INTO internal_test.mz_console_cluster_utilization_overview_3h VALUES + ('${replica.id}', '${cluster.id}', 'scale=1,workers=1', '${replica.name}', '${ts}', 0.1, 0.2, 0.3, 0.4, 0.5)`); + + // Ad-hoc path surfaces the OOM. + const adHoc = ( + await run( + buildReplicaUtilizationHistoryQuery({ + startDate: startTime, + bucketSizeMs: 60_000, + clusterIds: [cluster.id], + }).compile(), + ) + ).rows; + expect(adHoc.find((r) => r.offlineEvents)?.offlineEvents).toEqual([ + { + replicaId: replica.id, + reason: "oom-killed", + status: "offline", + occurredAt: "2030-01-01 00:00:30", + }, + ]); + + // Un-binned path (what `fetchReplicaUtilizationHistory` runs for "unbinned3h"): + // utilization survives but the OOM does not. + const samples = ( + await run( + buildConsoleClusterUtilizationUnbinned3hQuery({ + clusterIds: [cluster.id], + }).compile(), + ) + ).rows; + const rows = rebucketUtilizationSamples( + samples, + 60_000, + new Date(startTime).getTime(), + ); + expect(rows.length).toBeGreaterThan(0); + // The OOM the ad-hoc path found must also appear on this tier. + const events = rows.flatMap((r) => r.offlineEvents ?? []); + expect(events).toContainEqual( + expect.objectContaining({ status: "offline", reason: "oom-killed" }), + ); + }); + + // Regression test: the 3h..24h "Last 6 hours" tier SUBSCRIBEs with a `minDate` fixed + // at socket open (`buildConsoleClusterUtilizationOverview24hSubscribe` embeds a + // constant `bucket_start >= minDate`), and the binned render never re-clips the + // streamed rows to the current window, so old buckets linger and the axis widens past + // 6h toward 24h. + // FAILS until the bug is fixed: the window should stay clipped to the last 6h. + it("keeps the 'Last 6 hours' window clipped to 6h as the page ages", async () => { + const client = await getMaterializeClient(); + + await testdrive(` + > CREATE SCHEMA IF NOT EXISTS internal_test; + > SET schema = internal_test; + > DROP TABLE IF EXISTS mz_console_cluster_utilization_overview_24h; + > DROP TABLE IF EXISTS mz_cluster_deployment_lineage; + > CREATE TABLE mz_console_cluster_utilization_overview_24h ( + bucket_start TIMESTAMPTZ, bucket_end TIMESTAMPTZ, replica_id TEXT, cluster_id TEXT, size TEXT, name TEXT, + memory_percent DOUBLE, max_memory_at TIMESTAMPTZ, disk_percent DOUBLE, max_disk_at TIMESTAMPTZ, + max_cpu_percent DOUBLE, max_cpu_at TIMESTAMPTZ, heap_percent DOUBLE, max_heap_at TIMESTAMPTZ, + memory_and_disk_percent DOUBLE, max_memory_and_disk_memory_percent DOUBLE, + max_memory_and_disk_disk_percent DOUBLE, max_memory_and_disk_at TIMESTAMPTZ, offline_events TEXT); + > CREATE TABLE mz_cluster_deployment_lineage (cluster_id TEXT, current_deployment_cluster_id TEXT, cluster_name TEXT); + > INSERT INTO internal_test.mz_cluster_deployment_lineage VALUES ('u1', 'u1', 'test'); + # 5-minute buckets at 02:00, 04:00, 08:00 for one replica. + > INSERT INTO internal_test.mz_console_cluster_utilization_overview_24h VALUES + ('2030-01-01T02:00:00Z','2030-01-01T02:05:00Z','r1','u1','small','r1',0.4,'2030-01-01T02:00:00Z',0.3,'2030-01-01T02:00:00Z',0.5,'2030-01-01T02:00:00Z',0.2,'2030-01-01T02:00:00Z',0.6,0.4,0.3,'2030-01-01T02:00:00Z',NULL), + ('2030-01-01T04:00:00Z','2030-01-01T04:05:00Z','r1','u1','small','r1',0.4,'2030-01-01T04:00:00Z',0.3,'2030-01-01T04:00:00Z',0.5,'2030-01-01T04:00:00Z',0.2,'2030-01-01T04:00:00Z',0.6,0.4,0.3,'2030-01-01T04:00:00Z',NULL), + ('2030-01-01T08:00:00Z','2030-01-01T08:05:00Z','r1','u1','small','r1',0.4,'2030-01-01T08:00:00Z',0.3,'2030-01-01T08:00:00Z',0.5,'2030-01-01T08:00:00Z',0.2,'2030-01-01T08:00:00Z',0.6,0.4,0.3,'2030-01-01T08:00:00Z',NULL); + `); + + await client.query(`SET search_path TO ${mockedSearchPath};`); + + // Socket opens at 09:00 for "Last 6 hours" -> minDate = 03:00. This is the query the + // subscribe streams: only the fixed lower bound, no advancing/upper bound. + const minDate = subMinutes(new Date("2030-01-01T09:00:00Z"), 360); // 03:00 + const subscribedRows = ( + await run( + buildConsoleClusterUtilizationOverviewQuery({ + view: "mz_console_cluster_utilization_overview_24h", + clusterIds: ["u1"], + resolveLineage: true, + startDate: minDate.toISOString(), + }).compile(), + ) + ).rows as UtilizationBucketRow[]; + // The fixed lower bound drops 02:00 but keeps 04:00 and 08:00. + expect(subscribedRows.map((r) => r.bucketStart.toISOString())).toEqual([ + "2030-01-01T04:00:00.000Z", + "2030-01-01T08:00:00.000Z", + ]); + + // Three hours later the window is [06:00, 12:00], but the render never re-clips, so + // the axis widens back to the stale 04:00 bucket: a "6h" chart spanning 8h. + const renderNow = new Date("2030-01-01T12:00:00Z"); + const startDate = subMinutes(renderNow, 360); // 06:00 + const shaped = toReplicaUtilizationGraphData( + bucketRowsToBucketsByReplicaId([...subscribedRows]), + startDate, + renderNow, + ); + // The chart must not extend before the current 6h window start (06:00), and the + // now-8h-old 04:00 bucket must not be plotted. + expect(shaped.startDate.getTime()).toBeGreaterThanOrEqual( + startDate.getTime(), + ); + expect(shaped.graphData[0].data.map((d) => d.bucketStart)).not.toContain( + new Date("2030-01-01T04:00:00Z").getTime(), + ); + + // The un-binned path already clips to the window start; that is the filtering the + // binned path is missing. + const mkSample = (occurredAt: string): UtilizationSample => ({ + replicaId: "r1", + clusterId: "u1", + size: "small", + name: "r1", + occurredAt: new Date(occurredAt), + cpuPercent: 0.5, + memoryPercent: 0.4, + diskPercent: 0.3, + heapPercent: 0.2, + memoryAndDiskPercent: 0.6, + }); + const clipped = rebucketUtilizationSamples( + [mkSample("2030-01-01T04:00:00Z"), mkSample("2030-01-01T08:00:00Z")], + 5 * 60_000, + startDate.getTime(), + ); + expect(clipped.map((r) => r.bucketStart.toISOString())).toEqual([ + "2030-01-01T08:00:00.000Z", + ]); + }); }); diff --git a/console/src/hooks/useAutomaticallyConnectSocket.test.ts b/console/src/hooks/useAutomaticallyConnectSocket.test.ts new file mode 100644 index 0000000000..8222de7d48 --- /dev/null +++ b/console/src/hooks/useAutomaticallyConnectSocket.test.ts @@ -0,0 +1,113 @@ +// Copyright Materialize, Inc. and contributors. All rights reserved. +// +// Use of this software is governed by the Business Source License +// included in the LICENSE file. +// +// As of the Change Date specified in that file, in accordance with +// the Business Source License, use of this software will be governed +// by the Apache License, Version 2.0. + +import { renderHook } from "@testing-library/react"; + +import type { SubscribeManager } from "~/api/materialize/SubscribeManager"; +import type { SqlRequest } from "~/api/materialize/types"; +import { + type Connectable, + WebsocketConnectionManager, +} from "~/api/materialize/WebsocketConnectionManager"; + +import { useAutomaticallyConnectSocket } from "./useAutomaticallyConnectSocket"; + +function mkRequest(query: string): SqlRequest { + return { queries: [{ query, params: [] }] }; +} + +// A Connectable + SubscribeManager stub. The manager registers socket-event +// callbacks on construction and calls `setRequest` on request changes; none of these +// open a real socket in the test. +function mkStub() { + return { + reconnect: vi.fn(), + disconnect: vi.fn(), + isConnected: vi.fn(() => false), + registerOnClose: vi.fn(() => () => {}), + registerOnOpen: vi.fn(() => () => {}), + setRequest: vi.fn(), + }; +} + +function renderConnectHook(initialRequest: SqlRequest | undefined) { + const stub = mkStub(); + const utils = renderHook( + ({ request }: { request: SqlRequest | undefined }) => + useAutomaticallyConnectSocket({ + target: stub as unknown as Connectable, + subscribe: stub as unknown as SubscribeManager<object, unknown>, + request, + }), + { initialProps: { request: initialRequest } }, + ); + return { ...utils, stub }; +} + +describe("useAutomaticallyConnectSocket", () => { + let reconnectSpy: ReturnType<typeof vi.spyOn>; + + beforeEach(() => { + // The environment is never healthy in this test, so the real manager never opens a + // socket on its own (attemptConnection bails without an http address). The only + // reconnect() calls therefore come from the hook's request-change effect. Spy so + // we can observe them without a real websocket. + reconnectSpy = vi + .spyOn(WebsocketConnectionManager.prototype, "reconnect") + .mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + // Regression test for the tier-switch infinite spinner. A subscribe hook mounts + // disabled (request === undefined): the manager opens the socket, which passes + // ReadyForQuery idle. When the hook becomes enabled the request goes undefined -> + // defined, but `useAutomaticallyConnectSocket` calls `setRequest` and then returns + // before `reconnect()` (`previousRequest === undefined`), so the idle socket never + // re-runs ReadyForQuery, never sends the query, and the chart spins forever. + // FAILS until the bug is fixed: undefined -> defined must dispatch the request. + it("dispatches the request when it changes from undefined to defined", () => { + const { rerender, stub } = renderConnectHook(undefined); + + // Idle socket: nothing installed or reconnected yet. + expect(stub.setRequest).not.toHaveBeenCalled(); + expect(reconnectSpy).not.toHaveBeenCalled(); + + // The tier switch enables the hook: the request becomes defined. + const request = mkRequest( + "SUBSCRIBE (SELECT 1) WITH (PROGRESS) ENVELOPE UPSERT", + ); + rerender({ request }); + + // The request is installed, and it must also be dispatched (reconnect) so the idle + // socket re-runs ReadyForQuery and sends it. + expect(stub.setRequest).toHaveBeenCalledWith(request); + expect(reconnectSpy).toHaveBeenCalled(); + }); + + // Control: a defined -> different-defined change DOES reconnect. This is the working + // path and shows the bug above is specific to the undefined -> defined edge. + it("reconnects when the request changes between two defined values", () => { + const requestA = mkRequest("SUBSCRIBE a"); + const { rerender, stub } = renderConnectHook(requestA); + + // The first defined request is installed, but the manager owns the initial + // connect, so no explicit reconnect happens yet. + expect(stub.setRequest).toHaveBeenCalledWith(requestA); + expect(reconnectSpy).not.toHaveBeenCalled(); + + const requestB = mkRequest("SUBSCRIBE b"); + rerender({ request: requestB }); + + expect(stub.setRequest).toHaveBeenCalledWith(requestB); + expect(reconnectSpy).toHaveBeenCalledTimes(1); + }); +}); ``` Running `corepack yarn test src/hooks/useAutomaticallyConnectSocket.test.ts --run`: ``` ❯ src/hooks/useAutomaticallyConnectSocket.test.ts (2 tests | 1 failed) 22ms × useAutomaticallyConnectSocket > dispatches the request when it changes from undefined to defined 17ms → expected "reconnect" to be called at least once ✓ useAutomaticallyConnectSocket > reconnects when the request changes between two defined values 3ms ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ FAIL src/hooks/useAutomaticallyConnectSocket.test.ts > useAutomaticallyConnectSocket > dispatches the request when it changes from undefined to defined AssertionError: expected "reconnect" to be called at least once ❯ src/hooks/useAutomaticallyConnectSocket.test.ts:93:26 91| // socket re-runs ReadyForQuery and sends it. 92| expect(stub.setRequest).toHaveBeenCalledWith(request); 93| expect(reconnectSpy).toHaveBeenCalled(); | ^ 94| }); 95| ``` And `corepack yarn test:sql src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts --run`: ``` ❯ src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts (5 tests | 2 failed) 6391ms ✓ replicaUtilizationHistory > buckets cluster metrics 1906ms ✓ console cluster utilization indexed views > buildConsoleClusterUtilizationUnbinned3hQuery filters by cluster and optionally expands blue-green lineage 1179ms ✓ console cluster utilization indexed views > buildConsoleClusterUtilizationOverviewQuery reads the 24h view, filters by cluster, and clips by startDate 947ms × console cluster utilization indexed views > surfaces OOM/offline events in the ≤3h unbinned path 1196ms → expected [] to deep equally contain ObjectContaining{…} × console cluster utilization indexed views > keeps the 'Last 6 hours' window clipped to 6h as the page ages 1163ms → expected 1893470400000 to be greater than or equal to 1893477600000 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ FAIL src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts > console cluster utilization indexed views > surfaces OOM/offline events in the ≤3h unbinned path AssertionError: expected [] to deep equally contain ObjectContaining{…} - Expected: ObjectContaining { "reason": "oom-killed", "status": "offline", } + Received: [] ❯ src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts:606:20 604| // The OOM the ad-hoc path found must also appear on this tier. 605| const events = rows.flatMap((r) => r.offlineEvents ?? []); 606| expect(events).toContainEqual( | ^ 607| expect.objectContaining({ status: "offline", reason: "oom-killed" }), 608| ); ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯ FAIL src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts > console cluster utilization indexed views > keeps the 'Last 6 hours' window clipped to 6h as the page ages AssertionError: expected 1893470400000 to be greater than or equal to 1893477600000 ❯ src/api/materialize/cluster/replicaUtilizationHistory.test.sql.ts:672:40 670| // The chart must not extend before the current 6h window start (06:00), and the 671| // now-8h-old 04:00 bucket must not be plotted. 672| expect(shaped.startDate.getTime()).toBeGreaterThanOrEqual( | ^ 673| startDate.getTime(), 674| ); ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯ Test Files 1 failed (1) Tests 2 failed | 3 passed (5) Start at 12:17:12 Duration 9.79s (transform 689ms, setup 30ms, collect 1.71s, tests 6.39s, environment 264ms, prepare 44ms) ``` Found via the QA LLM review, sorry for only reporting after the PR merged: [https://github.com/MaterializeInc/qa-llm-review/blob/master/commit-bugs/done/analysis-commit-b7f845fe14744f97a50cc0f23847b16c66ae118f.md](<https://github.com/MaterializeInc/qa-llm-review/blob/master/commit-bugs/done/analysis-commit-b7f845fe14744f97a50cc0f23847b16c66ae118f.md>)