fix(ui): harden step-canvas disk cache — PR #204 review round (T-1183)
Atomic index writes (tmp+rename), orphan-payload reconciliation sweep folded into the background sweep, payload shape guard mirroring the protocol's own width/height discriminator, size_bytes via get_position() instead of a full payload re-read, and a 64-bit SHA-256 payload filename (String.hash()'s 31-bit space made a silent wrong-map filename collision a ~1-in-16k event per cap-full body; migration self-heals via the orphan sweep). Tier-3 class doc rewritten to name the real D-253 seam — the glaciation/flooded_q wire fields exist but carry static values and no staleness signal crosses the wire; T-1190 tracks threading a wire TTL into put(sim_ttl_sec) when the driving clock lands — replacing the false 'no sim-state wire field exists' premise. Four new tests plus a _stub_canvas fixture helper centralizing the canvas shape contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -42,12 +42,20 @@ extends RefCounted
|
||||
## first, run on a coarse background timer.
|
||||
## Tier 3 (sim-state-tagged planes) — explicit TTL, staleness-motivated,
|
||||
## structurally separate from tiers 1/2 (`now > written_at + sim_ttl`),
|
||||
## never touched by 2a/2b. NOT YET POPULATED: every EncodedStepCanvas
|
||||
## field is geometry per D-227 today (frozen/flooded ride the existing
|
||||
## glaciation/morphology-water-class fields, Araminta's round-1 schema —
|
||||
## no distinct sim-state WIRE field exists yet for this ticket to tag).
|
||||
## The `sim_ttl`/tier machinery below is WIRED and tested but has no
|
||||
## production caller until a sim-state field lands on EncodedStepCanvas.
|
||||
## never touched by 2a/2b. NOT YET POPULATED, but not for lack of a wire
|
||||
## field: `glaciation`/`flooded_q` ARE distinct sim-state fields on
|
||||
## EncodedStepCanvas today (step_canvas_protocol.gd, Araminta's round-2
|
||||
## sim-state plane, T-1181). The real seam is that their VALUES are still
|
||||
## static — `flooded_q` hardwired 0, `glaciation` the static
|
||||
## DistrictProfile classification — byte-identical to an indefinitely-
|
||||
## fresh derive (server/src/atlas/step_canvas.rs module doc, D-253
|
||||
## wired-ahead), and no staleness signal crosses the wire at all yet (the
|
||||
## server's SIM_STATE_TTL formula is tick-based and server-side only).
|
||||
## TIER_GEOMETRY is therefore the CORRECT classification for every canvas
|
||||
## today. The `sim_ttl`/tier machinery below is WIRED and tested but has
|
||||
## no production caller until D-253's sim-state driving clock lands and
|
||||
## the response gains a real TTL for on_response() to thread into the
|
||||
## existing `put(sim_ttl_sec)` parameter — tracked in T-1190.
|
||||
##
|
||||
## **HARDENING (D-255(d), both mandatory):**
|
||||
## (i) Per-body deep-rung retention cap — DEEP_RUNGS (Block, Chunk; the
|
||||
@@ -72,11 +80,13 @@ extends RefCounted
|
||||
##
|
||||
## **Sweep triggers (never per-frame — stig-round2.md §"Sweep triggers"):**
|
||||
## callers invoke run_visit_sweep() once on body-open (cheap, index-only) and
|
||||
## run_background_sweep() on a coarse timer (LRU-capacity + Tier-3 TTL,
|
||||
## backgroundable). Neither is wired to _process()/a per-frame signal by this
|
||||
## file — the caller (step_canvas_viewer.gd) owns invoking these at the
|
||||
## right moments, matching D-227's "bookkeeping, not gameplay-adjacent work"
|
||||
## instruction.
|
||||
## run_background_sweep() on a coarse timer (LRU-capacity + Tier-3 TTL +
|
||||
## orphan-payload reconciliation, backgroundable — the orphan sweep does list
|
||||
## the body directory, unlike the other two, which is exactly why it lives on
|
||||
## the coarse timer and not body-open). Neither is wired to _process()/a
|
||||
## per-frame signal by this file — the caller (step_canvas_viewer.gd) owns
|
||||
## invoking these at the right moments, matching D-227's "bookkeeping, not
|
||||
## gameplay-adjacent work" instruction.
|
||||
##
|
||||
## D-227: every tier here is an evictable CACHE, never a source of truth.
|
||||
## Deleting the whole cache root at any time changes client behavior only by
|
||||
@@ -185,14 +195,18 @@ func _index_path(body_id: String) -> String:
|
||||
return _body_dir(body_id) + INDEX_FILENAME
|
||||
|
||||
|
||||
## Stable non-negative filename for a composite key — the key string itself
|
||||
## isn't filesystem-safe on every target platform (':'/','), matching
|
||||
## stig-round2.md's own "named by a hash of key" directive. `key.hash() &
|
||||
## 0x7FFFFFFF` is the same non-negative-hash idiom reach_screen.gd already
|
||||
## uses elsewhere in this app for a stable derived value from a String.
|
||||
## Stable filename for a composite key — the key string itself isn't
|
||||
## filesystem-safe on every target platform (':'/','), matching
|
||||
## stig-round2.md's own "named by a hash of key" directive. 64 bits of
|
||||
## SHA-256 rather than String.hash(): the 31-bit space made a silent
|
||||
## filename collision (two live keys sharing one payload file — the wrong
|
||||
## map served as a valid hit) a ~1-in-16k event per cap-full body; at 64
|
||||
## bits it is negligible. The index stays keyed by the FULL key either way —
|
||||
## only the payload filename is derived. Changing this scheme later is safe:
|
||||
## old-scheme rows miss on their payload path and drop, old files are
|
||||
## reclaimed by run_orphan_sweep().
|
||||
static func _payload_filename(key: String) -> String:
|
||||
var h: int = key.hash() & 0x7FFFFFFF
|
||||
return "%08x.dat" % h
|
||||
return key.sha256_text().substr(0, 16) + ".dat"
|
||||
|
||||
|
||||
func _payload_path(body_id: String, key: String) -> String:
|
||||
@@ -227,7 +241,12 @@ static func _is_deep_rung(rung: String) -> bool:
|
||||
## empty cache, don't crash, don't block first paint" instruction. The
|
||||
## corrupt file is left on disk untouched (a caller that never puts() again
|
||||
## for that body leaves it inert; the first successful save_index() call
|
||||
## overwrites it with a valid one).
|
||||
## overwrites it with a valid one). `_save_index()` writes atomically (below),
|
||||
## so a torn/truncated index can only arise from outside interference (manual
|
||||
## edit, platform-level file corruption) — never from a crash mid-write on
|
||||
## this file's own path. Any entries a discarded corrupt index loses are not
|
||||
## gone for good: their payload `.dat` files are reclaimed as orphans by
|
||||
## `run_orphan_sweep()` rather than leaking silently.
|
||||
func _load_index(body_id: String) -> Dictionary:
|
||||
if _indexes.has(body_id):
|
||||
return _indexes[body_id]
|
||||
@@ -246,6 +265,14 @@ func _load_index(body_id: String) -> Dictionary:
|
||||
return idx
|
||||
|
||||
|
||||
## Atomic write: the full index is serialized to a `.tmp` sibling, closed,
|
||||
## then swapped into place with `DirAccess.rename_absolute()` — a single
|
||||
## filesystem rename, never a truncate-in-place. A crash between the tmp
|
||||
## write and the rename leaves the PREVIOUS index.json untouched (the tmp
|
||||
## file is simply orphaned garbage, ignored by `_load_index()`); a crash mid-
|
||||
## rename is not a Godot-visible state this store needs to reason about
|
||||
## (the OS makes rename atomic). This closes the truncate-in-place corruption
|
||||
## window `_load_index()`'s doc used to describe as the normal recovery case.
|
||||
func _save_index(body_id: String) -> void:
|
||||
var idx: Dictionary = _indexes.get(body_id, {})
|
||||
var dir_err := DirAccess.make_dir_recursive_absolute(_body_dir(body_id))
|
||||
@@ -255,12 +282,22 @@ func _save_index(body_id: String) -> void:
|
||||
% [body_id, error_string(dir_err)]
|
||||
)
|
||||
return
|
||||
var file := FileAccess.open(_index_path(body_id), FileAccess.WRITE)
|
||||
var index_path := _index_path(body_id)
|
||||
var tmp_path := index_path + ".tmp"
|
||||
var file := FileAccess.open(tmp_path, FileAccess.WRITE)
|
||||
if file == null:
|
||||
push_warning("StepCanvasDiskCache: cannot write index for '%s'" % body_id)
|
||||
push_warning("StepCanvasDiskCache: cannot write index tmp file for '%s'" % body_id)
|
||||
return
|
||||
file.store_string(JSON.stringify(idx))
|
||||
file.close()
|
||||
var rename_err := DirAccess.rename_absolute(tmp_path, index_path)
|
||||
if rename_err != OK:
|
||||
push_warning(
|
||||
"StepCanvasDiskCache: atomic index rename failed for '%s': %s"
|
||||
% [body_id, error_string(rename_err)]
|
||||
)
|
||||
if FileAccess.file_exists(tmp_path):
|
||||
DirAccess.remove_absolute(tmp_path)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
@@ -277,6 +314,14 @@ func _save_index(body_id: String) -> void:
|
||||
## and the filesystem can disagree (manual deletion, platform storage
|
||||
## pressure clearing files without updating the index), and a dangling index
|
||||
## row must never be handed to a caller as a hit.
|
||||
##
|
||||
## **Corrupt/truncated payload contract:** a payload file that decodes to
|
||||
## something other than a well-formed canvas Dictionary degrades to a miss
|
||||
## with full self-heal, symmetric with the index side's own malformed-JSON
|
||||
## recovery — `file.get_var()` returns null on a failed/garbage decode, the
|
||||
## shape check below catches a decoded-but-wrong-shaped value, and either
|
||||
## way `_drop_entry()` removes BOTH the index row and the payload file before
|
||||
## returning null. No caller ever sees a partial or malformed canvas.
|
||||
func get_canvas(
|
||||
body_id: String, rung: String, center: Vector2i, extent: Vector2i, min_wl_m: int = 0
|
||||
) -> Variant:
|
||||
@@ -301,7 +346,7 @@ func get_canvas(
|
||||
return null
|
||||
var canvas: Variant = file.get_var()
|
||||
file.close()
|
||||
if not canvas is Dictionary:
|
||||
if not canvas is Dictionary or not canvas.has("width") or not canvas.has("height"):
|
||||
_drop_entry(body_id, key)
|
||||
return null
|
||||
|
||||
@@ -335,6 +380,15 @@ func has(
|
||||
## from the rung it's writing, mirroring make_key()'s own Global handling) —
|
||||
## a floored entry is exempt from BOTH sweeps unconditionally.
|
||||
##
|
||||
## **Write order is deliberate: payload file, THEN index entry.** A crash
|
||||
## between the two steps leaves an orphaned payload `.dat` file with no
|
||||
## index row pointing at it — reclaimed later by `run_orphan_sweep()` — never
|
||||
## a dangling index row pointing at a payload that doesn't exist (the other
|
||||
## direction is already handled by `get_canvas()`'s missing-payload-file
|
||||
## check, but a crash can't actually produce it under this ordering). Both
|
||||
## directions of index/payload disagreement are covered: one by write order,
|
||||
## the other by the orphan sweep.
|
||||
##
|
||||
## (i) HARDENING: for a deep-rung (Block/Chunk) write, enforces
|
||||
## MAX_DEEP_RUNG_ENTRIES_PER_BODY SYNCHRONOUSLY before inserting — if this
|
||||
## put() would exceed the cap, the oldest (by last_read_at) deep-rung entry
|
||||
@@ -369,6 +423,12 @@ func put(
|
||||
push_warning("StepCanvasDiskCache: cannot write payload for '%s'/'%s'" % [body_id, key])
|
||||
return
|
||||
file.store_var(canvas)
|
||||
# Captured while the file is still open, right after the write — the
|
||||
# cursor position IS the byte count just written, so this needs no
|
||||
# separate re-read of the file to learn its size (get_file_as_bytes()
|
||||
# would otherwise load the whole payload a second time just to call
|
||||
# .size() on it).
|
||||
var payload_size: int = file.get_position()
|
||||
file.close()
|
||||
|
||||
var now: int = Time.get_unix_time_from_system()
|
||||
@@ -377,7 +437,7 @@ func put(
|
||||
"tier": TIER_SIM_STATE if sim_ttl_sec > 0 else TIER_GEOMETRY,
|
||||
"written_at": now,
|
||||
"last_read_at": now,
|
||||
"size_bytes": FileAccess.get_file_as_bytes(_payload_path(body_id, key)).size(),
|
||||
"size_bytes": payload_size,
|
||||
"sim_ttl": sim_ttl_sec if sim_ttl_sec > 0 else null,
|
||||
"retention_floor": _is_global_rung(rung),
|
||||
"schema_version": current_schema_version(),
|
||||
@@ -520,12 +580,48 @@ func run_staleness_sweep(body_id: String) -> void:
|
||||
_drop_entry(body_id, key)
|
||||
|
||||
|
||||
## Convenience: the three sweeps a coarse background timer runs together —
|
||||
## Reconciliation sweep — reclaims payload `.dat` files on disk that no
|
||||
## longer have an index row pointing at them (an "orphan"). This is the
|
||||
## other half of `put()`'s deliberate payload-before-index write order: a
|
||||
## crash between the two writes leaves exactly this shape, and a discarded
|
||||
## corrupt index (`_load_index()`'s malformed-JSON recovery) orphans EVERY
|
||||
## payload for that body at once. Nothing else in this file ever scans the
|
||||
## body directory, so without this sweep orphaned files leak on disk forever
|
||||
## and are invisible to `run_capacity_sweep()`'s byte accounting (they aren't
|
||||
## indexed, so they're never counted, and never evicted by it either). Builds
|
||||
## the set of payload basenames the CURRENT index actually references (same
|
||||
## derivation `_payload_path()` uses — a hash of the key, not the stored
|
||||
## `file_path` string, so this is robust even if `file_path` was ever wrong),
|
||||
## lists the body directory once, and removes every `.dat` file not in that
|
||||
## set. Never touches `index.json`, `index.json.tmp`, or an indexed payload.
|
||||
func run_orphan_sweep(body_id: String) -> void:
|
||||
var dir_path := _body_dir(body_id)
|
||||
if not DirAccess.dir_exists_absolute(dir_path):
|
||||
return
|
||||
var idx := _load_index(body_id)
|
||||
var referenced: Dictionary = {} # basename String -> true
|
||||
for key: String in idx.keys():
|
||||
referenced[_payload_filename(key)] = true
|
||||
|
||||
var dir := DirAccess.open(dir_path)
|
||||
if dir == null:
|
||||
return
|
||||
dir.list_dir_begin()
|
||||
var entry := dir.get_next()
|
||||
while entry != "":
|
||||
if not dir.current_is_dir() and entry.ends_with(".dat") and not referenced.has(entry):
|
||||
DirAccess.remove_absolute(dir_path + entry)
|
||||
entry = dir.get_next()
|
||||
dir.list_dir_end()
|
||||
|
||||
|
||||
## Convenience: the sweeps a coarse background timer runs together —
|
||||
## run_visit_sweep() is deliberately NOT included here (it belongs on
|
||||
## body-open only, per stig-round2.md's own sweep-trigger split).
|
||||
func run_background_sweep(body_id: String) -> void:
|
||||
run_capacity_sweep(body_id)
|
||||
run_staleness_sweep(body_id)
|
||||
run_orphan_sweep(body_id)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
|
||||
@@ -158,6 +158,9 @@ func on_response(response: Dictionary) -> void:
|
||||
_retries = 0
|
||||
_held_extent = _echoed_extent(canvas, _rung, response.get("extent", Vector2i.ZERO))
|
||||
_cache.put(_body_id, _rung, _center, _extent, canvas, _min_wl_m)
|
||||
# sim_ttl_sec deliberately unset — every wire field is static-valued
|
||||
# today; threads from the wire when D-253's clock lands (T-1190; see
|
||||
# step_canvas_disk_cache.gd's Tier-3 doc).
|
||||
_disk_cache.put(_body_id, _rung, _center, _extent, canvas, _min_wl_m)
|
||||
canvas_ready.emit(canvas)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user