Skip to content

fix(hf): load HF models offline-first to avoid Hub 429 rate-limiting#527

Open
wilke0818 wants to merge 3 commits into
alphafrom
fix/hf-offline-resilient-loading
Open

fix(hf): load HF models offline-first to avoid Hub 429 rate-limiting#527
wilke0818 wants to merge 3 commits into
alphafrom
fix/hf-offline-resilient-loading

Conversation

@wilke0818

Copy link
Copy Markdown
Collaborator
  • HF generates 429 errors due to too many model calls especially when running parallelized across many nodes
  • Attempts to fix this for ASR models
  • Should be applied across all HF code

Under many parallel jobs (e.g. analyze_audio across SLURM nodes), every
from_pretrained/pipeline call makes a network revision check against the
HF Hub even when the model is already cached. Concurrent jobs collectively
trip HTTP 429 rate limits, which surface as hard ASR/alignment task
failures (a recent batch showed ~3k Qwen3-ASR failures, all 429 on
huggingface.co/api/models/...).

Add in-library resilience — no external env vars required:
- hf_offline_loading(): ensure a snapshot is cached once across
  processes/nodes (existing cross-node heartbeat lock + retry), then load
  from the local cache only (HF_HUB_OFFLINE/TRANSFORMERS_OFFLINE) so no
  network revision check fires. Restores env afterward (even on error).
- load_hf_resilient(): loader-agnostic offline-load + retry-on-429 wrapper.
- hf_subprocess_env(): subprocess analogue; sets the offline flags in the
  child env only when all referenced models are cached (supports workers
  that load >1 model, e.g. Qwen3-ASR + its companion forced aligner).

Apply at the ASR + forced-alignment load sites that were 429-ing:
Whisper/HF ASR pipeline, Granite, MMS aligner + per-language wav2vec2
(in-process); Qwen, Canary, NeMo (subprocess workers).

Add unit tests for the new helpers (no network).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces resilient offline loading mechanisms for HuggingFace models to prevent rate-limiting (429 errors) during parallel execution. It adds helper functions (hf_offline_loading, hf_subprocess_env, and load_hf_resilient) and integrates them across several audio and speech-to-text tasks. The review feedback highlights a potential concurrency bottleneck caused by a process-global lock in hf_offline_loading and suggests refactoring it to use a reference counter instead. Additionally, the feedback points out a missing revision parameter in the Granite speech-to-text task and recommends adding unit tests for nested and concurrent loading scenarios.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/senselab/utils/dependencies.py Outdated
Comment on lines +352 to +358
# ``HF_HUB_OFFLINE`` is process-global. ``hf_offline_loading`` serializes its
# temporary toggle through this lock so concurrent loaders in the same process
# don't observe each other's env mutation. Model *loads* (not inference) thus
# run sequentially while the env is flipped — acceptable since loads are
# one-time and cached. (Mirrors the global-state-lock precedent of
# ``speechbrain_loading_cwd``.)
_hf_offline_lock = threading.RLock()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using a process-global lock (_hf_offline_lock) to serialize the entire duration of hf_offline_loading creates a major concurrency bottleneck. While a thread is loading a large model, any other thread attempting to load a model (even a completely different one) will be blocked.

Instead of serializing the entire loading block, we can use a reference counter and a lock to manage the environment variables. This ensures that the environment variables are set to "1" when the first thread enters, and restored only when the last thread exits, allowing concurrent model loads to run in parallel.

Suggested change
# ``HF_HUB_OFFLINE`` is process-global. ``hf_offline_loading`` serializes its
# temporary toggle through this lock so concurrent loaders in the same process
# don't observe each other's env mutation. Model *loads* (not inference) thus
# run sequentially while the env is flipped — acceptable since loads are
# one-time and cached. (Mirrors the global-state-lock precedent of
# ``speechbrain_loading_cwd``.)
_hf_offline_lock = threading.RLock()
# ``HF_HUB_OFFLINE`` is process-global. To prevent concurrent loaders in the same
# process from racing on env mutation or prematurely restoring/unsetting the variables,
# we use a reference counter and a lock. This allows concurrent model loads to run
# in parallel without serializing them.
_hf_offline_counter = 0
_hf_offline_counter_lock = threading.Lock()
_hf_offline_saved_env = {}

Comment thread src/senselab/utils/dependencies.py Outdated
Comment on lines +417 to +428
with _hf_offline_lock:
saved = {var: os.environ.get(var) for var in _HF_OFFLINE_ENV_VARS}
for var in _HF_OFFLINE_ENV_VARS:
os.environ[var] = "1"
try:
yield True
finally:
for var, prev in saved.items():
if prev is None:
os.environ.pop(var, None)
else:
os.environ[var] = prev

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Update the context manager to use the reference counter and lock instead of holding the global lock for the entire duration of the block. This allows multiple threads to load models concurrently while safely managing the process-global environment variables.

    global _hf_offline_counter, _hf_offline_saved_env
    with _hf_offline_counter_lock:
        if _hf_offline_counter == 0:
            _hf_offline_saved_env = {var: os.environ.get(var) for var in _HF_OFFLINE_ENV_VARS}
            for var in _HF_OFFLINE_ENV_VARS:
                os.environ[var] = "1"
        _hf_offline_counter += 1

    try:
        yield True
    finally:
        with _hf_offline_counter_lock:
            _hf_offline_counter -= 1
            if _hf_offline_counter == 0:
                for var, prev in _hf_offline_saved_env.items():
                    if prev is None:
                        os.environ.pop(var, None)
                    else:
                        os.environ[var] = prev
                _hf_offline_saved_env.clear()

Comment on lines +90 to +91
processor = AutoProcessor.from_pretrained(model_name)
mdl = AutoModelForSpeechSeq2Seq.from_pretrained(model_name, dtype=dtype)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The revision parameter is not passed to AutoProcessor.from_pretrained and AutoModelForSpeechSeq2Seq.from_pretrained. If a custom revision is specified in model, hf_offline_loading will ensure that specific revision is cached and then engage offline mode. However, the subsequent loading calls will attempt to load the default revision ("main"), which will fail in offline mode (cache miss) or load the wrong version of the model.

Pass revision=revision to both loading calls to ensure the correct model version is loaded.

Suggested change
processor = AutoProcessor.from_pretrained(model_name)
mdl = AutoModelForSpeechSeq2Seq.from_pretrained(model_name, dtype=dtype)
processor = AutoProcessor.from_pretrained(model_name, revision=revision)
mdl = AutoModelForSpeechSeq2Seq.from_pretrained(model_name, dtype=dtype, revision=revision)

Comment on lines +71 to +72


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a unit test to verify that nested and concurrent invocations of hf_offline_loading work correctly with the reference counter implementation.

def test_offline_loading_nested_and_concurrent(cached: None, monkeypatch: pytest.MonkeyPatch) -> None:
    """Nested and concurrent offline loading blocks work correctly using the reference counter."""
    monkeypatch.delenv("HF_HUB_OFFLINE", raising=False)
    monkeypatch.delenv("TRANSFORMERS_OFFLINE", raising=False)

    with hf_offline_loading("model_a") as engaged_a:
        assert engaged_a is True
        assert os.environ["HF_HUB_OFFLINE"] == "1"

        with hf_offline_loading("model_b") as engaged_b:
            assert engaged_b is True
            assert os.environ["HF_HUB_OFFLINE"] == "1"

        assert os.environ["HF_HUB_OFFLINE"] == "1"

    assert "HF_HUB_OFFLINE" not in os.environ

…fix)

hf_offline_loading caches/locks the *requested* revision and flips HF to
offline, but AutoProcessor / AutoModelForSpeechSeq2Seq.from_pretrained were
called without `revision`, defaulting to "main" — an offline cache-miss (or
wrong-version load) for any non-main revision. Forward the resolved revision
to both loaders.

Adds a monkeypatched unit test asserting the revision reaches both loaders
(fails before the fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-pass

Addresses Gemini review + a revision-collision bug in the HF pipeline path:

- hf_offline_loading: replace the whole-block RLock (which serialized *all*
  in-process model loads) with a reference count — the first engaged loader
  captures the prior env and sets the offline vars, the last one out restores
  them. Concurrent cached loads now run in parallel; nested calls are correct
  without RLock reentrancy. Adds nested + concurrency (non-serialization) tests.
- huggingface._get_hf_asr_pipeline: `revision` was both placed in
  pipeline_kwargs and passed as load_hf_resilient's reserved `revision=` arg,
  raising "multiple values for keyword argument 'revision'" on every HF-pipeline
  transcription. Bind revision onto the loader via functools.partial and pass it
  to the cache step only. Adds a mocked regression test (the bug had no direct
  unit coverage).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@satra

satra commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

@wilke0818 - this seems weird. this was implemented before.

@wilke0818

Copy link
Copy Markdown
Collaborator Author

@wilke0818 - this seems weird. this was implemented before.

Claude quick response:

The problem this PR fixes: When lots of jobs load the same AI model at once, each one still "phones home" to Hugging Face to check the model version. Too many calls at once → Hugging Face throttles you (error 429) → the model fails to load. That's why Qwen kept coming back "missing."

Why the old code didn't fix it: There was already code to download each model only once. But (a) the model-loading paths didn't actually use it, and (b) it stopped duplicate downloads, not the version check that was causing the 429. So the fix wasn't aimed at the real culprit.

Honestly I don't love this solution. I think it would probably be better to make the version check and create a cache entry that can include a TTL or something similar. Part of it I think is also maybe a larger rework making sure that all models are going through a consistent API when being used under the senselab umbrella. Hopefully already happening but I'd want to do a more thorough check and rework of that later.

@satra

satra commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

indeed there should also be a TTL on the cache. somehow this PR is not passing my smell test.

@wilke0818

wilke0818 commented Jul 1, 2026 via email

Copy link
Copy Markdown
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants