Repository navigation
fix(ssh): no probe stall on the first remote action after a pause #29240
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| /** | ||
| * When each environment's ready control socket last heard from its server. A leaf module: the | ||
| * serving gate reads it without importing the connection cache, which imports the gate back. | ||
| */ | ||
| const sources = new Map<string, () => number | null>() | ||
|
|
||
| export function setSharedControlAnswerSource( | ||
| environmentId: string, | ||
| source: () => number | null | ||
| ): void { | ||
| sources.set(environmentId, source) | ||
| } | ||
|
|
||
| export function clearSharedControlAnswerSource(environmentId: string): void { | ||
| sources.delete(environmentId) | ||
| } | ||
|
|
||
| /** Null while there is no ready control socket for the environment. */ | ||
| export function getRemoteRuntimeSharedControlAnsweredAt(environmentId: string): number | null { | ||
| return sources.get(environmentId)?.() ?? null | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,8 @@ export type OrcadManagedServingInput = { | |
| connection: OrcadHostTransport | ||
| remotePort: number | ||
| probe: (environment: KnownRuntimeEnvironment, timeoutMs: number) => Promise<boolean> | ||
| /** When this environment's ready control socket last heard from its server, if it has. */ | ||
| answeredAt?: (environmentId: string) => number | null | ||
| } | ||
|
|
||
| /** A wake refused because another update, rollback or recovery holds the host; it clears itself. */ | ||
|
|
@@ -41,6 +43,9 @@ const PROBE_TIMEOUT_MS = 5_000 | |
| // Why: a connect checks right after its fresh tunnel did; one verdict serves both, on that | ||
| // transport and port only, so a kill, reboot or rebind is never answered from cache. | ||
| const VERDICT_REUSE_MS = 5_000 | ||
| // Why: the control socket hears a pong every 10 s while healthy, and a kill, reboot, rebind or | ||
| // stop closes it, so a recent answer on it proves serving without a fresh probe socket. | ||
| const LIVE_ANSWER_PROOF_MS = 20_000 | ||
|
|
||
| type ServingTransport = { connection: OrcadHostTransport; generation: number; remotePort: number } | ||
| // Why per transport: a check on a dropped connection fails, and a caller on the reconnected one | ||
|
|
@@ -85,6 +90,12 @@ export function ensureManagedOrcadServing( | |
| ) { | ||
| return entry.check | ||
| } | ||
| const answeredAt = input.answeredAt?.(id) ?? null | ||
| const answerAge = answeredAt === null ? null : now() - answeredAt | ||
| // Why the lower bound: a wall clock set backwards must not stretch the proof. | ||
| if (answerAge !== null && answerAge >= 0 && answerAge < LIVE_ANSWER_PROOF_MS) { | ||
| return Promise.resolve({ state: 'serving' }) | ||
|
Comment on lines
+96
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: set -o pipefail
printf '%s\n' '--- diff ---'
git diff --find-renames 33759f91f5f5a113d9b2eff5bca6e3a30ec3636e 16033c1d1d38f2d4e5d1023d7b5c95657eb4111c -- src/main/ssh/orcad-managed-serving.ts src/main/ssh src/shared/remote-runtime-shared-control-connection.ts src/shared/remote-runtime-shared-control-test-server.ts src/main/ssh/orcad-managed-serving.test.ts
printf '%s\n' '--- relevant references ---'
rg -n -F --glob '*.ts' -e 'answeredAt' -e 'onAnswered' -e 'autoPong' -e 'status.get' -e 'LIVE_ANSWER_PROOF_MS' src/main/ssh src/shared || test "$?" -eq 1
printf '%s\n' '--- changed implementation ---'
nl -ba src/main/ssh/orcad-managed-serving.ts | sed -n '1,180p'
printf '%s\n' '--- shared connection ---'
nl -ba src/shared/remote-runtime-shared-control-connection.ts | sed -n '1,260p'
printf '%s\n' '--- test server ---'
nl -ba src/shared/remote-runtime-shared-control-test-server.ts | sed -n '1,280p'Repository: stablyai/orca Length of output: 45010 🏁 Script executed: set -o pipefail
printf '%s\n' '--- callback and answeredAt definitions ---'
rg -n -F --glob '*.ts' -e 'onActivity' -e 'getRemoteRuntimeSharedControlAnsweredAt' -e 'onAnswered' -e 'answeredAt' src/shared src/main/ipc
printf '%s\n' '--- socket opener and liveness ---'
nl -ba src/shared/remote-runtime-shared-control-open.ts | sed -n '1,280p'
nl -ba src/shared/remote-runtime-shared-control-types.ts | sed -n '50,110p'
rg -n -F --glob '*.ts' -e 'pong' -e 'ping' src/shared/remote-runtime-shared-control-*.ts src/shared/remote-runtime-socket*.ts || test "$?" -eq 1
printf '%s\n' '--- answeredAt owner and changed tests ---'
nl -ba src/main/ipc/runtime-environment-request-connections.ts | sed -n '1,280p'
git diff --find-renames 33759f91f5f5a113d9b2eff5bca6e3a30ec3636e 16033c1d1d38f2d4e5d1023d7b5c95657eb4111c -- src/main/ipc/runtime-environment-request-connections.ts src/main/ipc/runtime-environment-request-connections.test.ts src/shared/remote-runtime-shared-control-connection.test.ts src/shared/remote-runtime-shared-control-open.ts src/shared/remote-runtime-shared-control-types.tsRepository: stablyai/orca Length of output: 30213 🏁 Script executed: set -o pipefail
printf '%s\n' '--- repository-owned control server bindings ---'
rg -n -F --glob '*.ts' --glob '*.js' --glob '*.tsx' --glob '*.jsx' -e 'new WebSocketServer' -e 'WebSocketServer(' -e 'autoPong' -e 'status.get' -e 'silentMethods' -e 'createSharedControlTestServer' . || test "$?" -eq 1
printf '%s\n' '--- server/control-related files ---'
rg --files | rg 'remote-runtime|runtime-control|shared-control|websocket|ws' | sed -n '1,240p'Repository: stablyai/orca Length of output: 41187 Do not treat a pong alone as proof that
|
||
| } | ||
| const next: ServingCheck = { ...transport, check: checkAndStart(input) } | ||
| checks.set(id, next) | ||
| // A rejected check is never reused. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: stablyai/orca
Length of output: 11131
🏁 Script executed:
Repository: stablyai/orca
Length of output: 42989
🏁 Script executed:
Repository: stablyai/orca
Length of output: 42263
🏁 Script executed:
Repository: stablyai/orca
Length of output: 32870
🏁 Script executed:
Repository: stablyai/orca
Length of output: 7489
Keep
status.getas the serving check.answeredAtrecords transport activity, not an application response. A recent timestamp returnsservingbeforeprobeManagedOrcadTunnelsendsstatus.get. If that probe times out, it returnsfalseand enterswakeIfStopped, which can start a stopped server or returnunverifiablefor a live server that does not answer. The timestamp shortcut bypasses both outcomes, soresolveManagedRuntimeEnvironmentcan continue with the remote call on an unresponsive runtime.Use an authoritative serving response for this shortcut, or retain the
status.getprobe whileansweredAtrepresents only socket activity.