Skip to content

Commit 49f2879

Browse files
rodboevpre-commit-ci[bot]danielhanchenEtherll
authored
fix(studio): recover stalled Hub downloads over HTTP (#6858)
* fix(studio): recover stalled Hub downloads over HTTP * fix(studio): preserve retry generation and progress baseline * fix(studio): keep XET retry handoff nonterminal * fix(studio): preserve retry cancellation on claim failure * fix(studio): make retry failure cancellation atomic * fix(studio): close skipped retry state gaps * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stabilize chat-only export gate detection on Windows * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Retrigger CI on a user-authored head * fix(studio): serialize XET HTTP retry handoff * List XET to HTTP retries that are briefly released from the repo guard as active downloads for PR #6858 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Settle no-process active downloads on shutdown so a parked XET retry cannot spawn after cleanup for PR #6858 * Settle exited-error and no-process downloads on shutdown and persist their cancel markers for PR #6858 * Keep terminal HTTP failures uncancelled and block companion deletion for released retry peers for PR #6858 * Trim download lifecycle test coverage * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Daniel Han <danielhanchen@gmail.com> Co-authored-by: Etherll <61019402+Etherll@users.noreply.github.com>
1 parent 1c7bce4 commit 49f2879

3 files changed

Lines changed: 557 additions & 34 deletions

File tree

studio/backend/hub/services/download_lifecycle.py

Lines changed: 259 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import signal
99
import subprocess
1010
import sys
11+
import time
1112
import threading
1213
from pathlib import Path
1314
from typing import Callable, Optional
@@ -210,7 +211,9 @@ def finalize_worker_exit(
210211
repo_type: Optional[RepoType] = None,
211212
repo_id: Optional[str] = None,
212213
transport: Optional[str] = None,
213-
) -> None:
214+
cancel_marker_transport: Optional[str] = None,
215+
defer_error: bool = False,
216+
) -> str:
214217
"""Block until *proc* exits, then record the job's terminal state in
215218
*registry*. Drains and scrubs stderr first, then classifies the exit code.
216219
A no-op when the process was already dropped (e.g. superseded).
@@ -222,14 +225,16 @@ def finalize_worker_exit(
222225
rc = proc.wait()
223226
cancel_requested = registry.cancel_requested(key)
224227
if not registry.drop_process(key, proc):
225-
return
228+
return "idle"
226229
stderr_text = download_registry.scrub_secrets(
227230
(stderr_data or b"").decode("utf-8", "replace").strip(),
228231
hf_token = hf_token,
229232
)
230233
state = classify_exit(rc, cancel_requested = cancel_requested)
231234
if state == "complete":
232235
registry.set_job(key, "complete")
236+
if transport == download_registry.TRANSPORT_HTTP:
237+
registry.update_job_transport(key, download_registry.TRANSPORT_HTTP)
233238
if stderr_text:
234239
if download_manifest.MANIFEST_DEGRADED_MARKER in stderr_text:
235240
logger.warning(
@@ -262,18 +267,226 @@ def finalize_worker_exit(
262267
metadata.variant
263268
if metadata is not None and metadata.variant
264269
else download_registry.variant_from_key(key),
265-
transport,
270+
cancel_marker_transport or transport,
266271
logger = logger,
267272
)
268273
else:
269-
registry.set_job(
274+
if not defer_error:
275+
registry.set_job(
276+
key,
277+
"error",
278+
stderr_text or f"worker exited with code {rc}",
279+
)
280+
logger.error(
281+
f"{log_prefix} failed for {label} (rc={rc}): {stderr_text}",
282+
)
283+
return state
284+
285+
286+
def _set_retry_failure_state(
287+
registry: download_registry.DownloadRegistry,
288+
key: str,
289+
error: str,
290+
*,
291+
repo_type: RepoType,
292+
repo_id: str,
293+
fallback_variant: Optional[str],
294+
fallback_transport: Optional[str],
295+
logger,
296+
) -> str:
297+
state, metadata = registry.set_error_unless_cancelled(key, error)
298+
if state == "cancelled":
299+
download_registry.persist_cancel_marker(
300+
repo_type,
301+
repo_id,
302+
metadata.variant if metadata is not None and metadata.variant else fallback_variant,
303+
metadata.transport
304+
if metadata is not None and metadata.transport
305+
else fallback_transport,
306+
logger = logger,
307+
)
308+
return state
309+
310+
311+
def _try_http_retry(
312+
registry: download_registry.DownloadRegistry,
313+
key: str,
314+
*,
315+
hf_token: Optional[str],
316+
label: str,
317+
log_prefix: str,
318+
logger,
319+
repo_type: RepoType,
320+
repo_id: str,
321+
watch_name: str,
322+
) -> bool:
323+
"""Reclaim *key* with HTTP transport and spawn a recovery worker.
324+
325+
Returns ``True`` when the HTTP worker was successfully registered.
326+
Caller is responsible for ensuring this is only called when: the job is
327+
in ``"error"`` state, the original transport was XET, and HTTP is available.
328+
329+
Derives variant and blob-hash metadata from the registry entry written by
330+
the original XET claim so callers do not re-construct worker arguments.
331+
Re-queries peer protection hashes at spawn time to reflect any concurrent
332+
sibling changes between the XET failure and this call.
333+
"""
334+
original_metadata = registry.get_job_metadata(key)
335+
if original_metadata is None:
336+
logger.debug("%s XET retry skipped for %s; metadata unavailable", log_prefix, label)
337+
_set_retry_failure_state(
338+
registry,
339+
key,
340+
"XET retry skipped: metadata unavailable",
341+
repo_type = repo_type,
342+
repo_id = repo_id,
343+
fallback_variant = download_registry.variant_from_key(key),
344+
fallback_transport = download_registry.TRANSPORT_XET,
345+
logger = logger,
346+
)
347+
return False
348+
if original_metadata.transport != download_registry.TRANSPORT_XET:
349+
logger.debug(
350+
"%s XET retry skipped for %s; original transport was %s",
351+
log_prefix,
352+
label,
353+
original_metadata.transport,
354+
)
355+
_set_retry_failure_state(
356+
registry,
357+
key,
358+
f"XET retry skipped: original transport was {original_metadata.transport}",
359+
repo_type = repo_type,
360+
repo_id = repo_id,
361+
fallback_variant = original_metadata.variant,
362+
fallback_transport = original_metadata.transport,
363+
logger = logger,
364+
)
365+
return False
366+
variant = original_metadata.variant
367+
blob_hashes = original_metadata.blob_hashes
368+
progress_blob_hashes = original_metadata.progress_blob_hashes
369+
completed_baseline_bytes = (
370+
download_registry.completed_blob_bytes(
371+
repo_type,
372+
repo_id,
373+
progress_blob_hashes,
374+
)
375+
if progress_blob_hashes
376+
else 0
377+
)
378+
generation = registry.current_generation(key)
379+
registry.release_active_slot(key)
380+
while True:
381+
if registry.cancel_requested(key):
382+
_set_retry_failure_state(
383+
registry,
384+
key,
385+
"HTTP retry cancelled before reclaiming the download slot",
386+
repo_type = repo_type,
387+
repo_id = repo_id,
388+
fallback_variant = variant,
389+
fallback_transport = original_metadata.transport,
390+
logger = logger,
391+
)
392+
return False
393+
394+
claimed, conflict_state = registry.claim(
270395
key,
271-
"error",
272-
stderr_text or f"worker exited with code {rc}",
396+
download_registry.TRANSPORT_HTTP,
397+
repo_type = repo_type,
398+
repo_id = repo_id,
399+
variant = variant,
400+
blob_hashes = blob_hashes,
401+
progress_blob_hashes = progress_blob_hashes,
402+
completed_baseline_bytes = completed_baseline_bytes,
403+
generation = generation,
404+
replace_active = True,
405+
cancel_marker_transport = original_metadata.transport,
406+
)
407+
if claimed:
408+
break
409+
if conflict_state == "deleting":
410+
logger.debug(
411+
"%s XET retry claim rejected for %s; repo is being deleted",
412+
log_prefix,
413+
label,
414+
)
415+
_set_retry_failure_state(
416+
registry,
417+
key,
418+
"HTTP retry could not reclaim the download slot",
419+
repo_type = repo_type,
420+
repo_id = repo_id,
421+
fallback_variant = variant,
422+
fallback_transport = original_metadata.transport,
423+
logger = logger,
424+
)
425+
return False
426+
logger.debug(
427+
"%s XET retry claim blocked for %s by active sibling state %s; waiting",
428+
log_prefix,
429+
label,
430+
conflict_state,
431+
)
432+
time.sleep(0.05)
433+
434+
args: list[str] = ["--repo-id", repo_id]
435+
if repo_type == "dataset":
436+
args.append("--dataset")
437+
elif variant:
438+
args.extend(["--variant", variant])
439+
440+
# Re-query at spawn time: sibling state may have changed since XET failed.
441+
peer_hashes = registry.peer_blob_hashes(key) if variant else frozenset()
442+
443+
logger.warning(
444+
"%s XET worker failed for %s; retrying over HTTP",
445+
log_prefix,
446+
label,
447+
)
448+
try:
449+
proc = spawn_worker(
450+
args,
451+
hf_token,
452+
use_xet = False,
453+
protected_blob_hashes = peer_hashes or None,
273454
)
455+
except Exception as exc:
456+
scrubbed = download_registry.scrub_secrets(str(exc), hf_token = hf_token)
274457
logger.error(
275-
f"{log_prefix} failed for {label} (rc={rc}): {stderr_text}",
458+
"%s HTTP retry spawn failed for %s: %s",
459+
log_prefix,
460+
label,
461+
scrubbed,
462+
)
463+
registry.update_job_transport(key, original_metadata.transport)
464+
_set_retry_failure_state(
465+
registry,
466+
key,
467+
scrubbed,
468+
repo_type = repo_type,
469+
repo_id = repo_id,
470+
fallback_variant = variant,
471+
fallback_transport = original_metadata.transport,
472+
logger = logger,
276473
)
474+
return False
475+
476+
return register_worker(
477+
registry,
478+
key,
479+
proc,
480+
hf_token = hf_token,
481+
label = label,
482+
log_prefix = log_prefix,
483+
logger = logger,
484+
repo_type = repo_type,
485+
repo_id = repo_id,
486+
transport = download_registry.TRANSPORT_HTTP,
487+
cancel_marker_transport = original_metadata.transport,
488+
watch_name = watch_name,
489+
)
277490

278491

279492
def kill_and_reap_process(
@@ -309,6 +522,7 @@ def register_worker(
309522
repo_type: RepoType,
310523
repo_id: str,
311524
transport: str,
525+
cancel_marker_transport: Optional[str] = None,
312526
watch_name: str,
313527
) -> bool:
314528
if not registry.register_process(key, proc):
@@ -319,7 +533,14 @@ def register_worker(
319533

320534
def _watch() -> None:
321535
try:
322-
finalize_worker_exit(
536+
can_retry_http = (
537+
transport == download_registry.TRANSPORT_XET
538+
and download_registry.download_transport_unavailable_reason(
539+
download_registry.TRANSPORT_HTTP
540+
)
541+
is None
542+
)
543+
state = finalize_worker_exit(
323544
registry,
324545
key,
325546
proc,
@@ -330,7 +551,25 @@ def _watch() -> None:
330551
repo_type = repo_type,
331552
repo_id = repo_id,
332553
transport = transport,
554+
cancel_marker_transport = cancel_marker_transport,
555+
defer_error = can_retry_http,
333556
)
557+
# XET-to-HTTP recovery: when a non-cancelled XET worker fails and
558+
# HTTP is available, attempt one automatic retry over HTTP. The
559+
# transport check is the recursion guard: an HTTP worker that errors
560+
# never satisfies `transport == TRANSPORT_XET`, so it stays terminal.
561+
if can_retry_http and state == "error":
562+
_try_http_retry(
563+
registry,
564+
key,
565+
hf_token = worker_token,
566+
label = label,
567+
log_prefix = log_prefix,
568+
logger = logger,
569+
repo_type = repo_type,
570+
repo_id = repo_id,
571+
watch_name = watch_name,
572+
)
334573
except Exception:
335574
# finalize_worker_exit is the only thing that clears running/cancelling;
336575
# if it raises, force a terminal state so claim() isn't blocked until restart.
@@ -426,8 +665,19 @@ def cancel_worker(
426665
return "cancelling"
427666
return registry.get_job(key).state
428667
# Worker already exited; let its watcher classify the real return code.
429-
# Arming a pending cancel here could mislabel a genuine failure as a cancel.
430668
if proc.poll() is not None:
669+
get_metadata = getattr(registry, "get_job_metadata", None)
670+
metadata = get_metadata(key) if get_metadata is not None else None
671+
can_retry_http = (
672+
metadata is not None
673+
and metadata.transport == download_registry.TRANSPORT_XET
674+
and download_registry.download_transport_unavailable_reason(
675+
download_registry.TRANSPORT_HTTP
676+
)
677+
is None
678+
)
679+
if can_retry_http and registry.mark_pending_cancel(key, generation):
680+
return "cancelling"
431681
return registry.get_job(key).state
432682

433683
if not registry.request_cancel(key, proc, generation):

0 commit comments

Comments
 (0)