grenade/rustingface

Fork 0

Files

rob thijssen 9b35dfdc09

ci / web (push) Successful in 1m36s

Details

ci / check (push) Successful in 6m24s

Details

fix(core): a cold range that stops short of the end no longer stalls

Since #1 a ranged GET on a file the bucket does not hold streams its slice
from the tee instead of waiting for the whole transfer. For a range that
runs to the end of the file -- what a resumed `hf download` sends -- that
is exactly right, and it stays.

For a range that stops short of the end it is not enough. The slice
arrives quickly and then the *last chunk* is withheld until the manifest
write lands, which waits on every remaining byte, because the digest is
verified only once the whole file has passed. Measured against the live
service, asking for the first 1MiB of a 3.3GB shard:

    http=206 ttfb=1.040851s total=120.001638s bytes=1048169

1,048,169 of 1,048,576 bytes in about a second, then an idle connection
for the rest of the transfer. The client's read timeout ends it long
before, so in practice the request fails anyway -- slowly and with no
explanation.

The holdback is not the thing to change: releasing that tail early would
signal "durable and recorded" over bytes that are neither, which is
precisely what it exists to prevent. So answer 503 with Retry-After
instead, and let the fetch run on detached; the retry is served from the
bucket. `hf_transfer` splits every download into bounded ranged chunks, so
this shape is not hypothetical wherever it is enabled.

The existing ranged test now asserts the two-step behaviour and keeps its
guarantee that the bucket holds the whole file and never a fragment. A new
test pins that a resume to EOF is still served from the tee, so the fix for
#1 cannot be undone by this one.

Closes #4

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L

2026-09-02 14:24:37 +03:00

6.2 KiB

Raw Permalink Blame History

rustingface — notes for agents

Read doc/spec.md first; it is the contract, not a sketch. Then read ~/git/architecture/generic.md for the conventions this repo inherits (workspace layout, deployment, systemd, firewalld, SELinux, commits) and port-allocations.md for why the port is 20482.

What must not regress

The five guarantees in doc/spec.md §1 are contractual. In particular:

Never normalise a replayed header. ETag, X-Repo-Commit, X-Linked-Etag and X-Linked-Size are recorded as upstream sent them and emitted unchanged, quotes included. Client cache layouts are keyed on these. Do not compute a git blob SHA-1, derive an etag from content, or decide what a commit "should" be.
Never let a redirect eat the Hub's headers. The Hub answers a resolve request with a redirect and puts the replayable metadata on that response; the CDN it points at has none of it. rustingface-data's upstream client follows redirects by hand for this reason. If you ever re-enable reqwest's own redirect policy, every recorded etag silently becomes wrong.
Never set Content-Encoding on the resolve path. The client sends Accept-Encoding: identity precisely so the sizes it learns are real.
Never write a manifest entry before its blob upload has completed. The ordering in rustingface-core/src/fetch.rs is deliberate: a crash may leak an orphan blob, which gc reclaims, but a manifest entry pointing at incomplete bytes is a silent correctness failure.
Never put a total-duration cap on a blob transfer. A blob response lasts size / bandwidth, so a ceiling on the whole request is a maximum servable file size wearing a timeout's clothes, and no client retry can converge against it. Every guard on this path is an idle timeout — server.client_stall_timeout, storage.read_timeout, upstream.read_timeout. object_store defaults to a 30s total request timeout and silently retries the body errors it causes until its 180s retry_timeout runs out, which caps a proxied blob at 180s x bandwidth and logs nothing at all; rustingface-data's S3 client disables that timeout explicitly, and there is a test asserting it stays disabled.
Never complete a client response before the manifest write lands. The tee runs one chunk behind for this reason. A file that was served but not recorded is invisible the moment the instance is sealed. This is also why a cold range that stops short of the end is answered 503 rather than served: its tail could only be released early by signalling "durable and recorded" over bytes that are neither.

Testing

cargo test --workspace covers it, including tests/sovereignty.rs, which runs the real service against a fake Hub speaking the real header protocol. Add a case there for any behaviour a client depends on.

The conformance suite (test/conformance/) needs network and a pinned huggingface_hub. Run it before claiming a change is safe for clients — most of the real bugs in this repo's history were found by it and not by the unit tests. It has also been run against the live MinIO on caveman; do that for anything touching rustingface-data, because the local filesystem store hides whole classes of problem (it has no multipart, no conditional update, and no presigning).

The web UI

web/ is a Vite + React + SWC + TS app (architecture/generic.md §4), served statically from the nginx host and reading the /v1/ API.

Keep UI routes out of /api/. That surface is the Hub's, replayed verbatim. Anything rustingface invents goes under /v1/.
Model cards are untrusted third-party content. components/Markdown.tsx parses their HTML and then sanitises it; the plugin order (rehype-raw, then rehype-sanitize) is not interchangeable, and reversing it would reintroduce the raw HTML unchecked. Card images are deliberately not loaded.
Any new route must not shadow a repository id. rustingface owns the whole URL root, which is the Hub's namespace, so every prefix collides with some repo. Where it is ambiguous, disambiguate in the handler the way v1_repo_detail does — and test it against /resolve/ in a directory name, not just in a repo name.

Auth, and what actually bounds the bucket

Two credentials that are easy to confuse, pointing in opposite directions:

upstream.token_file (hf-token) authenticates rustingface to the Hub. Outbound only, never echoed downstream, never logged. Needed for gated repos.
auth.token_file (client-tokens) authenticates clients to rustingface.

A client's token is never forwarded upstream, and rustingface's token is never shown to a client. Do not let these merge.

auth.mode = "bearer" controls who, not what — a token holder can still cause any repository to be fetched. policy.allow_new_repos is what decides whether that matters, and the deployment now sets it true: an authenticated request for a repository the bucket does not hold fetches it from upstream and stores it on the way past. That is deliberate — it is how a consumer pointed here by HF_ENDPOINT mirrors what it uses without an operator pre-seeding anything.

The consequence is that the bucket is bounded by auth, not by policy. Every issued client token is a licence to store arbitrary upstream repositories, so treat client-tokens as a capability list rather than a convenience. If that bound is ever needed back, policy.allowlist is the setting that keeps on-demand mirroring while confining it to expected namespaces; turning allow_new_repos off again makes growth an operator action instead.

auth.anonymous = "catalog" opens /v1/ to unauthenticated requests. That is only safe while every handler under /v1/ reads the bucket and never consults upstream. If you add a route there that can fetch, you have made the public face able to fill the bucket — there is a test asserting the whole surface causes zero upstream requests, and it should stay that way.

Where things live

Types in entities, logic in core, I/O in data, HTTP in api, wiring in the binary. core defines the Store and Upstream ports and must not gain a direct dependency on object_store or reqwest.

6.2 KiB Raw Permalink Blame History