mirror of
https://github.com/AlexsJones/llmfit.git
synced 2026-10-18 05:38:26 +00:00
fix(scraper): keep a rate-limited base lookup out of the params cache (#1071)
* fix(scraper): keep a rate-limited base lookup out of the params cache _repo_total_params swallowed every failure into None and cached it for the rest of the run, so a 429 that survived the retries on the base_model lookup was indistinguishable from a repo with no safetensors.total. check_gguf_repo_exists then returned False for the candidate, enrich_gguf_sources cached that miss for GGUF_CACHE_MAX_AGE_DAYS, and every later candidate declaring the same base was rejected until the next run. Only an answer is cached now: a total, or None when the repo is gone (401/403/404) or has no safetensors.total. A 429 past the retries, a 5xx or a transport error propagates, so the existing handler in check_gguf_repo_exists maps the 429 to "unknown" with nothing cached, and the next candidate on the same base asks again. Test: a 429 on the base lookup leaves the candidate unknown and the cache untouched, a second candidate on the same base re-asks and is accepted, and a base without a size is still cached as None. Fixes #1067 * fix(scraper): a failed GGUF lookup is unknown, not a miss check_gguf_repo_exists mapped only an exhausted 429 to None; a 5xx or a transport failure, on the candidate or (since the previous commit) on its base_model lookup, fell through to False, which enrich_gguf_sources cached as a miss for GGUF_CACHE_MAX_AGE_DAYS. The handler now returns None for anything HuggingFace did not answer: 429 past the retries, 5xx, network trouble, a truncated or non-JSON body. A 4xx and a repo without gguf tags are still a miss. Test: 503, a dropped connection and a truncated read on the base lookup, a 502 on the candidate, and a 503 through enrich_gguf_sources with nothing written to the cache. Refs #1067
This commit is contained in:
parent
9781270c1c
commit
29bab4d109
2 changed files with 105 additions and 11 deletions
|
|
@ -14,6 +14,7 @@ Usage:
|
|||
import argparse
|
||||
import concurrent.futures
|
||||
import email.utils
|
||||
import http.client
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
|
|
@ -1745,20 +1746,40 @@ _REPO_PARAMS_CACHE: dict[str, int | None] = {}
|
|||
_MIRROR_PARAMS_TOLERANCE = 0.30
|
||||
|
||||
|
||||
# Answers that settle a base lookup: the repo is gone or cannot be read.
|
||||
# Anything else (a 429 past the retries, a 5xx, a transport error) says
|
||||
# nothing about the repo and must not be remembered for the rest of the run.
|
||||
_NO_REPO_HTTP_CODES = frozenset({401, 403, 404})
|
||||
|
||||
|
||||
def _repo_total_params(repo_id: str) -> int | None:
|
||||
"""Total parameter count of a repo from its safetensors metadata."""
|
||||
"""Total parameter count of a repo from its safetensors metadata.
|
||||
|
||||
Cached for the rest of the run only when HuggingFace answered: a repo
|
||||
without safetensors.total, or one that is gone, is a real None. A 429
|
||||
that survived the retries, a 5xx or a transport failure propagates
|
||||
instead, so check_gguf_repo_exists keeps its own handling (a 429 is
|
||||
"unknown", nothing cached) and the next candidate declaring the same
|
||||
base asks again (#1067).
|
||||
"""
|
||||
if repo_id in _REPO_PARAMS_CACHE:
|
||||
return _REPO_PARAMS_CACHE[repo_id]
|
||||
url = f"{HF_API}/{repo_id}"
|
||||
total = None
|
||||
try:
|
||||
with _hf_urlopen(url, timeout=10, kind="gguf_probe") as resp:
|
||||
info = json.loads(resp.read().decode())
|
||||
st = info.get("safetensors") or {}
|
||||
raw = st.get("total")
|
||||
total = int(raw) if raw else None
|
||||
except Exception:
|
||||
pass
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code not in _NO_REPO_HTTP_CODES:
|
||||
raise
|
||||
info = {}
|
||||
if not isinstance(info, dict):
|
||||
raise ValueError(f"{repo_id}: expected an object, got {type(info).__name__}")
|
||||
st = info.get("safetensors")
|
||||
raw = st.get("total") if isinstance(st, dict) else None
|
||||
try:
|
||||
total = int(raw) if raw else None
|
||||
except (TypeError, ValueError):
|
||||
total = None
|
||||
_REPO_PARAMS_CACHE[repo_id] = total
|
||||
return total
|
||||
|
||||
|
|
@ -1782,8 +1803,10 @@ def check_gguf_repo_exists(
|
|||
the canonical upstream). Repos without base_model tags are accepted as
|
||||
before (unverifiable).
|
||||
|
||||
Returns None when HuggingFace stayed rate limited after the retries: the
|
||||
answer is unknown, and enrich_gguf_sources must not cache it as a miss.
|
||||
Returns None when HuggingFace did not answer, for the candidate or for a
|
||||
base_model lookup: rate limited after the retries, a 5xx, or a transport
|
||||
failure. The answer is unknown, and enrich_gguf_sources must not cache
|
||||
it as a miss.
|
||||
"""
|
||||
url = f"{HF_API}/{repo_id}"
|
||||
try:
|
||||
|
|
@ -1807,9 +1830,12 @@ def check_gguf_repo_exists(
|
|||
return abs(ratio - 1.0) <= _MIRROR_PARAMS_TOLERANCE
|
||||
return True
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code == 429:
|
||||
if e.code == 429 or e.code >= 500:
|
||||
return None
|
||||
return False
|
||||
except (OSError, ValueError, http.client.HTTPException):
|
||||
# HF did not answer: unknown, never cached as a miss
|
||||
return None
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
|
@ -1820,7 +1846,7 @@ def _resolve_gguf_sources(
|
|||
"""Resolve GGUF sources for a single model repo.
|
||||
|
||||
Returns (sources, checks) where checks is [(candidate_repo, exists), ...]
|
||||
and exists is None for a probe that stayed rate limited.
|
||||
and exists is None for a probe HuggingFace did not answer.
|
||||
"""
|
||||
sources: list[dict] = []
|
||||
checks: list[tuple[str, bool | None]] = []
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
import contextlib
|
||||
import datetime
|
||||
import email.message
|
||||
import http.client
|
||||
import io
|
||||
import sys
|
||||
import urllib.error
|
||||
|
|
@ -311,6 +312,71 @@ def test_exhausted_gguf_probe_is_not_cached_as_a_miss():
|
|||
assert gguf.cache_writes == [{}], gguf.cache_writes
|
||||
|
||||
|
||||
def test_rate_limited_base_lookup_is_not_cached_as_no_size():
|
||||
# #1067: a 429 on the base_model lookup was swallowed into
|
||||
# _REPO_PARAMS_CACHE as None, so the candidate was rejected as a
|
||||
# wrong-model quant (and cached as a miss one level up), and every later
|
||||
# candidate declaring the same base was rejected for the rest of the run.
|
||||
shm._REPO_PARAMS_CACHE.clear()
|
||||
quant = b'{"tags": ["gguf", "base_model:other/base"]}'
|
||||
burst = [
|
||||
_http_error(f"{shm.HF_API}/other/base", 429, ratelimit='"api";r=0;t=3')
|
||||
for _ in range(RATE_LIMIT_MAX_RETRIES + 1)
|
||||
]
|
||||
with _FakeHF([_FakeResponse(quant, None)] + burst) as hf:
|
||||
assert shm.check_gguf_repo_exists("mirror/model-GGUF", "org/model", 1_000) is None
|
||||
assert RATE_LIMIT_STATS["gave_up"] == 1
|
||||
assert "other/base" not in shm._REPO_PARAMS_CACHE
|
||||
assert len(hf.requests) == 1 + RATE_LIMIT_MAX_RETRIES + 1
|
||||
|
||||
# Pause over, another candidate declares the same base: it is asked
|
||||
# again, and a matching size accepts the mirror.
|
||||
base = b'{"safetensors": {"total": 1000}}'
|
||||
with _FakeHF([_FakeResponse(quant, None), _FakeResponse(base, None)]) as hf:
|
||||
assert shm.check_gguf_repo_exists("mirror2/model-GGUF", "org/model", 1_000) is True
|
||||
assert shm._REPO_PARAMS_CACHE["other/base"] == 1000
|
||||
assert len(hf.requests) == 2
|
||||
|
||||
# A base HF answers for, but without safetensors.total, is a real
|
||||
# answer: cached as None, and the next candidate is rejected without
|
||||
# asking HF again.
|
||||
shm._REPO_PARAMS_CACHE.clear()
|
||||
no_size = b'{"safetensors": {}}'
|
||||
script = [_FakeResponse(quant, None), _FakeResponse(no_size, None), _FakeResponse(quant, None)]
|
||||
with _FakeHF(script) as hf:
|
||||
assert shm.check_gguf_repo_exists("mirror/model-GGUF", "org/model", 1_000) is False
|
||||
assert shm.check_gguf_repo_exists("mirror2/model-GGUF", "org/model", 1_000) is False
|
||||
assert shm._REPO_PARAMS_CACHE["other/base"] is None
|
||||
assert len(hf.requests) == 3
|
||||
shm._REPO_PARAMS_CACHE.clear()
|
||||
|
||||
|
||||
def test_failed_lookup_is_unknown_not_a_miss():
|
||||
# A 5xx or a dropped connection, on the candidate or on its base_model,
|
||||
# says nothing about the repo: unknown like an exhausted 429, never a
|
||||
# miss that enrich_gguf_sources would cache for GGUF_CACHE_MAX_AGE_DAYS.
|
||||
shm._REPO_PARAMS_CACHE.clear()
|
||||
quant = b'{"tags": ["gguf", "base_model:other/base"]}'
|
||||
base_url = f"{shm.HF_API}/other/base"
|
||||
failures = [
|
||||
_http_error(base_url, 503),
|
||||
urllib.error.URLError("connection reset"),
|
||||
http.client.IncompleteRead(b"{"),
|
||||
]
|
||||
for failure in failures:
|
||||
with _FakeHF([_FakeResponse(quant, None), failure]):
|
||||
assert shm.check_gguf_repo_exists("mirror/model-GGUF", "org/model", 1_000) is None
|
||||
assert "other/base" not in shm._REPO_PARAMS_CACHE
|
||||
with _FakeHF([_http_error(PROBE_URL, 502)]):
|
||||
assert shm.check_gguf_repo_exists("unsloth/model-GGUF") is None
|
||||
# Through the enrichment path: nothing is written to the GGUF cache.
|
||||
model = {"name": "org/model", "format": "gguf", "parameters_raw": 1}
|
||||
with _FakeHF([_http_error(PROBE_URL, 503)]), _OneCandidateNoDisk() as gguf:
|
||||
assert shm.enrich_gguf_sources([model], threads=1) == 0
|
||||
assert "gguf_sources" not in model
|
||||
assert gguf.cache_writes == [{}], gguf.cache_writes
|
||||
|
||||
|
||||
def test_missing_gguf_repo_is_still_cached_as_a_miss():
|
||||
model = {"name": "org/model", "format": "gguf", "parameters_raw": 1}
|
||||
with _FakeHF([_http_error(PROBE_URL, 404)]), _OneCandidateNoDisk() as gguf:
|
||||
|
|
@ -743,6 +809,8 @@ if __name__ == "__main__":
|
|||
test_wait_is_capped_against_bogus_reset_values,
|
||||
test_in_flight_threads_report_one_pause_not_eight,
|
||||
test_exhausted_gguf_probe_is_not_cached_as_a_miss,
|
||||
test_rate_limited_base_lookup_is_not_cached_as_no_size,
|
||||
test_failed_lookup_is_unknown_not_a_miss,
|
||||
test_missing_gguf_repo_is_still_cached_as_a_miss,
|
||||
test_yarn_context_is_not_scaled_twice,
|
||||
test_rope_factor_still_scales_a_pre_scaling_window,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue