fix(client): cold-body Pending responses reach the retry path — the launch-shape starvation root cause
The true cause of the black first launch, server-confirmed after four disproven theories: a stone-cold body answers the FIRST window request with a whole-response status 'Pending' (only the whole-body cache-hit branch sets Ready), and on_response()'s very first check — status != Ready -> return — swallowed it before the retry machinery could run. retries stayed 0 forever; the DERIVING state never resolved. Second connections worked by luck (the first request warms the whole-body cache, so they read Ready and take the healthy path). Both tile fan-out AND single-window first-descents were affected — one shared function, one fix: branch on the outer status first (the atlas_generation_proxy reference shape): Pending -> retry, Ready -> existing null-window retry, NotFound/Error -> give up immediately (the principled give-up policy, replacing the elapsed-retries ceiling). Hardening in the same round: deterministic exponential backoff (0.5s doubling, 4s cap) + per-tile stagger (0.1s * index — six tiles retry at 0.5/0.6/0.7/0.8/0.9/1.0s, strictly-increasing asserted, not jittered); MAX_RETRIES 20->30 (~110s horizon under backoff). Regressions are wire-accurate by construction: whole-response Pending (body_id only, no center/n/granularity — verified against the server's own response construction) delivered through the REAL fan-out (tile_set._on_atlas_layers_received, never tile.on_response directly), mirrored at single-window level. Two first-draft tests that passed with the bug reverted were caught and strengthened before reporting; every fix and hardening piece revert-verified independently (7+1 failing tests without them).
This commit is contained in:
@@ -57,6 +57,27 @@ static func _mock_response(body_id: String, window: Variant) -> Dictionary:
|
||||
return {"body_id": body_id, "status": "Ready", "district_window": window}
|
||||
|
||||
|
||||
## PR #192 cold-start round 3: the WIRE-ACCURATE shape of a cold body's
|
||||
## first-ever response (server/src/atlas/layer_proxy.rs's
|
||||
## get_or_generate()/serve_district_window(), whole-body cache MISS branch —
|
||||
## `AtlasLayerResponse { status: Pending, district_window: None, ... }`,
|
||||
## confirmed directly against that source). `body_id` is the only
|
||||
## identifying field.
|
||||
static func _pending_response(body_id: String) -> Dictionary:
|
||||
return {"body_id": body_id, "status": "Pending", "district_window": null}
|
||||
|
||||
|
||||
static func _not_found_response(body_id: String) -> Dictionary:
|
||||
return {"body_id": body_id, "status": "NotFound", "district_window": null}
|
||||
|
||||
|
||||
## Matches atlas_map_protocol.gd's `_decode_status_field()` — the decoded
|
||||
## `AtlasLayerStatus::Error(String)` variant's status STRING is always just
|
||||
## "Error" (the message rides in a separate `error` field).
|
||||
static func _error_response(body_id: String, message: String = "boom") -> Dictionary:
|
||||
return {"body_id": body_id, "status": "Error", "error": message, "district_window": null}
|
||||
|
||||
|
||||
func _make_request() -> Variant:
|
||||
var owner_stub := RefCounted.new()
|
||||
var req = auto_free(AtlasWindowRequest.new(owner_stub))
|
||||
@@ -454,3 +475,143 @@ func test_clamp_window_n_mirror_v2_region_per_axis_cap_alone_satisfies_wire_cap_
|
||||
func test_cell_grid_side_region_mirror_minimum_is_one() -> void:
|
||||
assert_int(AtlasWindowRequest._cell_grid_side_region_mirror(1)).is_equal(1)
|
||||
assert_int(AtlasWindowRequest._cell_grid_side_region_mirror(50)).is_equal(1)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# PR #192 cold-start round 3 — the single-window half of the launch-shape
|
||||
# gap: the SAME status-gate bug the tile fan-out has (test_atlas_window_tile_set.gd's
|
||||
# own regressions) applies equally here, since on_response() is the shared
|
||||
# class both paths use. A first descent onto a cold body with NO tiles
|
||||
# (District/Quarter rung, or a small Region body) hits the identical
|
||||
# whole-body-cache-miss -> status:"Pending" wire shape.
|
||||
# =============================================================================
|
||||
|
||||
|
||||
## The exact bug, single-window shape: a whole-response Pending on a cold
|
||||
## first request must increment the retry counter and schedule a re-poll —
|
||||
## not be silently dropped by the OLD `status != "Ready" -> return` gate.
|
||||
func test_cold_request_whole_response_pending_increments_retry_count() -> void:
|
||||
var req = _make_request()
|
||||
req.request_now("GJ380c", Vector2i(2, 2), 2)
|
||||
assert_bool(req.is_pending()).is_true()
|
||||
|
||||
req.on_response(_pending_response("GJ380c"))
|
||||
|
||||
assert_bool(req.is_pending()).override_failure_message(
|
||||
"a whole-response Pending must leave the request still pending, not"
|
||||
+ " silently give up"
|
||||
).is_true()
|
||||
assert_int(req._retries).override_failure_message(
|
||||
"a whole-response Pending must increment the retry counter — the"
|
||||
+ " exact bug: the OLD status-gate dropped this before ever reaching"
|
||||
+ " the retry-scheduling code, leaving retries at 0 forever"
|
||||
).is_equal(1)
|
||||
|
||||
|
||||
## Full convergence: a whole-response Pending, then a real Ready for the
|
||||
## RE-REQUEST, must be accepted — proving the retry loop's own re-request
|
||||
## actually gets picked up, not just that the counter increments. The
|
||||
## mid-test retries==1 assertion is what makes this genuinely load-bearing:
|
||||
## without it, a Ready delivered ANY time after a Pending (retried or not)
|
||||
## trivially passes this test's final assertion, since accepting a fresh
|
||||
## Ready response was never the broken behavior — only the retry itself was.
|
||||
func test_cold_request_converges_after_pending_then_real_ready() -> void:
|
||||
var req = _make_request()
|
||||
req.request_now("GJ380c", Vector2i(2, 2), 2)
|
||||
|
||||
req.on_response(_pending_response("GJ380c"))
|
||||
assert_bool(req.is_pending()).is_true()
|
||||
assert_int(req._retries).override_failure_message(
|
||||
"sanity: the retry must have actually been scheduled before this test"
|
||||
+ " waits for it to fire — otherwise the final assertion below would"
|
||||
+ " pass even if the retry never happened at all"
|
||||
).is_equal(1)
|
||||
|
||||
await get_tree().create_timer(0.6).timeout # past the first retry delay
|
||||
|
||||
var window: Dictionary = _mock_window(Vector2i(2, 2), 2)
|
||||
req.on_response(_mock_response("GJ380c", window))
|
||||
assert_bool(req.is_pending()).override_failure_message(
|
||||
"a real Ready response after the pending/retry cycle must be accepted"
|
||||
).is_false()
|
||||
|
||||
|
||||
## NotFound must give up immediately, not retry.
|
||||
func test_cold_request_not_found_gives_up_immediately_without_retry() -> void:
|
||||
var req = _make_request()
|
||||
req.request_now("GJ380c", Vector2i(2, 2), 2)
|
||||
|
||||
req.on_response(_not_found_response("GJ380c"))
|
||||
|
||||
assert_bool(req.is_pending()).override_failure_message(
|
||||
"NotFound must give up immediately, not stay pending waiting for a retry"
|
||||
).is_false()
|
||||
assert_int(req._retries).is_equal(0)
|
||||
|
||||
|
||||
## Error must give up immediately too.
|
||||
func test_cold_request_error_gives_up_immediately_without_retry() -> void:
|
||||
var req = _make_request()
|
||||
req.request_now("GJ380c", Vector2i(2, 2), 2)
|
||||
|
||||
req.on_response(_error_response("GJ380c"))
|
||||
|
||||
assert_bool(req.is_pending()).override_failure_message(
|
||||
"Error must give up immediately, not stay pending waiting for a retry"
|
||||
).is_false()
|
||||
assert_int(req._retries).is_equal(0)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# _retry_delay_for() — deterministic exponential backoff + per-tile stagger
|
||||
# (PR #192 cold-start round 3 hardening: 6 tiles retrying in perfect
|
||||
# lockstep on a whole-response Pending is a real request-pulse risk even
|
||||
# though it isn't what caused the starvation bug above).
|
||||
# =============================================================================
|
||||
|
||||
|
||||
func test_retry_delay_for_first_retry_is_the_initial_delay() -> void:
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(1, 0)).is_equal_approx(
|
||||
AtlasWindowRequest.INITIAL_RETRY_DELAY, 0.0001
|
||||
)
|
||||
|
||||
|
||||
func test_retry_delay_for_doubles_each_retry_until_the_cap() -> void:
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(1, 0)).is_equal_approx(0.5, 0.0001)
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(2, 0)).is_equal_approx(1.0, 0.0001)
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(3, 0)).is_equal_approx(2.0, 0.0001)
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(4, 0)).is_equal_approx(4.0, 0.0001)
|
||||
# Retry 5 would double past MAX_RETRY_DELAY (8.0) — must clamp, not keep growing.
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(5, 0)).is_equal_approx(
|
||||
AtlasWindowRequest.MAX_RETRY_DELAY, 0.0001
|
||||
)
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(20, 0)).is_equal_approx(
|
||||
AtlasWindowRequest.MAX_RETRY_DELAY, 0.0001
|
||||
)
|
||||
|
||||
|
||||
## The deterministic stagger: tile index i's delay is offset by
|
||||
## STAGGER_STEP*i on top of the same backoff schedule — directly assertable,
|
||||
## not a randomized jitter a test would have to tolerance-check.
|
||||
func test_retry_delay_for_staggers_deterministically_by_tile_index() -> void:
|
||||
var base: float = AtlasWindowRequest._retry_delay_for(1, 0)
|
||||
for i in range(6):
|
||||
var expected: float = base + AtlasWindowRequest.STAGGER_STEP * float(i)
|
||||
assert_float(AtlasWindowRequest._retry_delay_for(1, i)).override_failure_message(
|
||||
"tile index %d's first-retry delay must be exactly base + STAGGER_STEP*%d" % [i, i]
|
||||
).is_equal_approx(expected, 0.0001)
|
||||
|
||||
|
||||
## Six tiles that all went pending in the same frame must NOT all retry at
|
||||
## the exact same instant — the anti-storm property this hardening exists
|
||||
## for, pinned directly: every tile's delay for the SAME retry count must be
|
||||
## strictly increasing with its stagger index.
|
||||
func test_retry_delay_for_six_tiles_never_collide_on_the_same_retry() -> void:
|
||||
var delays: Array = []
|
||||
for i in range(6):
|
||||
delays.append(AtlasWindowRequest._retry_delay_for(1, i))
|
||||
for i in range(1, delays.size()):
|
||||
assert_float(delays[i]).override_failure_message(
|
||||
"tile %d's delay must be strictly greater than tile %d's — a storm"
|
||||
+ " pulse means two tiles retrying at the same instant" % [i, i - 1]
|
||||
).is_greater(delays[i - 1])
|
||||
|
||||
Reference in New Issue
Block a user