mirror of
https://github.com/unslothai/unsloth.git
synced 2026-07-10 00:08:58 +00:00
Studio: clean up empty leftover quant folders so they can be deleted (#6616)
* Studio: clean up empty leftover quant folders so they can be deleted An interrupted or cancelled split GGUF download leaves snapshots/<rev>/<quant>/ behind with no shards. Such a folder is neither a completed download nor a tracked partial (no .incomplete blobs, no manifest), so it was invisible in the variant list and a per-variant delete returned 404, leaving it on disk forever. - list_empty_gguf_variant_dirs: detect quant folders that are empty in every snapshot, excluding any quant that has shards in another snapshot. - get_gguf_variants_response: surface those quants as partial (cleanable) so the UI shows a delete affordance. - _delete_gguf_variant_from_repos: remove the empty (or just-emptied) quant subfolder and count it toward the result so the delete succeeds instead of 404. Adds hub/tests/test_empty_variant_folder.py. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: simplify empty-dir check to any(iterdir()) * Studio: tighten comments on empty-quant-folder cleanup * Studio: surface empty-folder removal failures and cleanables on local/offline paths Address review feedback on the empty leftover quant folder cleanup: - _remove_empty_variant_dirs now returns removal failures (read-only cache or a locked dir), and the variant delete raises 409 instead of a misleading 404; a concurrent download refilling the dir (ENOTEMPTY) is still treated as a skip. - Empty leftover folders are surfaced as cleanable on every variant-listing path (prefer_local_cache / offline / HF-fallback), not just a remote listing, via a single post-process that flips a listed quant to partial or appends an unlisted one. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Studio: surface empty-folder cleanables even when metadata fetch fails When the cache holds only an empty leftover snapshots/<rev>/<quant>/ folder from an interrupted split download and the client is offline or the HF metadata request fails, _compute() re-raised before cleanables were marked, leaving the folder undeletable. Now fall back to marking cleanables against an empty response and return them if any; otherwise re-raise the original error. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
2aef1a23cb
commit
c72da05741
4 changed files with 301 additions and 2 deletions
|
|
@ -6,6 +6,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import errno
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
|
|
@ -15,7 +16,7 @@ from loggers import get_logger
|
|||
from hub.utils import download_manifest
|
||||
from hub.utils import download_registry
|
||||
from hub.utils import inventory_scan as hf_cache_scan
|
||||
from hub.utils.gguf import extract_quant_label
|
||||
from hub.utils.gguf import extract_quant_label, extract_quant_token
|
||||
from hub.utils.hf_cache_state import (
|
||||
INCOMPLETE_SUFFIX,
|
||||
purge_partial_repo,
|
||||
|
|
@ -106,6 +107,52 @@ def _has_remaining_main_gguf(target_repo) -> bool:
|
|||
)
|
||||
|
||||
|
||||
def _remove_empty_variant_dirs(target_repos: list, variant: str) -> tuple[int, list[str]]:
|
||||
"""Remove now-empty ``snapshots/<rev>/<quant>/`` folders for *variant* (the
|
||||
quant label names the folder); only empty dirs go, so siblings are safe.
|
||||
Returns (count removed, removal failures other than a concurrent refill)."""
|
||||
variant_key = (extract_quant_token(variant) or variant).lower()
|
||||
removed = 0
|
||||
failures: list[str] = []
|
||||
for target_repo in target_repos:
|
||||
repo_path = getattr(target_repo, "repo_path", None)
|
||||
if not repo_path:
|
||||
continue
|
||||
snapshots = Path(repo_path) / "snapshots"
|
||||
if not snapshots.is_dir():
|
||||
continue
|
||||
try:
|
||||
snap_dirs = [s for s in snapshots.iterdir() if s.is_dir() and not s.is_symlink()]
|
||||
except OSError:
|
||||
continue
|
||||
for snap in snap_dirs:
|
||||
try:
|
||||
subs = list(snap.iterdir())
|
||||
except OSError:
|
||||
continue
|
||||
for sub in subs:
|
||||
try:
|
||||
if sub.is_symlink() or not sub.is_dir():
|
||||
continue
|
||||
folder_quant = extract_quant_token(sub.name)
|
||||
matches = (
|
||||
folder_quant is not None and folder_quant.lower() == variant_key
|
||||
) or sub.name.lower() == variant.lower()
|
||||
if not matches or any(sub.iterdir()):
|
||||
continue
|
||||
except OSError:
|
||||
continue
|
||||
try:
|
||||
sub.rmdir()
|
||||
removed += 1
|
||||
except OSError as e:
|
||||
# A concurrent download refilling the dir (ENOTEMPTY) is not a
|
||||
# failure; a read-only cache or locked dir is, so surface it.
|
||||
if e.errno != errno.ENOTEMPTY:
|
||||
failures.append(f"{sub.name}: {e}")
|
||||
return removed, failures
|
||||
|
||||
|
||||
def _delete_gguf_variant_from_repos(
|
||||
repo_id: str,
|
||||
variant: str,
|
||||
|
|
@ -206,11 +253,23 @@ def _delete_gguf_variant_from_repos(
|
|||
)
|
||||
|
||||
state_purged = download_manifest.purge_state("model", repo_id, variant)
|
||||
# Reclaim the empty quant folder so it stops 404ing on delete.
|
||||
removed_dirs, dir_failures = _remove_empty_variant_dirs(target_repos, variant)
|
||||
if dir_failures:
|
||||
raise HTTPException(
|
||||
status_code = 409,
|
||||
detail = (
|
||||
f"Couldn't fully delete {variant} for {repo_id}: "
|
||||
f"{len(dir_failures)} folder(s) could not be removed "
|
||||
"(read-only cache or in use). Try again."
|
||||
),
|
||||
)
|
||||
if (
|
||||
removed_snapshots == 0
|
||||
and deleted_blobs == 0
|
||||
and incomplete_result.deleted == 0
|
||||
and not state_purged
|
||||
and removed_dirs == 0
|
||||
):
|
||||
raise HTTPException(
|
||||
status_code = 404,
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ from hub.utils.gguf import (
|
|||
extract_quant_label,
|
||||
iter_hf_cache_snapshots,
|
||||
is_big_endian_gguf_path,
|
||||
list_empty_gguf_variant_dirs,
|
||||
list_gguf_variants,
|
||||
list_gguf_variants_from_hf_cache,
|
||||
list_local_gguf_variants,
|
||||
|
|
@ -334,6 +335,32 @@ def delete_variant_incomplete_blobs_result(
|
|||
return VariantIncompleteDeleteResult(deleted = deleted, unresolved = False)
|
||||
|
||||
|
||||
def _mark_empty_dir_cleanables(
|
||||
repo_id: str, response: GgufVariantsResponse
|
||||
) -> GgufVariantsResponse:
|
||||
"""Surface empty leftover ``<quant>/`` folders (interrupted downloads) as
|
||||
partial so the UI can delete them -- on local/offline paths too, not just a
|
||||
remote listing. A listed quant is flipped to partial; an unlisted one is
|
||||
appended as a zero-byte cleanable entry."""
|
||||
try:
|
||||
empty_labels = list_empty_gguf_variant_dirs(repo_id)
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to scan empty GGUF variant folders for {repo_id}: {e}")
|
||||
return response
|
||||
if not empty_labels:
|
||||
return response
|
||||
empty_by_key = {label.lower(): label for label in empty_labels}
|
||||
variants = list(response.variants)
|
||||
listed = {v.quant.lower() for v in variants}
|
||||
for i, v in enumerate(variants):
|
||||
if v.quant.lower() in empty_by_key and not v.downloaded and not v.partial:
|
||||
variants[i] = v.model_copy(update = {"partial": True})
|
||||
for key, label in sorted(empty_by_key.items()):
|
||||
if key not in listed:
|
||||
variants.append(GgufVariantDetail(filename = f"{label}.gguf", quant = label, partial = True))
|
||||
return response.model_copy(update = {"variants": variants})
|
||||
|
||||
|
||||
async def get_gguf_variants_response(
|
||||
repo_id: str,
|
||||
prefer_local_cache: bool = False,
|
||||
|
|
@ -653,8 +680,28 @@ async def get_gguf_variants_response(
|
|||
default_variant = default_variant,
|
||||
)
|
||||
|
||||
def _compute_with_cleanables() -> GgufVariantsResponse:
|
||||
skip = is_local_path(repo_id) or not _is_valid_repo_id(repo_id)
|
||||
try:
|
||||
response = _compute()
|
||||
except Exception:
|
||||
# Offline / metadata fetch failed with only an empty leftover
|
||||
# <quant>/ folder cached: still surface it so the UI can delete it,
|
||||
# otherwise re-raise the original error.
|
||||
if skip:
|
||||
raise
|
||||
enriched = _mark_empty_dir_cleanables(
|
||||
repo_id, GgufVariantsResponse(repo_id = repo_id, variants = [])
|
||||
)
|
||||
if enriched.variants:
|
||||
return enriched
|
||||
raise
|
||||
if skip:
|
||||
return response
|
||||
return _mark_empty_dir_cleanables(repo_id, response)
|
||||
|
||||
try:
|
||||
return await asyncio.to_thread(_compute)
|
||||
return await asyncio.to_thread(_compute_with_cleanables)
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
|
|
|
|||
166
studio/backend/hub/tests/test_empty_variant_folder.py
Normal file
166
studio/backend/hub/tests/test_empty_variant_folder.py
Normal file
|
|
@ -0,0 +1,166 @@
|
|||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
||||
|
||||
"""Cleanup of empty leftover quant folders from interrupted split downloads."""
|
||||
|
||||
import errno
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
from hub.schemas.inventory import GgufVariantDetail, GgufVariantsResponse
|
||||
from hub.services.models import deletion, gguf_variants
|
||||
from hub.utils import gguf
|
||||
|
||||
|
||||
def _make_snapshot(root: Path) -> Path:
|
||||
snap = root / "snapshots" / "rev0"
|
||||
(snap / "UD-IQ1_M").mkdir(parents = True)
|
||||
(snap / "UD-IQ1_M" / "GLM-UD-IQ1_M-00001-of-00002.gguf").write_bytes(b"x")
|
||||
(snap / "UD-IQ1_M" / "GLM-UD-IQ1_M-00002-of-00002.gguf").write_bytes(b"y")
|
||||
(snap / "UD-IQ1_S").mkdir(parents = True) # empty leftover
|
||||
return snap
|
||||
|
||||
|
||||
def test_list_empty_gguf_variant_dirs_finds_empty_leftover(tmp_path, monkeypatch):
|
||||
snap = _make_snapshot(tmp_path)
|
||||
monkeypatch.setattr(gguf, "iter_hf_cache_snapshots", lambda repo_id: iter([snap]))
|
||||
assert gguf.list_empty_gguf_variant_dirs("org/Repo-GGUF") == {"UD-IQ1_S"}
|
||||
|
||||
|
||||
def test_list_empty_excludes_quant_with_files_in_another_snapshot(tmp_path, monkeypatch):
|
||||
snap1 = tmp_path / "s1" / "snapshots" / "rev"
|
||||
(snap1 / "UD-IQ1_S").mkdir(parents = True) # empty here
|
||||
snap2 = tmp_path / "s2" / "snapshots" / "rev"
|
||||
(snap2 / "UD-IQ1_S").mkdir(parents = True)
|
||||
(snap2 / "UD-IQ1_S" / "m-UD-IQ1_S-00001-of-00001.gguf").write_bytes(b"z") # has shards
|
||||
monkeypatch.setattr(gguf, "iter_hf_cache_snapshots", lambda repo_id: iter([snap1, snap2]))
|
||||
assert gguf.list_empty_gguf_variant_dirs("org/Repo-GGUF") == set()
|
||||
|
||||
|
||||
def test_list_empty_ignores_non_quant_dirs(tmp_path, monkeypatch):
|
||||
snap = tmp_path / "snapshots" / "rev"
|
||||
(snap / "not-a-quant").mkdir(parents = True) # empty but not a quant label
|
||||
monkeypatch.setattr(gguf, "iter_hf_cache_snapshots", lambda repo_id: iter([snap]))
|
||||
assert gguf.list_empty_gguf_variant_dirs("org/Repo-GGUF") == set()
|
||||
|
||||
|
||||
def test_remove_empty_variant_dirs_removes_only_empty_match(tmp_path):
|
||||
snap = _make_snapshot(tmp_path)
|
||||
repo = SimpleNamespace(repo_path = str(tmp_path))
|
||||
removed, failures = deletion._remove_empty_variant_dirs([repo], "UD-IQ1_S")
|
||||
assert removed == 1
|
||||
assert failures == []
|
||||
assert not (snap / "UD-IQ1_S").exists()
|
||||
assert (snap / "UD-IQ1_M").is_dir()
|
||||
|
||||
|
||||
def test_remove_empty_variant_dirs_never_touches_populated_folder(tmp_path):
|
||||
snap = _make_snapshot(tmp_path)
|
||||
repo = SimpleNamespace(repo_path = str(tmp_path))
|
||||
removed, failures = deletion._remove_empty_variant_dirs([repo], "UD-IQ1_M")
|
||||
assert removed == 0
|
||||
assert failures == []
|
||||
assert len(list((snap / "UD-IQ1_M").iterdir())) == 2
|
||||
|
||||
|
||||
def test_remove_empty_variant_dirs_surfaces_real_failure(tmp_path, monkeypatch):
|
||||
_make_snapshot(tmp_path)
|
||||
repo = SimpleNamespace(repo_path = str(tmp_path))
|
||||
|
||||
def _denied(self):
|
||||
raise OSError(errno.EACCES, "permission denied")
|
||||
|
||||
monkeypatch.setattr(Path, "rmdir", _denied)
|
||||
removed, failures = deletion._remove_empty_variant_dirs([repo], "UD-IQ1_S")
|
||||
assert removed == 0
|
||||
assert len(failures) == 1
|
||||
|
||||
|
||||
def test_remove_empty_variant_dirs_ignores_concurrent_refill(tmp_path, monkeypatch):
|
||||
_make_snapshot(tmp_path)
|
||||
repo = SimpleNamespace(repo_path = str(tmp_path))
|
||||
|
||||
def _refilled(self):
|
||||
raise OSError(errno.ENOTEMPTY, "directory not empty")
|
||||
|
||||
monkeypatch.setattr(Path, "rmdir", _refilled)
|
||||
removed, failures = deletion._remove_empty_variant_dirs([repo], "UD-IQ1_S")
|
||||
assert removed == 0
|
||||
assert failures == []
|
||||
|
||||
|
||||
def test_mark_empty_dir_cleanables_appends_unlisted(monkeypatch):
|
||||
monkeypatch.setattr(gguf_variants, "list_empty_gguf_variant_dirs", lambda repo_id: {"UD-IQ1_S"})
|
||||
resp = GgufVariantsResponse(
|
||||
repo_id = "org/Repo-GGUF",
|
||||
variants = [GgufVariantDetail(filename = "m-UD-IQ1_M.gguf", quant = "UD-IQ1_M", downloaded = True)],
|
||||
)
|
||||
out = gguf_variants._mark_empty_dir_cleanables("org/Repo-GGUF", resp)
|
||||
by_q = {v.quant: v for v in out.variants}
|
||||
assert by_q["UD-IQ1_M"].downloaded is True
|
||||
assert by_q["UD-IQ1_S"].partial is True and by_q["UD-IQ1_S"].downloaded is False
|
||||
|
||||
|
||||
def test_mark_empty_dir_cleanables_flips_listed_variant(monkeypatch):
|
||||
monkeypatch.setattr(gguf_variants, "list_empty_gguf_variant_dirs", lambda repo_id: {"UD-IQ1_S"})
|
||||
resp = GgufVariantsResponse(
|
||||
repo_id = "org/Repo-GGUF",
|
||||
variants = [GgufVariantDetail(filename = "m-UD-IQ1_S.gguf", quant = "UD-IQ1_S")],
|
||||
)
|
||||
out = gguf_variants._mark_empty_dir_cleanables("org/Repo-GGUF", resp)
|
||||
assert len(out.variants) == 1
|
||||
assert out.variants[0].partial is True
|
||||
|
||||
|
||||
def _force_compute_to_raise(monkeypatch):
|
||||
# Drive _compute() down its remote path, fail metadata, and have both cache
|
||||
# fallbacks miss so the original error re-raises.
|
||||
def _boom(*a, **k):
|
||||
raise RuntimeError("offline")
|
||||
|
||||
monkeypatch.setattr(gguf_variants, "list_gguf_variants", _boom, raising = False)
|
||||
monkeypatch.setattr(
|
||||
gguf_variants, "list_gguf_variants_from_hf_cache", lambda repo_id: None, raising = False
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
gguf_variants, "list_partial_gguf_variants_from_state", lambda repo_id: None, raising = False
|
||||
)
|
||||
|
||||
|
||||
def test_get_variants_surfaces_cleanable_when_metadata_fails(monkeypatch):
|
||||
# Offline / model_info fails and only an empty leftover folder is cached:
|
||||
# the cleanable must still be returned instead of the error propagating.
|
||||
import asyncio
|
||||
|
||||
_force_compute_to_raise(monkeypatch)
|
||||
monkeypatch.setattr(gguf_variants, "list_empty_gguf_variant_dirs", lambda repo_id: {"UD-IQ1_S"})
|
||||
|
||||
resp = asyncio.run(
|
||||
gguf_variants.get_gguf_variants_response(
|
||||
"org/Repo-GGUF", prefer_local_cache = False, hf_token = None
|
||||
)
|
||||
)
|
||||
by_q = {v.quant: v for v in resp.variants}
|
||||
assert "UD-IQ1_S" in by_q
|
||||
assert by_q["UD-IQ1_S"].partial is True and by_q["UD-IQ1_S"].downloaded is False
|
||||
|
||||
|
||||
def test_get_variants_reraises_when_no_cleanable(monkeypatch):
|
||||
# Offline with nothing cleanable: original error must propagate (as HTTP).
|
||||
import asyncio
|
||||
|
||||
from fastapi import HTTPException
|
||||
|
||||
_force_compute_to_raise(monkeypatch)
|
||||
monkeypatch.setattr(gguf_variants, "list_empty_gguf_variant_dirs", lambda repo_id: set())
|
||||
|
||||
try:
|
||||
asyncio.run(
|
||||
gguf_variants.get_gguf_variants_response(
|
||||
"org/Repo-GGUF", prefer_local_cache = False, hf_token = None
|
||||
)
|
||||
)
|
||||
raised = False
|
||||
except (HTTPException, RuntimeError):
|
||||
raised = True
|
||||
assert raised
|
||||
|
|
@ -276,6 +276,33 @@ def iter_hf_cache_snapshots(repo_id: str):
|
|||
yield from snapshots
|
||||
|
||||
|
||||
def list_empty_gguf_variant_dirs(repo_id: str) -> set[str]:
|
||||
"""Quant labels present only as an EMPTY snapshot ``<quant>/`` folder (an
|
||||
interrupted split download); a quant with shards in any snapshot is excluded."""
|
||||
empty: dict[str, str] = {}
|
||||
nonempty: set[str] = set()
|
||||
for snapshot in iter_hf_cache_snapshots(repo_id):
|
||||
try:
|
||||
entries = list(snapshot.iterdir())
|
||||
except OSError:
|
||||
continue
|
||||
for sub in entries:
|
||||
try:
|
||||
if sub.is_symlink() or not sub.is_dir():
|
||||
continue
|
||||
quant = extract_quant_token(sub.name)
|
||||
if not quant:
|
||||
continue
|
||||
has_child = any(sub.iterdir())
|
||||
except OSError:
|
||||
continue
|
||||
if has_child:
|
||||
nonempty.add(quant.lower())
|
||||
else:
|
||||
empty.setdefault(quant.lower(), quant)
|
||||
return {label for key, label in empty.items() if key not in nonempty}
|
||||
|
||||
|
||||
def list_gguf_variants_from_hf_cache(repo_id: str) -> Optional[tuple[list[GgufVariantInfo], bool]]:
|
||||
for snapshot in iter_hf_cache_snapshots(repo_id):
|
||||
variants, has_vision = list_local_gguf_variants(str(snapshot))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue