|
| 1 | +# Console SQL Shell: report server time alongside round-trip time |
| 2 | + |
| 3 | +- Associated: [CS-218](https://linear.app/materializeinc/issue/CS-218/console-response-time-metric-should-show-actual-database-time) |
| 4 | + |
| 5 | +## The Problem |
| 6 | + |
| 7 | +The Console SQL Shell footer reports a single number, `Returned in 148ms`. During |
| 8 | +demos this undersells Materialize: the number is dominated by network round trip |
| 9 | +and browser work, so a query that Materialize answered in about a millisecond |
| 10 | +presents as a hundred and change. |
| 11 | + |
| 12 | +The metric is not wrong. `CommandResult.tsx` computes it via |
| 13 | +`calculateCommandDuration` (`console/src/platform/shell/timings.ts`), which |
| 14 | +subtracts two client-side `performance.now()` stamps: `commandSentTimeMs`, taken |
| 15 | +on the shell FSM's `SEND` transition, and `endTimeMs`, taken in |
| 16 | +`completeCommandResult` when the FSM processes the completion frame. It therefore |
| 17 | +spans WebSocket send, network out, server work, network back, JSON parse, and FSM |
| 18 | +dispatch, plus any main-thread stall in between. The existing tooltip says so |
| 19 | +explicitly: |
| 20 | + |
| 21 | +> "The total time to submit, execute, receive, and render the query results over |
| 22 | +> the network." |
| 23 | +
|
| 24 | +So this is not a bug report. It is a request to surface a number the client |
| 25 | +cannot compute, because **the server never tells the client how long it spent**. |
| 26 | + |
| 27 | +## Success Criteria |
| 28 | + |
| 29 | +- A user running a query in the Shell can see how much of the elapsed time |
| 30 | + Materialize was responsible for, distinct from network and browser time. |
| 31 | +- The Shell does not overstate Materialize's speed. Whatever is shown must remain |
| 32 | + true when the server is slow, not only when it is fast. |
| 33 | +- No behaviour change for any existing WebSocket API client that does not ask for |
| 34 | + the new information. |
| 35 | +- The Shell continues to work, and to look deliberate, against environments that |
| 36 | + predate this change. |
| 37 | + |
| 38 | +## Out of Scope |
| 39 | + |
| 40 | +- **Streaming commands.** A `SUBSCRIBE` has no completion, and the Shell already |
| 41 | + shows no duration for one until it ends. That stays true. |
| 42 | +- **A latency breakdown.** Splitting server time into queueing, optimization and |
| 43 | + execution is deferred; see Follow-up work. |
| 44 | +- **The HTTP SQL API.** Parity is desirable but is not required to close CS-218, |
| 45 | + and bundling it widens the blast radius of a public-API change. |
| 46 | +- **Query Insights thresholds.** See Open questions. |
| 47 | + |
| 48 | +## Solution Proposal |
| 49 | + |
| 50 | +The server measures the interval from receiving a statement to having its |
| 51 | +response ready, and reports it to clients that opted in. The Shell renders both |
| 52 | +numbers: |
| 53 | + |
| 54 | +``` |
| 55 | +1.2ms server · 148ms total |
| 56 | +``` |
| 57 | + |
| 58 | +### What "server time" means, and why not "execution time" |
| 59 | + |
| 60 | +The statement lifecycle already defines five points |
| 61 | +(`src/adapter/src/statement_logging.rs`): `ExecutionBegan`, |
| 62 | +`OptimizationFinished`, `StorageDependenciesFinished`, |
| 63 | +`ComputeDependenciesFinished`, `ExecutionFinished`. Separately, |
| 64 | +`LifecycleTimestamps.received` (`src/adapter/src/session.rs`) marks arrival. |
| 65 | + |
| 66 | +The narrowest interval, `ExecutionBegan` to `ExecutionFinished`, produces the |
| 67 | +smallest and most flattering number. It is the wrong choice. The gap between |
| 68 | +*received* and *execution-began* is time queued behind the coordinator, which is |
| 69 | +a single serialized task per environment. In several 2026 incidents that gap was |
| 70 | +the dominant term: every query got slower while no individual query was slow. A |
| 71 | +metric that excluded it would display roughly a millisecond while a user waited |
| 72 | +seconds, making the Console actively misleading in precisely the situation where |
| 73 | +someone is looking at it to understand a problem. |
| 74 | + |
| 75 | +So the reported interval is **received to response-ready**, and it is labelled |
| 76 | +*server time* rather than *execution time*. The demo value survives: on a healthy |
| 77 | +system an indexed peek is still low single-digit milliseconds against a |
| 78 | +three-digit round trip, which is the point we want to make. |
| 79 | + |
| 80 | +### Why the statement-logging machinery cannot supply this |
| 81 | + |
| 82 | +The natural implementation is to reuse the lifecycle timestamps. It does not |
| 83 | +work. `record_statement_lifecycle_event` |
| 84 | +(`src/adapter/src/coord/statement_logging.rs`) requires a `StatementLoggingId`, |
| 85 | +which exists only for statements sampled into the statement log, and it is |
| 86 | +additionally gated on the `ENABLE_STATEMENT_LIFECYCLE_LOGGING` dyncfg. It is |
| 87 | +doubly conditional, and one production environment samples on the order of 0.06% |
| 88 | +of statements. A number that appears in the Shell on every query needs its own, |
| 89 | +unconditional path. |
| 90 | + |
| 91 | +### Clock |
| 92 | + |
| 93 | +The measurement uses a monotonic `std::time::Instant` taken at receipt and at |
| 94 | +response-ready, reported as microseconds. |
| 95 | + |
| 96 | +`EpochMillis` (`src/ore/src/now.rs`), which the adapter's `NowFn` returns, is |
| 97 | +unsuitable on two counts. It has millisecond granularity, so a fast peek renders |
| 98 | +as `1ms` or `0ms`, and `0ms server` reads as broken rather than fast. And it is |
| 99 | +wall-clock, so an NTP step can produce a negative duration. `Instant` is |
| 100 | +monotonic by construction and sub-millisecond, matching the client half, which |
| 101 | +already uses `performance.now()`. |
| 102 | + |
| 103 | +### Protocol |
| 104 | + |
| 105 | +Add a `WebSocketResponse` variant carrying the measurement, emitted immediately |
| 106 | +before `CommandComplete` for each statement. |
| 107 | + |
| 108 | +`WebSocketResponse` is adjacently tagged (`#[serde(tag = "type", content = |
| 109 | +"payload")]`), so a new variant is additive on the wire and cannot alter the |
| 110 | +encoding of existing messages. Widening `CommandComplete`'s payload from its |
| 111 | +current bare string to an object was rejected for the opposite reason: it would |
| 112 | +break every existing client. |
| 113 | + |
| 114 | +The new message is **opt-in**. The WebSocket handshake already accepts an |
| 115 | +`options` map of session variables (`src/environmentd/src/http/sql.rs`, |
| 116 | +documented in `doc/user/content/integrations/websocket-api.md`). A client that |
| 117 | +does not set the option receives a byte-identical stream to today. This matters |
| 118 | +because the WebSocket SQL API is a documented public interface whose message-type |
| 119 | +table is presented as exhaustive, with no clause telling clients to tolerate |
| 120 | +unknown types. Rather than assume third-party clients are permissive, we require |
| 121 | +them to ask. |
| 122 | + |
| 123 | +The Console opts in at connection time. It is safe against a server that ignores |
| 124 | +the option: its dispatch is a `switch` with no `default` arm wrapped in a |
| 125 | +`try/catch`, so unknown message types are already discarded silently. |
| 126 | + |
| 127 | +### Errors and streaming |
| 128 | + |
| 129 | +Failed statements report server time. The Shell already stamps completion on the |
| 130 | +error path (`addErrorDuringCommandInProgress` calls `completeCommandResult`) and |
| 131 | +renders the duration in red. A statement that fails after three seconds is |
| 132 | +exactly when the split between server and network matters. |
| 133 | + |
| 134 | +Streaming commands report nothing, matching today's behaviour. |
| 135 | + |
| 136 | +### Display |
| 137 | + |
| 138 | +``` |
| 139 | +1.2ms server · 148ms total ⓘ |
| 140 | +``` |
| 141 | + |
| 142 | +Against an environment that does not send the message, the Shell renders today's |
| 143 | +line unchanged: |
| 144 | + |
| 145 | +``` |
| 146 | +Returned in 148ms ⓘ |
| 147 | +``` |
| 148 | + |
| 149 | +The fallback is not an error state and must not look like one. The tooltip is |
| 150 | +rewritten for both cases: today's wording describes the total accurately and |
| 151 | +would become misleading next to a second number. |
| 152 | + |
| 153 | +### Implementation touch points |
| 154 | + |
| 155 | +Three CODEOWNERS scopes in one repository, so this stacks rather than requiring |
| 156 | +cross-team coordination: |
| 157 | + |
| 158 | +1. `@MaterializeInc/adapter` — the session option, the `Instant` pair on the |
| 159 | + response path, and the new `WebSocketResponse` variant. |
| 160 | +2. `@MaterializeInc/console` — opt in at connect, handle the message, prefer the |
| 161 | + server value with fallback, render both, rewrite the tooltip. Adds |
| 162 | + `timings.test.ts`; there is no test for that module today. |
| 163 | +3. `@MaterializeInc/docs` — the new message type in the WebSocket API reference, |
| 164 | + and a section in `doc/user/content/console/sql-shell.md`, which is 27 lines |
| 165 | + and does not currently mention the metric at all. |
| 166 | + |
| 167 | +Step 1 must land first. Steps 2 and 3 are independent of each other. |
| 168 | + |
| 169 | +## Minimal Viable Prototype |
| 170 | + |
| 171 | +A branch that hardcodes a plausible server time in the Console and renders the |
| 172 | +two-number footer, shared as a screenshot, is enough to settle the display |
| 173 | +question before any protocol work begins. The wording and ordering are the parts |
| 174 | +most likely to attract revision, and they are the cheapest to change early. |
| 175 | + |
| 176 | +## Alternatives |
| 177 | + |
| 178 | +**Replace the total with server time.** Best demo optic and closest to the |
| 179 | +literal ticket title. Rejected: for a browser talking to a remote environment, |
| 180 | +round-trip time is a real component of what the user experiences, and hiding it |
| 181 | +trades one form of inaccuracy for another. |
| 182 | + |
| 183 | +**Tighten the client-side measurement instead.** Move the stamps closer to the |
| 184 | +socket so JS work falls outside them. Cheap, one CODEOWNERS scope, no protocol |
| 185 | +change. Rejected as insufficient: it reduces noise but still measures a |
| 186 | +round trip, so it does not answer the question CS-218 asks. |
| 187 | + |
| 188 | +**Query `mz_recent_activity_log` after execution.** No protocol change. Rejected: |
| 189 | +statement logging is sampled and throttled, requires `mz_monitor`, and adds a |
| 190 | +round trip to display a latency number. |
| 191 | + |
| 192 | +**Carry the timing in a `Notice`.** No protocol change at all, and clients |
| 193 | +already tolerate arbitrary notices. Rejected: notices are user-facing |
| 194 | +informational messages, the Shell renders them into output, and every other |
| 195 | +client displaying notices would inherit timing noise. |
| 196 | + |
| 197 | +**Emit the new message unconditionally.** Simpler than an opt-in, and safe for |
| 198 | +any client that ignores unknown types. Rejected on public-API grounds: the |
| 199 | +documented type table reads as exhaustive, and we would be changing observable |
| 200 | +output for every consumer to serve one. |
| 201 | + |
| 202 | +## Open questions |
| 203 | + |
| 204 | +**Should Query Insights follow the new number?** |
| 205 | +`PlanInsightsNotice.tsx` uses the same `calculateCommandDuration` helper to |
| 206 | +decide when to surface Query Insights. If the displayed metric changes basis, the |
| 207 | +insights threshold silently moves with it. The recommendation is to pin Query |
| 208 | +Insights to round-trip time: a user who waited three seconds cares that they |
| 209 | +waited three seconds, wherever the time went. This should be an explicit choice |
| 210 | +rather than an inherited side effect. |
| 211 | + |
| 212 | +**Should the public WebSocket API gain a forward-compatibility clause?** |
| 213 | +`doc/user/content/integrations/websocket-api.md` enumerates message types with no |
| 214 | +statement that clients should ignore unrecognised ones. That absence is what |
| 215 | +forces this design to be opt-in. Adding such a clause would not help existing |
| 216 | +deployed clients, but it would let future additions be unconditional. Worth doing |
| 217 | +independently of this work. |
0 commit comments