Repository navigation
fix(clusters): only instances on screen poll their status on the instances list #1821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
dawsontoth
merged 18 commits into
claude/1569-get-status-version-gate
from
claude/1603-offscreen-status-polls
Oct 12, 2026
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
9005e40
fix(clusters): only instances on screen poll their status on the inst…
dawsontoth 6fef406
fix(clusters): a row's visibility belongs to the element that was obs…
dawsontoth 3a341c3
fix(hooks): let useIsOnScreen release an element once it is gone
dawsontoth fd2e004
fix(clusters): give the instances list's status spinner screen-reader…
dawsontoth e5c118e
test(clusters): drop a comment the aria-hidden assertion now states
dawsontoth 8c2f6a5
fix(deployments): rejected deployment polls back off, and the page ne…
dawsontoth beda7f9
fix(deployments): say the page could not load when the instance versi…
dawsontoth 537c82a
feat(deploy): follow a rolling deploy's restart job, and show canary …
dawsontoth 6213350
fix(deploy): never read an unrecognized or buffered rollout as a clea…
dawsontoth 034dd9f
fix(deploy): a failed poll no longer counts toward a restart job's no…
dawsontoth 8eefb69
feat(deployments): filter and page the list, show staged releases and…
dawsontoth 806599e
fix(deployments): no stale rows across a page change, and no staged f…
dawsontoth 23beb6e
fix(deployments): drop a status filter the instance cannot have from …
dawsontoth 63c9f2a
feat(deploy): stage a release now and activate it later (Harper 5.3+)
dawsontoth c9b7cda
fix(deploy): an activation survives its detail closing, and is never …
dawsontoth 1b1bbdb
fix(deploy): an unknown activation is not also toasted as a failure
dawsontoth 7c13bb0
fix(deploy): resend a live deploy only when Harper refused it, never …
dawsontoth 377d6b0
docs(deploy): say a write may resend a live deploy only on a 4xx refusal
dawsontoth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| /** @vitest-environment jsdom */ | ||
| import type { Instance } from '@/integrations/api/api.patch'; | ||
| import { FakeIntersectionObserver } from '@/test/fakeIntersectionObserver'; | ||
| import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; | ||
| import { act, cleanup, render } from '@testing-library/react'; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { InstanceStatusCell } from './InstanceStatusCell'; | ||
|
|
||
| /** Real `getStatusQueryOptions` and a real QueryClient: only the transport is faked, so these | ||
| * count the requests the instances list actually sends. */ | ||
| const state = vi.hoisted(() => ({ requests: [] as string[] })); | ||
| vi.mock('@/hooks/usePermissions', () => ({ useOrganizationClusterInstancePermissions: () => ({ update: true }) })); | ||
| vi.mock('@/config/useInstanceClient', () => ({ | ||
| useInstanceClientIdParams: ({ instanceId }: { instanceId: string }) => ({ | ||
| entityId: instanceId, | ||
| instanceClient: { | ||
| post: async () => { | ||
| state.requests.push(instanceId); | ||
| return { data: { systemStatus: [{ id: 'availability', status: 'Available' }], componentStatus: [] } }; | ||
| }, | ||
| }, | ||
| }), | ||
| })); | ||
| vi.mock('@/integrations/api/instance/status/setStatus', () => ({ useSetStatus: () => ({ mutate: vi.fn() }) })); | ||
|
|
||
| const ids = ['ins-0', 'ins-1', 'ins-2']; | ||
|
|
||
| beforeEach(() => { | ||
| vi.useFakeTimers(); | ||
| vi.stubGlobal('IntersectionObserver', FakeIntersectionObserver); | ||
| state.requests = []; | ||
| }); | ||
| afterEach(() => { | ||
| cleanup(); | ||
| vi.useRealTimers(); | ||
| vi.unstubAllGlobals(); | ||
| FakeIntersectionObserver.reset(); | ||
| }); | ||
|
|
||
| function renderList() { | ||
| const queryClient = new QueryClient(); | ||
| render( | ||
| <QueryClientProvider client={queryClient}> | ||
| {ids.map((id, index) => ( | ||
| <div key={id} data-row={id}> | ||
| <InstanceStatusCell | ||
| instance={{ id, status: 'RUNNING', instanceFqdn: `${id}.example.com`, version: '4.7.3' } as Instance} | ||
| index={index} | ||
| /> | ||
| </div> | ||
| ))} | ||
| </QueryClientProvider>, | ||
| ); | ||
| return queryClient; | ||
| } | ||
|
|
||
| function rowIs(id: string) { | ||
| return (target: Element) => target.closest('[data-row]')?.getAttribute('data-row') === id; | ||
| } | ||
|
|
||
| function requestsFor(id: string) { | ||
| return state.requests.filter(r => r === id).length; | ||
| } | ||
|
|
||
| describe('instances list get_status polling', () => { | ||
| it('polls only the rows on screen', async () => { | ||
| const queryClient = renderList(); | ||
| // Past the per-row stagger; React flushes the `ready` update only when an act() ends. | ||
| await act(() => vi.advanceTimersByTimeAsync(2_000)); | ||
| act(() => FakeIntersectionObserver.setIntersecting(true, rowIs('ins-0'))); | ||
| await act(() => vi.advanceTimersByTimeAsync(60_000)); | ||
|
|
||
| expect(requestsFor('ins-0')).toBe(7); | ||
| expect(requestsFor('ins-1')).toBe(0); | ||
| expect(requestsFor('ins-2')).toBe(0); | ||
| queryClient.clear(); | ||
| }); | ||
|
|
||
| it('fetches a row as soon as it scrolls into view, then keeps its 10s cadence', async () => { | ||
| const queryClient = renderList(); | ||
| await act(() => vi.advanceTimersByTimeAsync(30_000)); | ||
| expect(state.requests).toHaveLength(0); | ||
|
|
||
| act(() => FakeIntersectionObserver.setIntersecting(true, rowIs('ins-1'))); | ||
| await act(() => vi.advanceTimersByTimeAsync(0)); | ||
| expect(requestsFor('ins-1')).toBe(1); | ||
|
|
||
| await act(() => vi.advanceTimersByTimeAsync(30_000)); | ||
| expect(requestsFor('ins-1')).toBe(4); | ||
| queryClient.clear(); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Applications — design notes | ||
|
|
||
| ## When a deploy is done | ||
|
|
||
| Studio deploys with `restart: 'rolling'`. The node that receives the deploy answers once its own part is done and hands | ||
| the other nodes to a `restart_service` job, named in the response's `restartJobId` (Harper 4.6+). The deployment record | ||
| does not help: only the receiving node writes it, so it reads `success` while that job runs and after it fails. | ||
|
|
||
| So [`deployAndFollowComponent`](../../../integrations/api/instance/applications/deployComponent.ts) follows the job with | ||
| `get_job` before the mutation settles ([`restartJob.ts`](../../../integrations/api/instance/applications/restartJob.ts)): | ||
|
|
||
| - **A failed job rejects with `DeployRolloutError`.** The release is live on the receiving node; the error and the | ||
| progress panel name each node that did not take it. Its message quotes those nodes' errors, so Datadog withholds it | ||
| like `SSEOperationError`. | ||
| - **A job Studio could not follow resolves, as `rollout.state === 'unfollowable'`, and is never reported as success.** | ||
| That covers a job that ended with a result in neither shape below. [`reportDeployOutcome`](modals/reportDeployHealth.ts) | ||
| skips the post-deploy health check for it, because that check reads the one node Studio reaches, which can be healthy | ||
| on the release a rejecting node kept; `reportDeployResult` is the same policy for a caller that does not health-check. | ||
| Any new caller of the deploy mutation reports through one of the two. | ||
| - **The job may not be there.** `hdb_job` is a non-replicating system table, so only the receiving node can answer for | ||
| it. Central manager's cluster proxy sends every operation to the first running instance, so over the proxy the deploy | ||
| and its polls reach the same node; a direct connection to a cluster's address can reach another node, which answers | ||
| `[]`. A row missing for 20 seconds counts as `not-found`. | ||
| - **Every node's outcome arrives at once.** Harper writes the job's row when it starts and when it ends, never in between. | ||
|
|
||
| The job comes in two shapes. Harper 5.4's certified rollout activates the staged release on each other node, and each | ||
| certifies it with a canary of its own: `result.activated` on success, `message: { error, activated }` on failure. Older | ||
| versions, and a 5.4 deploy whose release was not armed for a canary, run the plain restart: `result.replicated`, where a | ||
| node the job could not reach is listed with its error and the job still completes, so that node reads `unconfirmed`. | ||
|
|
||
| Canary certification is shown only when the response carries it, which only 5.4+ does. There is no version check on | ||
| the deploy path: `registration_info` is denied to a role with an operations allowlist, which is exactly the kind of | ||
| role that may be allowed to deploy, so a version gate there would hide the result from it. | ||
|
|
||
| ## Staging and activation are gated the other way | ||
|
|
||
| Stage only (`activate: false`) and Activate (`deployment_id`) are offered only when `registration_info` reads 5.3.0 or | ||
| later (`useStagedDeploymentsAvailable`), and an unreadable version offers neither. The difference from certification is | ||
| what happens when the guess is wrong: a node older than 5.3.0 accepts `activate: false` as an unknown field and makes | ||
| the release live, which is the opposite of what the person asked for. Certification only adds a field to a response. | ||
|
|
||
| All three kinds of request are built in one place, [`deployRequestBody`](../../../integrations/api/instance/applications/deployComponent.ts), | ||
| because Harper's validator refuses the wrong mix outright: a stage with `restart`, or an activation carrying `package`, | ||
| install options or `activate`. A stage succeeds only once every other node confirmed staging it, so a stage restarts | ||
| nothing and is reported without a health check. Rollback to a kept release (#1232) is the same activation call. | ||
|
|
||
| ## A live deploy is sent once | ||
|
|
||
| A live (SSE) deploy that gets no event stream goes out again as the buffered POST only when its status proves Harper | ||
| never started it: a 4xx. Harper refuses a request before the operation starts (authentication, permissions, | ||
| validation) with a 4xx, and answers one it started with a 200 event stream at once. Everything else can follow a deploy | ||
| that is already running: a gateway's 5xx after forwarding, a 2xx that is not a stream, a dropped connection, the idle | ||
| watchdog firing before headers. Harper's own request-queue 503 never started anything either, but status alone cannot | ||
| tell it from a gateway's. So [`onDeployComponentSubmit`](../../../integrations/api/instance/applications/deployComponent.ts) | ||
| turns those into `SSEInconclusiveError`, which every deploy caller already answers by checking rather than resending. | ||
| `streamOperation` stays neutral: it puts the `status` on `SSEUnsupportedError` and leaves the decision to the caller, | ||
| because a read (`get_deployment`) may fall back on anything. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.