Repository navigation
Conversation
Problem: a direct launch (--launch-app-id, opennow://launch, Playnite) runs its own session.remote.list about a second after the startup background sweep. When one regional request of that sweep failed and no session was found, discovery returned session_discovery_failed and the launch stopped, although the next click worked. Cause: discover_sessions had no retry, so a single transient regional failure, such as a timeout or a dropped keep-alive connection, decided the whole sweep. The per-region error was also discarded, so current.log could not say which region failed or why. Change: - While no session has been found and no region rejected the credentials, regions whose failure is transient are retried once, 750 ms later, inside the existing 12 s discovery budget. The retry runs only if more than 2.25 s of the budget remain (the delay plus 1.5 s for a useful attempt), and retried requests keep the min(remaining, 3 s) timeout. v2/serverInfo gets the same single retry. - Transient means HTTP 408, 425, 429, 500, 502, 503 or 504, the set get_session already retries (now shared as is_transient_status), or any transport failure. Other statuses, invalid bodies and rejected requestStatus values are final, and authentication errors still short-circuit. - The delay uses a new Cancellation::sleep that waits on the request's condvar, so cancelling the RPC ends the wait and no retry is sent. - The fail-closed contract in docs/core-protocol.md (OpenCloudGaming#898) is unchanged: if a region still failed or was never checked and no session was found, discovery returns session_discovery_failed, and callers must not create a session. - Each sweep queues one diagnostics line (at most four queued, oldest dropped), and the session.remote.list and session.active.get handlers write every queued line to current.log: [session] discovery: serverInfo=<ok|recovered|failed> regions=N failed=N retried=N recovered=N failures=<name>:<kind>#<attempt>,... Kinds are http=<status>, timeout, dns, tls, connect, closed, body, rejected, network and unchecked, and the list stops after six entries with +N. The timeout, dns, tls and connect stages come from the shared transport_failure classifier (OpenCloudGaming#1109); closed marks a connection closed or reset under the request. Regions are named by CloudMatch metadata key (region<index> when the key is not a plain identifier). Lines never contain URLs, tokens or error messages. - gfn.rs remote_sessions, like hinted session reconciliation, holds the session_routing and auth_operation locks for the whole sweep, so a sweep that retries holds them longer: the 750 ms delay plus the retried requests, still within the 12 s budget. Concurrent routing calls wait for it. Verification (native/opennow-core, debug, as in CI): - cargo fmt --check and cargo clippy --locked --all-targets -- -D warnings: clean. - cargo test --locked: the opennow-core binary passes 713/713, and the other binaries and integration tests pass. New tests cover a transient failure recovering on the retry, fail-closed after a failed retry, no retry for 4xx, 501, body, rejected or authentication failures, no retry once a session is found or without budget, cancellation during the delay, the serverInfo retry, the summary queue and line format, and closed versus timeout classification. Four of them run over loopback HTTP. - The library target's update_apply process tests fail intermittently in this environment, with the same assertion on unmodified dev. That target builds only push/ and update_apply/, which this change does not touch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
 |
|
Thanks. The retry is well bounded otherwise: 4 workers, at most 32 regions, one retry per failed endpoint per sweep, a delay that respects cancellation, and failure stays closed. One issue before merge:
Tests: 30 discovery tests and the full core suite (859, 4 ignored) pass, and fmt is clean. |
A region that kept timing out got a second full 3 s request after the 750 ms delay, so a persistent timeout took 6.75 s to fail discovery instead of 3 s. - The retry pass now uses its own 1.5 s per-request timeout. - A timed-out request is retried at once, since it already waited out its timeout. Fast failures still wait 750 ms first. - The retry budget check is now the delay plus 1.5 s. - v2/serverInfo is retried the same way. A region that keeps timing out now fails the sweep after about 4.5 s. Timeouts stay retryable: in the live runs, both launches the retry rescued were timeouts that succeeded on the second attempt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks. Agreed: a persistent timeout shouldn't double the time to fail. Fixed in 8abf4c5 by capping the retry:
Result: a region that keeps timing out now fails the sweep after about 4.5 s instead of 6.75 s. The worst case, a fast failure plus a persistent timeout in the same sweep, is 5.25 s. Why I kept timeouts retryable: in the live runs behind this PR, both launches the retry rescued were timeouts that succeeded on the second attempt ( Tests:
|
|
@etgbrown GitHub currently reports merge conflicts with |
Resolves the conflicts with dev and ports dev's discovery changes into the retrying sweep: - remote_sessions keeps dev's allocation admission lock and its completeness-gated cleanup of unresolved allocations and pending stops. discover_remote_sessions now returns a DiscoverySweep with the sessions, whether every region was checked, and the endpoints it covered. - discover_sessions_with_status carries the retry and returns completeness; a region that recovers on the retry counts as checked. - fetch_region_sessions applies dev's per-region rules: ended (status 7) sessions are skipped and a malformed entry fails the region as a final (non-retried) body failure. - Cancellation::sleep waits on dev's combined receipt lock and checks cancellation on the guarded receipt, rather than calling cancelled(), which takes the same lock. - docs/core-protocol.md describes both the retry and OpenCloudGaming#1334's new-launch fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Updated: I merged the latest Conflicts resolved:
How this relates to #1334: with #1334, new launches no longer depend on a perfect sweep. Reconnect, reconciliation and known-conflict discovery stay strict, so this retry is what keeps a briefly failing region from failing those. The PR description has a note on it. Checks (core, merged tree):
The |
Resolves the conflict with dev's unresolved-allocation discovery: the region of a verified unresolved allocation now joins the sweep's starting regions (after the selected endpoint), so the retrying sweep checks it even when serverInfo omits it, as dev's inline sweep does. discover_remote_sessions takes the starting regions as one list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merged the latest Dev's unresolved-allocation change now runs inside the retrying sweep. The region of a verified unresolved allocation joins the sweep's starting regions, right after the selected endpoint, so it is checked even when Core checks on the merged tree:
|
Problem
A direct launch (
--launch-app-id,opennow://launch, Playnite) runs its ownsession.remote.listabout a second after the startup background sweep. If one regional request in that sweep failed and no session was found, discovery returnedsession_discovery_failedand the launch stopped, although the next click worked.One
current.logwith v1.0.3's discovery code shows how often: 10 of 23 launches that started within 2.2 s of the previous sweep failed this way, against 0 of 15 with a gap of 2.7 s or more and 4 of 83 background sweeps. The failing sweeps took 1.5–3.1 s, so these were fast per-region errors, not the 12 s budget running out. The logs attached to #985 (a color-mode report) show the same failure.Since #1334: a new launch now continues to
session.createaftersession_discovery_failed, so this no longer blocks first-click launches. Reconnect, reconciliation and known-conflict discovery remain strict, though, so one transient regional failure still fails those. With this PR the sweep itself recovers from such failures. An existing session in a briefly failing region is then found and resumed directly, without the create →session_conflictround trip.Cause
discover_sessionshad no retry, so one transient regional failure decided the whole sweep. The per-region error was also discarded, so the log could not say which region failed or why.Change
Core only, no QML changes.
v2/serverInfogets the same single retry.get_sessionalready retries (now one shared helper), or any transport failure. Other statuses, invalid bodies and rejectedrequestStatusvalues are final, and auth errors still short-circuit.Cancellation::sleep: cancelling the RPC ends it and sends no retry.session_discovery_failed.current.log, for example[session] discovery: serverInfo=ok regions=25 failed=2 retried=2 recovered=2 failures=region19:timeout#1,Bulgaria:closed#1. Regions are named by CloudMatch metadata key, never by URL.FailureKindtakes thetimeout/dns/tls/connectstage from fix(core): name the failed transport stage in network errors #1109'stransport_failureand adds what the retry and the log need on top: HTTP status, closed connections, invalid bodies and rejected requests.gfn.rsremote_sessionsholdssession_routingandauth_operationfor the whole sweep, so a sweep that retries holds them longer (the delay plus the retried requests, each at most 1.5 s); since dev's allocation admission lock, a retrying sweep also holds that lock for the same extra time, still within the 12 s budget.Verification
cargo fmt --checkandcargo clippy --locked --all-targets -- -D warnings(core): clean.cargo test --locked(core, debug, merged with dev 5b89338): 926 passed, 0 failed, 4 ignored across the core binaries and integration tests. New tests cover recovery on the retry, fail-closed after a failed retry, no retry for 4xx/501/body/rejected/auth failures, a found session, a missing budget, cancellation during the delay, theserverInforetry, the log line, and closed versus timeout classification. Four run over loopback HTTP.update_applylib tests fail intermittently here, with the same assertion on unmodified dev; this change does not touch that target.timeoutandclosedregions, all recovered), kinds this version still retries. The other 5 resumed an existing session.--launch-app-id) withsession.remote.list: outcome=session_discovery_failed. Each failing sweep took 1.4–2.2 s and ran 2–3 s after the startup sweep. One launch failed 3 times in a row before a relaunch streamed.Not verified
Related
#985, #898, #1109
🤖 Generated with Claude Code