Skip to content

Commit d31a44c

Browse files
fix: harden sync-progress file writes
A second independent review found two more real gaps and one design issue in the sync-progress heartbeat work. heartbeat.sh dropped the whole progress line whenever sync_progress was null, even though era/epoch/slot were known and useful on their own. cardano-cli can omit syncProgress; the Python side already has a fallback for this case. The heartbeat now prints era/epoch/slot with "syncProgress unavailable" instead of nothing at all. jq's // operator only substitutes for null or false, not an empty string. node.py defaults a missing era to "", not null, so the "?" fallback for era never actually fired. Fixed by checking for an empty string explicitly. upsert_json_key duplicated an existing, unused helper, update_json_file, which already did the same read-merge-write. Since update_json_file had zero callers in the codebase, extended it with the same missing/corrupt-file tolerance and atomic write instead of keeping a second near-identical function, and pointed both progress call sites at it. Also adds a missing Args section to write_progress_file's docstring, to match its sibling wait_for_shelley_era.
1 parent bb49a84 commit d31a44c

4 files changed

Lines changed: 62 additions & 66 deletions

File tree

sync_tests/scripts/heartbeat.sh

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,22 @@ _progress_line() {
3434
if [ ! -s "$file" ] || ! command -v jq >/dev/null 2>&1; then
3535
return
3636
fi
37+
local present
38+
present="$(jq -r --arg k "$label" 'has($k)' "$file" 2>/dev/null)"
39+
[ "$present" != "true" ] && return
3740
local era epoch slot pct updated
38-
pct="$(jq -r --arg k "$label" '.[$k].sync_progress // empty' "$file" 2>/dev/null)"
39-
[ -z "$pct" ] && return
40-
era="$(jq -r --arg k "$label" '.[$k].era // "?"' "$file" 2>/dev/null)"
41+
era="$(jq -r --arg k "$label" '(.[$k].era // "") | if . == "" then "?" else . end' "$file" 2>/dev/null)"
4142
epoch="$(jq -r --arg k "$label" '.[$k].epoch // "?"' "$file" 2>/dev/null)"
4243
slot="$(jq -r --arg k "$label" '.[$k].slot // "?"' "$file" 2>/dev/null)"
4344
updated="$(jq -r --arg k "$label" '.[$k].updated_at // "?"' "$file" 2>/dev/null)"
44-
printf 'progress[%s]: %s%% synced - era=%s epoch=%s slot=%s (as of %s)\n' \
45-
"$label" "$pct" "$era" "$epoch" "$slot" "$updated"
45+
pct="$(jq -r --arg k "$label" '.[$k].sync_progress // empty' "$file" 2>/dev/null)"
46+
if [ -n "$pct" ]; then
47+
printf 'progress[%s]: %s%% synced - era=%s epoch=%s slot=%s (as of %s)\n' \
48+
"$label" "$pct" "$era" "$epoch" "$slot" "$updated"
49+
else
50+
printf 'progress[%s]: syncProgress unavailable - era=%s epoch=%s slot=%s (as of %s)\n' \
51+
"$label" "$era" "$epoch" "$slot" "$updated"
52+
fi
4653
}
4754

4855
_tail_group() {

sync_tests/utils/db_sync/__init__.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -317,18 +317,19 @@ def _log_sync_progress(config: DbSyncConfig, env: str, start_sync: float) -> flo
317317
db_sync_tip.slot_no,
318318
)
319319
try:
320-
helpers.upsert_json_key(
320+
helpers.update_json_file(
321321
config.workdir / f"sync_progress_{env}.json",
322-
"dbsync",
323322
{
324-
"epoch": db_sync_tip.epoch_no,
325-
"block": db_sync_tip.block_no,
326-
"slot": db_sync_tip.slot_no,
327-
"sync_progress": db_sync_progress,
328-
"sync_time_h_m_s": sync_time_h_m_s,
329-
"updated_at": datetime.datetime.now(tz=datetime.timezone.utc).strftime(
330-
"%Y-%m-%dT%H:%M:%SZ"
331-
),
323+
"dbsync": {
324+
"epoch": db_sync_tip.epoch_no,
325+
"block": db_sync_tip.block_no,
326+
"slot": db_sync_tip.slot_no,
327+
"sync_progress": db_sync_progress,
328+
"sync_time_h_m_s": sync_time_h_m_s,
329+
"updated_at": datetime.datetime.now(tz=datetime.timezone.utc).strftime(
330+
"%Y-%m-%dT%H:%M:%SZ"
331+
),
332+
}
332333
},
333334
)
334335
except OSError:

sync_tests/utils/helpers.py

Lines changed: 22 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,29 @@ def execute_command(
257257
raise
258258

259259

260-
def update_json_file(file_path: pl.Path, updates: dict) -> None:
261-
"""Read a JSON file, updates it with the provided dictionary, and writes it back."""
262-
with open(file_path) as json_file:
263-
data = json.load(json_file)
264-
260+
def update_json_file(file_path: str | pl.Path, updates: dict) -> None:
261+
"""Read a JSON file, merge it with ``updates``, and write it back.
262+
263+
A missing, unreadable, or invalid existing file is treated as empty
264+
rather than raised, so this is safe to use for status files with more
265+
than one writer, that must never abort a caller just because a previous
266+
write was interrupted. The write itself goes through
267+
``write_json_to_file``, so it is atomic (temp file + rename).
268+
"""
269+
file_path = pl.Path(file_path)
270+
data: dict = {}
271+
if file_path.exists():
272+
try:
273+
with open(file_path) as json_file:
274+
loaded = json.load(json_file)
275+
if isinstance(loaded, dict):
276+
data = loaded
277+
except (json.JSONDecodeError, OSError):
278+
LOGGER.warning(
279+
"Ignoring unreadable status file %s, starting fresh", file_path, exc_info=True
280+
)
265281
data.update(updates)
266-
267-
with open(file_path, "w") as json_file:
268-
json.dump(data, json_file, indent=2)
282+
write_json_to_file(file_path, data)
269283

270284

271285
def remove_json_keys(file_path: pl.Path, keys: list[str]) -> None:
@@ -368,39 +382,6 @@ def write_json_to_file(file_path: str | pl.Path, data: dict | list) -> None:
368382
os.replace(tmp_path, file_path)
369383

370384

371-
def upsert_json_key(file_path: str | pl.Path, key: str, value: dict) -> None:
372-
"""Update a single top-level key in a shared JSON status file.
373-
374-
Reads the existing file (if any), sets ``data[key] = value``, and writes
375-
the whole file back, so unrelated keys written by other callers are
376-
preserved. Same read-merge-write pattern as ``conftest.py``'s
377-
``_write_marker_to_status``, for status files with more than one writer.
378-
379-
An existing file that is missing, unreadable, or not valid JSON is
380-
treated as empty rather than raised, since this is used for
381-
observability status files that must never abort the caller.
382-
383-
Args:
384-
file_path: Path to the shared JSON status file.
385-
key: Top-level key to upsert.
386-
value: Value to store under ``key``.
387-
"""
388-
file_path = pl.Path(file_path)
389-
data: dict = {}
390-
if file_path.exists():
391-
try:
392-
with open(file_path) as fh:
393-
loaded = json.load(fh)
394-
if isinstance(loaded, dict):
395-
data = loaded
396-
except (json.JSONDecodeError, OSError):
397-
LOGGER.warning(
398-
"Ignoring unreadable status file %s, starting fresh", file_path, exc_info=True
399-
)
400-
data[key] = value
401-
write_json_to_file(file_path, data)
402-
403-
404385
def manage_directory(dir_name: str, action: str, root: str = ".") -> str | None:
405386
"""Manage a directory by creating or removing it based on the action specified.
406387

sync_tests/utils/node/__init__.py

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -451,22 +451,29 @@ def write_progress_file(workdir: pl.Path | None, env: str, tip: "Tip") -> None:
451451
452452
Never raises: this is observability only, and must not abort a
453453
multi-hour sync run over a disk-full or permission error.
454+
455+
Args:
456+
workdir: Directory to write the CI heartbeat progress file to. When
457+
``None``, progress-file writing is skipped.
458+
env: Environment name (preview, preprod, mainnet).
459+
tip: Current node tip, as returned by ``get_current_tip``.
454460
"""
455461
if workdir is None:
456462
return
457463
try:
458-
helpers.upsert_json_key(
464+
helpers.update_json_file(
459465
workdir / f"sync_progress_{env}.json",
460-
"node",
461466
{
462-
"era": tip.era,
463-
"epoch": tip.epoch,
464-
"block": tip.block,
465-
"slot": tip.slot,
466-
"sync_progress": tip.sync_progress,
467-
"updated_at": datetime.datetime.now(tz=datetime.timezone.utc).strftime(
468-
"%Y-%m-%dT%H:%M:%SZ"
469-
),
467+
"node": {
468+
"era": tip.era,
469+
"epoch": tip.epoch,
470+
"block": tip.block,
471+
"slot": tip.slot,
472+
"sync_progress": tip.sync_progress,
473+
"updated_at": datetime.datetime.now(tz=datetime.timezone.utc).strftime(
474+
"%Y-%m-%dT%H:%M:%SZ"
475+
),
476+
}
470477
},
471478
)
472479
except OSError:

0 commit comments

Comments
 (0)