mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
chore(studio): address review comments on Multigres topology diagram (#49592)
<!-- ccr-slack-attribution --> _Requested by **Alaister Young** · [Slack thread](https://supabase.slack.com/archives/C0161K73J1J/p1787738517181409?thread_ts=1787635785.354489&cid=C0161K73J1J)_ Follow-up to #49298, which was squash-merged before @joshenlim's last review round was addressed. Picking up the review comments here. ## I have read the [CONTRIBUTING.md](https://github.com/supabase/supabase/blob/master/CONTRIBUTING.md) file. YES ## What kind of change does this PR introduce? Chore — dead code removal and comment corrections. No behavior change. ## What is the current behavior? Three of @joshenlim's review comments on #49298 are still open on master: - [Dead code in `ha-cluster-cells-query.ts`](https://github.com/supabase/supabase/pull/49298#discussion_r3861029414) — "seems to be dead code? no one's consuming this file" - [Dead code in `ha-cluster-databases-query.ts`](https://github.com/supabase/supabase/pull/49298#discussion_r3861031230) — "likewise - seems to be dead code" - [`STATUS_BADGE_VARIANTS` statuses](https://github.com/supabase/supabase/pull/49298#discussion_r3861095281) — "just to sanity check these are the only statuses? are there any failure states? e.g 'Failed'" Concretely, on master today: - `apps/studio/data/ha-admin/ha-cluster-cells-query.ts` and `apps/studio/data/ha-admin/ha-cluster-databases-query.ts` ship query options that nothing imports. The diagram only reads `/poolers` and `/gateways`. - `multipoolerSchema.lifecycleStatus` is an undocumented `z.string()`, while its neighbours `type` and `servingStatus` both list their expected proto values in a comment. - The comment above `HA_POOLER_STATUS_LABELS` claims the labels "Matches the status vocabulary of the read replica surfaces (getStatusLabel)". They don't — `getStatusLabel` in `ReadReplicas/ReadReplicas.utils.ts` also returns `Failed`, `Restarting`, `Resizing` and `Restoring`, none of which the HA labels have. ## What is the new behavior? - Deleted both dead query modules and pruned the orphaned `cells` and `databases` factories from `haAdminKeys`, keeping `poolers` and `gateways`. Verified by grep that neither file name nor any of their exported symbols (`haClusterCellsQueryOptions`, `HaClusterCellsData`, `haClusterDatabasesQueryOptions`, `HaClusterDatabasesData`, the `*Variables`/`*Error` types) nor `haAdminKeys.cells` / `haAdminKeys.databases` has a single reference left anywhere outside the deleted files. No re-export shims left behind. `get-ha-admin.ts` stays — `poolers` and `gateways` still use it. - Documented `lifecycleStatus` against the actual enum, `PoolerLifecycleStatus` in [multigres `proto/clustermetadata.proto`](https://github.com/multigres/multigres/blob/main/proto/clustermetadata.proto): `LIFECYCLE_UNKNOWN` (zero value, omitted from JSON) | `STARTING` | `ACTIVE` | `STOPPING` | `SHUTDOWN` | `QUARANTINED`. `getPoolerStatus` already maps every member. - Reworded the `HA_POOLER_STATUS_LABELS` comment to say the labels are a subset drawn from the read replica vocabulary rather than a match for it, and noted where the read replica `Failed` lands on the HA side. **On the `Failed` question:** the answer from the proto is that there is no dedicated failure member. The terminal states are `QUARANTINED` — the pooler "has given up trying to become a healthy replica: it cannot automatically recover to a functioning state (e.g. it could not complete a pg_rewind, could not restore from backup to start postgres, or fell irrecoverably behind on replication)", kept alive for forensics — and `SHUTDOWN`, "durably down". Both already map to `unhealthy` / the `Unhealthy` warning badge, so the four statuses on `STATUS_BADGE_VARIANTS` are complete for the enum as it stands. If we'd rather show `QUARANTINED` as its own `Failed` status with a destructive badge (matching the read replica surface), that's a small follow-up — a product/copy call rather than a gap, so not folded in here. **Not included: [the replication page UX comment](https://github.com/supabase/supabase/pull/49298#discussion_r3861055216)** ("is there any other content we plan to add here? it feels empty atm... it's just a repeat of the home page + settings/infrastructure"). @joshenlim flagged that one himself as "UX feedback which can be addressed separately". It's a product and IA question about what that page is for, not something to answer with a code change here — leaving it for @alaister and design. ## Additional context - Verified locally: `tsc --noEmit` (0 errors), ESLint on the touched files (clean), Prettier check (clean), and `HaTopology.utils.test.ts` + `HaInstanceConfiguration.utils.test.ts` (26/26 passing). CI is green as well. - Exhaustive grep across the repo (excluding `node_modules`/`.git`/build output, covering `apps/**` incl. `lite-studio`, `packages/**` and `e2e/**`) confirmed zero remaining references to the deleted files, their exported symbols, and the removed key factories. - No test changes: the diff deletes unreferenced code and edits comments only, so there's no new behavior to cover. `HaTopology.utils.test.ts` already pins every `lifecycleStatus` value listed in the new comment. --- _Generated by [Claude Code](https://claude.ai/code/session_012StGVQSmPzpGTXrduo9Xyi)_ --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Alaister Young <10985857+alaister@users.noreply.github.com>
This commit is contained in:
5 files changed
+17
-61
No files matched your search
+8
-1
@@ -54,6 +54,8 @@ export const getPoolerStatus = (pooler: Multipooler): HaPoolerStatus => {
|
||||
// Lifecycle values may arrive with or without the proto enum prefix.
|
||||
const lifecycle = (pooler.lifecycleStatus?.status ?? '').replace(/^LIFECYCLE_/, '')
|
||||
|
||||
// The proto's two terminal states: QUARANTINED (gave up recovering, kept alive
|
||||
// for forensics) and SHUTDOWN (durably down). There is no separate FAILED state.
|
||||
if (lifecycle === 'QUARANTINED' || lifecycle === 'SHUTDOWN') return 'unhealthy'
|
||||
if (lifecycle === 'STARTING') return 'coming_up'
|
||||
if (lifecycle === 'STOPPING') return 'going_down'
|
||||
@@ -66,7 +68,12 @@ export const getPoolerStatus = (pooler: Multipooler): HaPoolerStatus => {
|
||||
return 'healthy'
|
||||
}
|
||||
|
||||
// Matches the status vocabulary of the read replica surfaces (getStatusLabel).
|
||||
// Labels are drawn from the read replica status vocabulary (`getStatusLabel` in
|
||||
// ReadReplicas.utils.ts) so both surfaces read the same way, but they are only a
|
||||
// subset of it. The read replica labels also cover 'Failed', 'Restarting',
|
||||
// 'Resizing' and 'Restoring', which have no one-to-one lifecycle equivalents;
|
||||
// the closest to 'Failed' is QUARANTINED, surfaced as 'Unhealthy' alongside
|
||||
// SHUTDOWN.
|
||||
export const HA_POOLER_STATUS_LABELS: Record<HaPoolerStatus, string> = {
|
||||
healthy: 'Healthy',
|
||||
coming_up: 'Coming up',
|
||||
|
||||
@@ -1,27 +0,0 @@
|
||||
import { queryOptions } from '@tanstack/react-query'
|
||||
import { z } from 'zod'
|
||||
|
||||
import { getHaAdmin, parseHaAdminResponse } from './get-ha-admin'
|
||||
import { haAdminKeys } from './keys'
|
||||
import { IS_PLATFORM } from '@/lib/constants'
|
||||
import type { ResponseError } from '@/types'
|
||||
|
||||
export type HaClusterCellsVariables = { projectRef?: string }
|
||||
export type HaClusterCellsError = ResponseError
|
||||
|
||||
// Every field is optional because proto3 JSON omits zero values.
|
||||
const haClusterCellsResponseSchema = z.object({ names: z.array(z.string()).optional() })
|
||||
|
||||
async function getHaClusterCells({ projectRef }: HaClusterCellsVariables, signal?: AbortSignal) {
|
||||
const data = await getHaAdmin(projectRef, 'cells', signal)
|
||||
return parseHaAdminResponse(haClusterCellsResponseSchema, data)
|
||||
}
|
||||
|
||||
export type HaClusterCellsData = Awaited<ReturnType<typeof getHaClusterCells>>
|
||||
|
||||
export const haClusterCellsQueryOptions = ({ projectRef }: HaClusterCellsVariables) =>
|
||||
queryOptions({
|
||||
queryKey: haAdminKeys.cells(projectRef),
|
||||
queryFn: ({ signal }) => getHaClusterCells({ projectRef }, signal),
|
||||
enabled: IS_PLATFORM && typeof projectRef !== 'undefined',
|
||||
})
|
||||
@@ -1,30 +0,0 @@
|
||||
import { queryOptions } from '@tanstack/react-query'
|
||||
import { z } from 'zod'
|
||||
|
||||
import { getHaAdmin, parseHaAdminResponse } from './get-ha-admin'
|
||||
import { haAdminKeys } from './keys'
|
||||
import { IS_PLATFORM } from '@/lib/constants'
|
||||
import type { ResponseError } from '@/types'
|
||||
|
||||
export type HaClusterDatabasesVariables = { projectRef?: string }
|
||||
export type HaClusterDatabasesError = ResponseError
|
||||
|
||||
// Every field is optional because proto3 JSON omits zero values.
|
||||
const haClusterDatabasesResponseSchema = z.object({ names: z.array(z.string()).optional() })
|
||||
|
||||
async function getHaClusterDatabases(
|
||||
{ projectRef }: HaClusterDatabasesVariables,
|
||||
signal?: AbortSignal
|
||||
) {
|
||||
const data = await getHaAdmin(projectRef, 'databases', signal)
|
||||
return parseHaAdminResponse(haClusterDatabasesResponseSchema, data)
|
||||
}
|
||||
|
||||
export type HaClusterDatabasesData = Awaited<ReturnType<typeof getHaClusterDatabases>>
|
||||
|
||||
export const haClusterDatabasesQueryOptions = ({ projectRef }: HaClusterDatabasesVariables) =>
|
||||
queryOptions({
|
||||
queryKey: haAdminKeys.databases(projectRef),
|
||||
queryFn: ({ signal }) => getHaClusterDatabases({ projectRef }, signal),
|
||||
enabled: IS_PLATFORM && typeof projectRef !== 'undefined',
|
||||
})
|
||||
@@ -28,6 +28,15 @@ const multipoolerSchema = z.object({
|
||||
// 'SERVING' | 'DISABLED' | 'DRAINING'
|
||||
servingStatus: z.string().optional(),
|
||||
hostname: z.string().optional(),
|
||||
// `PoolerLifecycleStatus` (multigres proto/clustermetadata.proto):
|
||||
// 'STARTING' | 'ACTIVE' | 'STOPPING' | 'SHUTDOWN' | 'QUARANTINED', optionally
|
||||
// prefixed with 'LIFECYCLE_'. 'LIFECYCLE_UNKNOWN' is the zero value, so it is
|
||||
// omitted from JSON. There is no dedicated failure member — QUARANTINED is the
|
||||
// terminal failure state (the pooler gave up recovering, e.g. a failed
|
||||
// pg_rewind or backup restore, and is kept alive for forensics) and SHUTDOWN is
|
||||
// "durably down". `getPoolerStatus` maps every member. An unrecognized value
|
||||
// (a future enum member) falls through to the `servingStatus` check, and since
|
||||
// SERVING is that enum's zero value it usually renders as Healthy until mapped.
|
||||
lifecycleStatus: z.object({ status: z.string().optional() }).optional(),
|
||||
routingState: z
|
||||
.object({
|
||||
|
||||
@@ -1,7 +1,4 @@
|
||||
export const haAdminKeys = {
|
||||
cells: (projectRef: string | undefined) => ['projects', projectRef, 'ha-admin', 'cells'] as const,
|
||||
databases: (projectRef: string | undefined) =>
|
||||
['projects', projectRef, 'ha-admin', 'databases'] as const,
|
||||
poolers: (projectRef: string | undefined) =>
|
||||
['projects', projectRef, 'ha-admin', 'poolers'] as const,
|
||||
gateways: (projectRef: string | undefined) =>
|
||||
|
||||
Reference in new issue
Block a user