Author SHA1 Message Date
grenadeandClaude Opus 5 b41964fae0 fix(serve): bound the drain so a restart cannot escalate to SIGABRT
ci / web (push) Successful in 1m24s
ci / check (push) Successful in 6m29s
`systemctl restart` during an in-flight blob transfer hung the drain until
`TimeoutStopSec` expired, at which point systemd escalated to SIGABRT and
the process core-dumped. Observed on bob during deploy 28:

    rustingface.service: Main process exited, code=dumped, status=6/ABRT
    rustingface.service: Failed with result 'timeout'.

axum's graceful shutdown waits for in-flight requests, and a blob response
*is* an in-flight request lasting size/bandwidth -- seven minutes for a
3.3GB shard at 8MB/s, against a 120s stop timeout. That wait cannot
converge, so the escalation was not an edge case but the normal outcome.

It also got more likely the moment #1 was fixed: a proxied blob previously
could not stream for longer than ~180s, so the size ceiling was doing the
job of a stop timeout by accident.

Cap the drain with `server.shutdown_grace` (15s) and exit cleanly when it
expires. Cutting a download deliberately is the better failure: the client
retries, which is a normal event, and nothing in the bucket depends on the
response, because a manifest entry is only written once its blob is
durable. TimeoutStopSec drops to 60s and is now only a backstop.

Verified against the live bucket with a transfer in flight: SIGTERM ->
clean exit 0 after exactly 15s, with a warning naming the reason. Before,
the same shape produced a two-minute stall and a core dump.

Closes #3

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L
2026-09-02 14:16:50 +03:00
grenadeandClaude Opus 5 f724d8f256 ci(deploy): a log diagnostic must not fail a deploy that worked
deploy / build-web (push) Successful in 1m33s
deploy / build (push) Successful in 6m36s
deploy / deploy (push) Successful in 2m26s
deploy / deploy-web (push) Successful in 15s
deploy-web failed on run 27 after every health check had passed. The
service was live, the frontend was serving, the public vhost was
answering and /metrics was closed -- and then the job went red because
`tail` could not read an nginx log.

5244585 dropped sudo here on the premise that "nginx's logs are
world-readable and /var/log/nginx is traversable". Half right:
/var/log/nginx is drwx--x--x, so traversable but not listable, and
logrotate creates the files 0640 nginx:root. The deploy account cannot
read them as itself at all, so the step could only ever have failed.

Try sudo, say so plainly when it is not permitted, and mark the step
continue-on-error. A step that exists to print context after a deploy
should never be able to fail one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L
2026-09-02 13:43:37 +03:00
grenadeandClaude Opus 5 b638862026 Merge branch 'fix/no-total-duration-cap-on-blob-transfers'
deploy / build-web (push) Successful in 1m40s
deploy / build (push) Successful in 6m35s
deploy / deploy (push) Successful in 20s
deploy / deploy-web (push) Failing after 11s
Closes #1

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L
2026-09-02 13:08:39 +03:00
grenadeandClaude Opus 5 5f91032e6f fix(core): a ranged fetch of a cold file left the client in silence
A ranged GET on a file the bucket does not hold used to fetch and store
the whole thing before sending a single byte. Asking for the first
megabyte of a 3.3GB shard therefore held the connection open, silent, for
the length of the entire transfer: 93 seconds measured against the live
service at 28MB/s, and 400-800s at the 8MB/s a GPU host gets. No client
waits that long for a first byte.

What makes it unrecoverable rather than merely slow is that
`huggingface_hub` resumes with a Range header. So the first failure
leaves an `.incomplete` file, and *every retry from then on* takes this
path and is answered with silence. The partial file never grows, its
mtime goes stale, and the retry loop cannot converge -- while the server
logs nothing but `stored blob and recorded manifest entry`, because from
its side the fetch succeeded.

The whole file must still be fetched and stored, since a fragment must
never reach the bucket. What must not happen is the client waiting on
that in silence. So narrow the tee instead of blocking on it: the client
is sent its slice as those bytes pass, one chunk behind as before, with
the last piece still withheld until the manifest write lands. For the
open-ended `bytes=N-` a resume actually asks for, that is a continuous
stream to the end of the file.

`blocking` and `LeadResult::Completed` go with it; nothing else used
them. The test asserting a ranged cold request stores the whole file and
never a fragment is unchanged and still passes.

Refs #1

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L
2026-09-02 13:03:49 +03:00
grenadeandClaude Opus 5 cfa53e14bd fix(data): a hidden 30s body timeout capped every proxied blob
`object_store` puts a 30-second `reqwest` timeout on the whole request,
body included. On a proxied blob that is not a timeout at all: the bucket
is read at the pace the client drains it, so it is a maximum servable
file size.

The reason it never looked like one is that object_store hides its own
consequences. It catches every response-body error and silently retries
with a resumed Range, so a large read does not fail at 30s -- it
reconnects every 30s and keeps going, until RetryConfig's 180s
retry_timeout is spent. Then the body simply ends, with a 200 already on
the wire and nothing logged anywhere. The ceiling is 180s x bandwidth: at
the 8MB/s a GPU host gets to caveman, no file over ~1.4GB can ever be
served, and no client retry converges because the next attempt is no
faster. Measured against the live bucket, 500MiB at 1MB/s: cut at 184.1s,
193,340,351 of 524,288,000 bytes. With the timeout disabled, 499.6s and
complete -- the same wall time curl takes reading the same object
straight from MinIO.

Disable it, and guard the stream the way a stream should be guarded:
`storage.connect_timeout` and an idle `storage.read_timeout` that bounds
silence rather than progress. A test asserts the total cap stays off.

Issue #1 named `server.request_timeout` as the cause. It was not -- that
key was parsed and then read by nothing, which is its own problem and is
why the hunt started 300 seconds away from a 180-second bug. Remove it.

Also log when a blob response ends before its Content-Length. Once the
headers are out the bytes leave through hyper, so a cut mid-body was
invisible on this side: the service logged a clean 200 while the client
saw a short read and retried forever.

Refs #1

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PqNtYNhov3fukx46KS9R7L
2026-09-02 13:03:34 +03:00
grenade 64c1e0e3ad Merge branch 'feat/allow-on-demand-mirroring'
deploy / build-web (push) Successful in 1m49s
deploy / build (push) Successful in 9m19s
deploy / deploy (push) Successful in 18s
deploy / deploy-web (push) Successful in 11s
2026-08-31 15:10:38 +03:00
15 changed files with 431 additions and 64 deletions
+9 -5
View File
@@ -360,14 +360,18 @@ jobs:
fi'
echo "frontend and API both answering through rf.internal"
# No sudo: nginx's logs are world-readable and /var/log/nginx is
# traversable, so the deploy account can read them as itself. Reaching
# for sudo here would have meant widening the scoped whitelist for
# something it does not need.
# A diagnostic, and diagnostics must not fail a deploy that worked --
# which is exactly what happened when this was assumed to be readable
# without sudo. /var/log/nginx is drwx--x--x (traversable but not
# listable) and logrotate creates the files 0640 nginx:root, so the
# deploy account cannot read them as itself. Try sudo, fall back to
# saying so, and never exit non-zero either way.
- name: nginx log
if: always() && steps.auth.outcome == 'success'
continue-on-error: true
run: |
for log in rf.internal rustingface.com; do
echo "--- /var/log/nginx/$log.error.log"
ssh "$PROXY_HOST" "tail -n 30 /var/log/nginx/$log.error.log"
ssh "$PROXY_HOST" "sudo -n tail -n 30 /var/log/nginx/$log.error.log 2>/dev/null \
|| echo '(not readable by the deploy account; add tail to its sudo allowlist to see this)'"
done
+10
View File
@@ -25,6 +25,16 @@ The five guarantees in `doc/spec.md` §1 are contractual. In particular:
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.
+15 -1
View File
@@ -12,8 +12,16 @@
# different host from the service (bob). firewalld bounds who may reach it.
listen = "{{LISTEN_ADDR}}"
external_url = "https://rf.internal"
request_timeout = "300s"
# There is no total-duration cap on a request, deliberately: a blob response
# lasts size/bandwidth, so one would be a maximum file size in disguise. Every
# timeout here is an idle timeout instead. This one bounds how long a client may
# stop reading before the tee detaches it and finishes the transfer anyway.
client_stall_timeout = "60s"
# How long in-flight responses may drain on SIGTERM before the service stops
# anyway. A blob response lasts size/bandwidth, so an unbounded drain never
# converges: systemd's TimeoutStopSec expires and escalates to SIGABRT. Cutting
# a download deliberately is the better failure -- the client retries.
shutdown_grace = "15s"
[storage]
endpoint = "{{S3_ENDPOINT}}"
@@ -26,6 +34,12 @@ secret_access_key_file = "/etc/rustingface/s3-secret-key"
# which takes rustingface out of the data path but exposes the object store's
# address to every client.
blob_delivery = "proxy"
connect_timeout = "10s"
# Idle only: a proxied blob is read from the bucket at the pace the client
# drains it, so this bounds silence from the object store, not the length of the
# read. Anything measuring the whole request would cap the size of file this
# service can serve.
read_timeout = "60s"
presign_ttl = "15m"
multipart_part_size = "16MiB"
+6 -3
View File
@@ -21,9 +21,12 @@ LoadCredential=s3-access-key:/etc/rustingface/s3-access-key
LoadCredential=s3-secret-key:/etc/rustingface/s3-secret-key
LoadCredential=hf-token:/etc/rustingface/hf-token
# Long transfers must not be killed mid-flight on stop; a detached upload that
# is cut short leaves an orphan blob, which is safe but wasteful.
TimeoutStopSec=120
# A backstop, not the mechanism. The service caps its own drain at
# server.shutdown_grace and then exits cleanly, because a blob response lasts
# size/bandwidth and waiting for one to finish does not converge -- letting this
# expire instead means SIGABRT and a core dump. Keep it comfortably above the
# configured grace so systemd never escalates first.
TimeoutStopSec=60
# Hardening. The process holds no durable state — the bucket is the whole of it
# — so there is no StateDirectory to protect and ProtectSystem can stay strict
+56 -4
View File
@@ -9,11 +9,14 @@ use std::time::Instant;
use axum::body::Body as AxumBody;
use axum::extract::State;
use axum::response::{IntoResponse, Response};
use futures::StreamExt;
use bytes::Bytes;
use futures::stream::BoxStream;
use futures::{Stream, StreamExt};
use http::{HeaderMap, HeaderValue, StatusCode, header};
use rustingface_core::{Body, ResolvedFile, Source};
use rustingface_entities::error::{Error, Result};
use rustingface_entities::repo::RepoType;
use tracing::warn;
use crate::error::ApiError;
use crate::path::{ResolvePath, parse_resolve};
@@ -126,9 +129,12 @@ pub async fn get(
};
telemetry::served(source, len);
telemetry::request("resolve_get", status.as_u16());
let body = AxumBody::from_stream(
stream.map(|chunk| chunk.map_err(|err| std::io::Error::other(err.to_string()))),
);
let body = AxumBody::from_stream(observed(
stream,
len,
source,
format!("{repo_id}/{filename}"),
));
Ok((status, headers, body).into_response())
}
Body::None => Err(ApiError(Error::Internal(
@@ -137,6 +143,52 @@ pub async fn get(
}
}
/// Wrap a body stream so a response that ends before its `Content-Length` says
/// so on this side of the wire.
///
/// Once the headers are out the bytes leave through hyper, and a handler that
/// has already returned cannot report anything. Without this, a transfer cut
/// mid-body — a storage read that died, a client that vanished — is logged as a
/// successful 200 while the client sees a short read and retries forever. That
/// asymmetry is what makes such a failure read as "the client is flaky".
fn observed(
stream: BoxStream<'static, Result<Bytes>>,
expected: u64,
source: Source,
what: String,
) -> impl Stream<Item = std::result::Result<Bytes, std::io::Error>> {
futures::stream::unfold(
(stream, 0u64, what),
move |(mut stream, sent, what)| async move {
match stream.next().await {
Some(Ok(chunk)) => {
let sent = sent + chunk.len() as u64;
Some((Ok(chunk), (stream, sent, what)))
}
Some(Err(err)) => {
warn!(
path = %what, sent, expected, %err,
"blob response failed mid-body; the client sees a short read, not an error"
);
telemetry::truncated(source, sent);
let err = std::io::Error::other(err.to_string());
Some((Err(err), (stream, sent, what)))
}
None => {
if sent < expected {
warn!(
path = %what, sent, expected,
"blob response ended early with no error; the client sees a short read"
);
telemetry::truncated(source, sent);
}
None
}
}
},
)
}
/// Build the header set both `HEAD` and `GET` must carry.
///
/// `Content-Encoding` is deliberately absent and must stay that way: the
+14
View File
@@ -34,6 +34,20 @@ pub fn served(source: Source, bytes: u64) {
counter!("rustingface_bytes_served_total", "source" => label).increment(bytes);
}
/// Record a blob response that ended before its `Content-Length`.
///
/// The bytes leave through hyper rather than through a handler that can return
/// a status, so a cut mid-body is otherwise invisible on this side: the client
/// retries forever and the service logs a clean 200.
pub fn truncated(source: Source, bytes: u64) {
let label = match source {
Source::Bucket => "bucket",
Source::Upstream => "upstream",
};
counter!("rustingface_blob_truncations_total", "source" => label).increment(1);
counter!("rustingface_bytes_truncated_total", "source" => label).increment(bytes);
}
/// Record the outcome of an upstream fetch.
pub fn upstream_fetch(result: &'static str, seconds: f64) {
counter!("rustingface_upstream_fetches_total", "result" => result).increment(1);
@@ -364,6 +364,75 @@ async fn a_client_that_disconnects_mid_transfer_still_leaves_the_file_stored() {
assert_eq!(served, weights);
}
#[tokio::test]
async fn a_cold_ranged_request_streams_while_the_file_is_still_being_fetched() {
// The failure this pins down: a ranged GET on a file the bucket does not
// hold used to fetch and store the whole thing before sending a single
// byte. A client asking for a 3GB shard therefore saw an open connection
// and total silence for the length of the transfer, which its read timeout
// ends long before -- and because `hf download` resumes with a Range
// header, *every retry after the first* took that path. The retry loop
// could never converge, and the server logged nothing but success.
//
// The whole file must still be stored (never a fragment); what must not
// happen is the client waiting on that in silence.
let hub = FakeHub::start().await;
let weights = publish_model(&hub).await;
hub.throttle(8 * 1024, Duration::from_millis(5)).await;
let bucket = bucket_dir("cold-range-streams");
let rf = instance(&bucket, Some(&hub), |_| {});
// Resume from an offset, open-ended, which is the shape huggingface_hub
// actually sends.
let response = rf
.app
.clone()
.oneshot(
Request::builder()
.uri("/Qwen/Qwen3-32B/resolve/main/model.safetensors")
.header("range", "bytes=1000-")
.body(Body::empty())
.unwrap(),
)
.await
.unwrap();
assert_eq!(response.status(), StatusCode::PARTIAL_CONTENT);
assert_eq!(
response.headers()["content-range"],
format!("bytes 1000-{}/{}", weights.len() - 1, weights.len())
);
use futures::StreamExt;
let mut body = response.into_body().into_data_stream();
let first = body
.next()
.await
.expect("a chunk before the fetch ends")
.unwrap();
assert!(
first.len() < weights.len() - 1000,
"the first chunk arrived while the transfer was still running, not after it"
);
let mut served = first.to_vec();
while let Some(chunk) = body.next().await {
served.extend_from_slice(&chunk.unwrap());
}
assert_eq!(served, weights[1000..], "and the range itself is intact");
// The bucket still holds the whole file, not the slice that was served.
let oid = {
use sha2::{Digest, Sha256};
hex::encode(Sha256::digest(&weights))
};
assert_eq!(
std::fs::read(bucket.join(key::blob(&oid))).unwrap(),
weights,
"a ranged cold request stores the whole file, never a fragment"
);
}
#[tokio::test]
async fn a_ranged_request_resumes_from_the_offset_it_asks_for() {
let hub = FakeHub::start().await;
+45 -13
View File
@@ -6,6 +6,7 @@
//! Ctrl-C at 30GB does not discard 30GB: the retention guarantee is better
//! served by finishing (spec §7).
use std::ops::Range;
use std::sync::Arc;
use bytes::Bytes;
@@ -36,16 +37,19 @@ enum Sink {
impl Registry {
/// Lead an upstream fetch for one file.
///
/// `blocking` forces the transfer to run to completion before returning,
/// which is what a ranged request on a cold file needs: a partial blob
/// must never reach the bucket.
/// `client_range` narrows what the *client* is sent; the whole file is
/// fetched and stored either way, because a partial blob must never reach
/// the bucket. Narrowing the tee rather than waiting for the store is what
/// keeps a ranged request on a cold file from sitting in silence for the
/// length of a multi-gigabyte transfer — which is what a resumed
/// `hf download` asks for on every retry after its first failure.
pub(crate) async fn lead_fetch(
&self,
repo_type: RepoType,
repo_id: &RepoId,
commit: &str,
path: &str,
blocking: bool,
client_range: Option<Range<u64>>,
) -> Result<LeadResult> {
let body = self
.upstream()
@@ -82,16 +86,9 @@ impl Registry {
sink,
part_size: self.config().storage.multipart_part_size,
stall_timeout: self.config().server.client_stall_timeout,
client_range: client_range.clone(),
};
if blocking {
// Nobody is reading the tee, so drop the client half up front and
// let the transfer run at full speed.
drop(client_rx);
let entry = transfer.run(self, body.stream, client_tx).await?;
return Ok(LeadResult::Completed(entry));
}
let registry = self.clone();
tokio::spawn(async move {
let result = transfer.run(&registry, body.stream, client_tx).await;
@@ -106,6 +103,7 @@ impl Registry {
head,
rx: client_rx,
task: FetchTask::new(outcome_rx),
range: client_range,
})
}
@@ -137,6 +135,8 @@ struct Transfer {
sink: Sink,
part_size: u64,
stall_timeout: std::time::Duration,
/// Which bytes of the file the client asked for. `None` is the whole file.
client_range: Option<Range<u64>>,
}
impl Transfer {
@@ -166,19 +166,33 @@ impl Transfer {
}
};
// Where the next chunk starts in the file, so a narrowed tee can work
// out which slice of it the client asked for.
let mut offset: u64 = 0;
let transfer = async {
while let Some(chunk) = stream.next().await {
let chunk = chunk?;
hasher.update(&chunk);
let start = offset;
offset += chunk.len() as u64;
match &mut writer {
Some(writer) => writer.write(chunk.clone()).await?,
None => buffered.push(chunk.clone()),
}
// The client's share of this chunk. The whole chunk when it
// asked for the whole file; the overlap with its range
// otherwise; nothing at all for a chunk outside it.
let Some(share) = self.client_share(&chunk, start, offset) else {
continue;
};
// Tee to the client one chunk behind, dropping the client --
// not the transfer -- if it has gone away or stopped reading.
if let Some(previous) = holdback.replace(chunk)
if let Some(previous) = holdback.replace(share)
&& let Some(tx) = &client
{
match tokio::time::timeout(self.stall_timeout, tx.send(Ok(previous))).await {
@@ -316,6 +330,24 @@ impl Transfer {
}
}
impl Transfer {
/// The part of `chunk` — which spans `[start, end)` of the file — that the
/// client asked for, or `None` when none of it is.
///
/// Slicing is cheap: `Bytes` shares the buffer rather than copying it.
fn client_share(&self, chunk: &Bytes, start: u64, end: u64) -> Option<Bytes> {
let Some(range) = &self.client_range else {
return Some(chunk.clone());
};
let from = start.max(range.start);
let to = end.min(range.end);
if from >= to {
return None;
}
Some(chunk.slice((from - start) as usize..(to - start) as usize))
}
}
/// Build the manifest entry for a completed transfer, carrying upstream's
/// values through unchanged.
fn entry_from(path: &str, size: u64, hex: &str, head: &UpstreamHead) -> Entry {
+19 -19
View File
@@ -734,7 +734,7 @@ impl Registry {
}
Role::Leader(lead) => {
let outcome = self
.lead_fetch(repo_type, repo_id, &commit, path, range.is_some())
.lead_fetch(repo_type, repo_id, &commit, path, range.clone())
.await;
match outcome {
Err(err) => {
@@ -753,12 +753,25 @@ impl Registry {
})?;
Ok((file, Source::Bucket))
}
Ok(LeadResult::Streaming { head, rx, task }) => {
Ok(LeadResult::Streaming {
head,
rx,
task,
range,
}) => {
// The upload task owns the flight from here: it
// publishes the outcome whether or not this client
// stays connected.
task.attach(lead);
let size = head.linked_size.or(head.content_length).unwrap_or(0);
// A ranged request is answered from the same tee,
// narrowed. The whole file is still fetched and stored
// -- a fragment must never reach the bucket -- but the
// client is not made to wait for that in silence.
let len = match &range {
Some(range) => range.end - range.start,
None => size,
};
Ok((
ResolvedFile {
commit,
@@ -768,25 +781,13 @@ impl Registry {
linked_size: head.linked_size,
body: Body::Stream {
stream: Box::pin(tokio_stream_of(rx)),
len: size,
range: None,
len,
range,
},
},
Source::Upstream,
))
}
Ok(LeadResult::Completed(entry)) => {
lead.finish(Outcome::Stored(entry.clone()));
let file = self
.serve_stored(&entry, &commit, range)
.await?
.ok_or_else(|| {
Error::Storage(format!(
"the blob for {repo_id}@{commit}/{path} vanished after being stored"
))
})?;
Ok((file, Source::Upstream))
}
}
}
}
@@ -898,11 +899,10 @@ pub(crate) enum LeadResult {
Streaming {
head: UpstreamHead,
rx: mpsc::Receiver<Result<Bytes>>,
/// The slice of the file the tee is narrowed to, if any.
range: Option<Range<u64>>,
task: FetchTask,
},
/// The transfer ran to completion before the response was built, because
/// the client asked for a range and a partial blob may not be stored.
Completed(Entry),
}
/// Handle to a detached transfer, which owns the flight once attached.
+62 -3
View File
@@ -17,8 +17,8 @@ use object_store::local::LocalFileSystem;
use object_store::path::Path;
use object_store::signer::Signer;
use object_store::{
GetOptions, GetRange, ObjectStore, ObjectStoreExt, PutMode, PutOptions, PutPayload,
UpdateVersion, WriteMultipart,
ClientOptions, GetOptions, GetRange, ObjectStore, ObjectStoreExt, PutMode, PutOptions,
PutPayload, UpdateVersion, WriteMultipart,
};
use rustingface_core::ports::{BlobRead, BlobWriter, Store, StoredObject, Version};
use rustingface_entities::config::Storage as StorageConfig;
@@ -63,7 +63,8 @@ impl ObjectStoreAdapter {
let mut builder = AmazonS3Builder::new()
.with_bucket_name(&cfg.bucket)
.with_region(&cfg.region)
.with_virtual_hosted_style_request(!cfg.path_style);
.with_virtual_hosted_style_request(!cfg.path_style)
.with_client_options(Self::client_options(cfg));
if let Some(endpoint) = &cfg.endpoint {
// MinIO on the mesh is reached over plain HTTP; the mesh itself is
@@ -93,6 +94,36 @@ impl ObjectStoreAdapter {
})
}
/// HTTP client settings for the S3 backend.
///
/// `object_store` defaults to a 30-second timeout on the *whole* request,
/// body included, and that default must stay disabled here. A proxied blob
/// is read from the bucket at the pace the client drains it, so a cap on
/// the whole request is not a timeout at all -- it is a maximum servable
/// file size.
///
/// The way it fails is worth knowing, because none of it is visible from
/// outside. The 30s deadline kills the response body mid-stream;
/// `object_store` catches every body error and silently retries, resuming
/// with a `Range` from where it stopped (`client/get.rs`, "Retry all
/// response body errors"). So a large read does not fail at 30s -- it
/// quietly reconnects every 30s and keeps going, until `RetryConfig`'s
/// 180s `retry_timeout` is exhausted. Then the body simply ends, with a
/// 200 already on the wire and nothing logged. The ceiling is therefore
/// `180s x bandwidth`: at 8MB/s, no file over ~1.4GB can ever be served,
/// and no client retry can converge because the next attempt is no faster.
///
/// The guard that belongs on a stream is an idle one, which bounds silence
/// rather than progress and resets on every chunk. `read_timeout` covers
/// the wait for response headers too, so a hung backend still fails a
/// small metadata request promptly.
fn client_options(cfg: &StorageConfig) -> ClientOptions {
ClientOptions::new()
.with_timeout_disabled()
.with_connect_timeout(cfg.connect_timeout)
.with_read_timeout(cfg.read_timeout)
}
/// A filesystem-backed adapter, for tests.
pub fn local(path: &FsPath) -> Result<Self> {
std::fs::create_dir_all(path)
@@ -363,3 +394,31 @@ impl BlobWriter for MultipartBlobWriter {
.map_err(|e| storage(&format!("aborting upload of {key}"), e))
}
}
#[cfg(test)]
mod tests {
use super::*;
use object_store::ClientConfigKey;
use object_store::aws::AmazonS3ConfigKey;
/// A total-duration timeout on an S3 request is a maximum servable file
/// size, because a proxied blob is read at the pace the client drains it.
/// `object_store` ships one on by default (30s); it must be off here, and
/// the guards that remain must be idle ones.
#[test]
fn the_s3_client_has_no_total_request_timeout() {
let cfg = StorageConfig {
endpoint: Some("http://localhost:9000".into()),
connect_timeout: Duration::from_secs(10),
read_timeout: Duration::from_secs(60),
..Default::default()
};
let builder =
AmazonS3Builder::new().with_client_options(ObjectStoreAdapter::client_options(&cfg));
let get = |key| builder.get_config_value(&AmazonS3ConfigKey::Client(key));
assert_eq!(get(ClientConfigKey::Timeout), None);
assert_eq!(get(ClientConfigKey::ConnectTimeout).as_deref(), Some("10s"));
assert_eq!(get(ClientConfigKey::ReadTimeout).as_deref(), Some("1m"));
}
}
+34 -6
View File
@@ -32,6 +32,13 @@ pub struct Config {
}
/// HTTP listener settings.
///
/// There is deliberately no total-duration cap on a request. A blob response
/// lasts `size / bandwidth`, so a ceiling on the whole request is a ceiling on
/// file size in disguise: the same client asking for the same 3GB shard
/// succeeds on a fast link and fails on every retry on a slow one. Every guard
/// here and in [`Storage`] and [`Upstream`] is therefore an *idle* timeout,
/// which bounds silence rather than progress.
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(default, deny_unknown_fields)]
pub struct Server {
@@ -40,13 +47,20 @@ pub struct Server {
pub listen: String,
/// How this service is reached from outside, used to build absolute URLs.
pub external_url: Option<String>,
/// Ceiling on a single request, including a cold multi-gigabyte fetch.
#[serde(with = "humantime_serde")]
pub request_timeout: Duration,
/// How long a stalled client may hold up the tee before the transfer is
/// detached and finished without it.
#[serde(with = "humantime_serde")]
pub client_stall_timeout: Duration,
/// How long to let in-flight requests drain on SIGTERM before stopping
/// anyway.
///
/// A blob response is an in-flight request that lasts `size / bandwidth`,
/// so an unbounded drain does not converge on a restart: systemd's
/// `TimeoutStopSec` expires and escalates to SIGABRT, which is a core dump
/// rather than a shutdown. Cutting a download deliberately is the better
/// failure -- clients retry, and nothing in the bucket depends on it.
#[serde(with = "humantime_serde")]
pub shutdown_grace: Duration,
}
impl Default for Server {
@@ -54,8 +68,8 @@ impl Default for Server {
Self {
listen: "127.0.0.1:20482".into(),
external_url: None,
request_timeout: Duration::from_secs(300),
client_stall_timeout: Duration::from_secs(60),
shutdown_grace: Duration::from_secs(15),
}
}
}
@@ -84,6 +98,15 @@ pub struct Storage {
pub access_key_id_file: Option<PathBuf>,
pub secret_access_key_file: Option<PathBuf>,
pub blob_delivery: BlobDelivery,
/// How long to wait for a TCP connection to the object store.
#[serde(with = "humantime_serde")]
pub connect_timeout: Duration,
/// How long the object store may go silent mid-response before the read is
/// abandoned. Idle only, and it resets on every chunk: a proxied blob is
/// read at the client's pace, so anything measuring the whole request
/// instead would cap the size of file this service can serve.
#[serde(with = "humantime_serde")]
pub read_timeout: Duration,
#[serde(with = "humantime_serde")]
pub presign_ttl: Duration,
#[serde(deserialize_with = "de_byte_size")]
@@ -104,6 +127,8 @@ impl Default for Storage {
access_key_id_file: None,
secret_access_key_file: None,
blob_delivery: BlobDelivery::Proxy,
connect_timeout: Duration::from_secs(10),
read_timeout: Duration::from_secs(60),
presign_ttl: Duration::from_secs(15 * 60),
multipart_part_size: 16 * 1024 * 1024,
manifest_write_retries: 5,
@@ -353,7 +378,7 @@ mod tests {
[server]
listen = "127.0.0.1:20482"
external_url = "https://rf.internal"
request_timeout = "300s"
client_stall_timeout = "60s"
[storage]
endpoint = "http://caveman.kosherinata.internal:9000"
@@ -361,6 +386,8 @@ mod tests {
region = "us-east-1"
path_style = true
blob_delivery = "proxy"
connect_timeout = "10s"
read_timeout = "60s"
presign_ttl = "15m"
multipart_part_size = "16MiB"
@@ -387,7 +414,8 @@ mod tests {
cfg.validate().unwrap();
assert_eq!(cfg.storage.multipart_part_size, 16 * 1024 * 1024);
assert_eq!(cfg.policy.ref_resolution, RefResolution::Freeze);
assert_eq!(cfg.server.request_timeout, Duration::from_secs(300));
assert_eq!(cfg.server.client_stall_timeout, Duration::from_secs(60));
assert_eq!(cfg.storage.read_timeout, Duration::from_secs(60));
assert!(!cfg.sealed());
}
+10
View File
@@ -51,6 +51,16 @@ pub async fn run(config: rustingface_entities::config::Config) -> Result<()> {
}
);
println!(" blob delivery {:?}", config.storage.blob_delivery);
// Named here because the failure they prevent looks like a client problem:
// a total-duration cap on a blob read is a maximum servable file size, and
// an operator chasing one needs to see that these are idle guards.
println!(
" idle guards storage {:?}, upstream {:?}, client {:?} \
(no total-duration cap)",
config.storage.read_timeout,
config.upstream.read_timeout,
config.server.client_stall_timeout,
);
println!(
" part size {}",
human_bytes(config.storage.multipart_part_size)
+33 -8
View File
@@ -70,7 +70,7 @@ pub async fn run(config: Config) -> Result<()> {
let app = router(AppState::new(registry, tokens.into(), metrics));
axum::serve(listener, app)
.with_graceful_shutdown(shutdown())
.with_graceful_shutdown(shutdown(config.server.shutdown_grace))
.await
.context("serving")
}
@@ -87,12 +87,22 @@ fn token_file_has_content(config: &Config) -> bool {
})
}
/// Stop accepting on `SIGTERM`/`SIGINT` and let in-flight requests drain.
/// Stop accepting on `SIGTERM`/`SIGINT` and let in-flight requests drain --
/// for a bounded time.
///
/// Detached transfers are not requests and are not waited on here: the process
/// exiting mid-upload leaves an orphan blob, which is safe and reclaimable,
/// while a manifest is only ever written after an upload completes.
async fn shutdown() {
/// The bound is the point. A blob response *is* an in-flight request and it
/// lasts `size / bandwidth`, so waiting for one to finish does not converge:
/// a 3GB shard at 8MB/s holds the drain open for seven minutes, systemd's
/// `TimeoutStopSec` expires long before that, and the service is killed with
/// SIGABRT and a core dump. Cutting the download deliberately after
/// `server.shutdown_grace` is strictly better -- the client retries, which is
/// a normal event, and the restart is prompt.
///
/// Detached transfers are not requests and are not waited on here either: the
/// process exiting mid-upload leaves an orphan blob, which is safe and
/// reclaimable, while a manifest is only ever written after an upload
/// completes.
async fn shutdown(grace: std::time::Duration) {
let mut term = match signal(SignalKind::terminate()) {
Ok(signal) => signal,
Err(err) => {
@@ -101,11 +111,26 @@ async fn shutdown() {
}
};
tokio::select! {
_ = term.recv() => info!("SIGTERM received; draining"),
_ = term.recv() => info!(?grace, "SIGTERM received; draining"),
result = tokio::signal::ctrl_c() => {
if result.is_ok() {
info!("interrupt received; draining");
info!(?grace, "interrupt received; draining");
}
}
}
// Returning here only stops the listener accepting; axum then waits on
// every in-flight response, blob streams included. This watchdog is what
// makes that wait bounded.
tokio::spawn(async move {
tokio::time::sleep(grace).await;
tracing::warn!(
?grace,
"in-flight responses did not finish within server.shutdown_grace; \
stopping anyway and cutting them. Clients retry; stored blobs are \
unaffected, because a manifest entry is only written once its blob \
is durable."
);
std::process::exit(0);
});
}
+49 -1
View File
@@ -225,7 +225,8 @@ config.
[server]
listen = "127.0.0.1:8080"
external_url = "https://models.internal.example" # used to build redirect URLs
request_timeout = "300s"
client_stall_timeout = "60s" # idle guard on the tee; see below
shutdown_grace = "15s" # bounded drain on SIGTERM; see below
[storage]
endpoint = "https://s3.internal.example:9000"
@@ -235,6 +236,8 @@ path_style = true
access_key_id_file = "/etc/rustingface/s3-access-key"
secret_access_key_file = "/etc/rustingface/s3-secret-key"
blob_delivery = "proxy" # proxy | redirect
connect_timeout = "10s"
read_timeout = "60s" # idle guard; see below
presign_ttl = "15m" # redirect mode only
multipart_part_size = "16MiB"
@@ -260,6 +263,51 @@ metrics = true # /metrics, Prometheus text format
log_format = "json" # json | text
```
### Timeouts
Every timeout in this configuration is an **idle** timeout: it bounds silence,
never progress. There is deliberately no ceiling on how long a request may
take.
A blob response lasts `size / bandwidth`. A total-duration cap on it is
therefore not a timeout but a maximum servable file size, and one expressed in
the wrong units: the same client asking for the same file succeeds over a fast
link and fails on every retry over a slow one, with no diagnostic difference
between the two. A retry loop cannot converge against it, because nothing about
the next attempt is any faster.
- `server.client_stall_timeout` — how long a client may stop reading before the
tee detaches it and finishes the transfer without it.
- `storage.read_timeout` — how long the object store may go silent mid-response.
It resets on every chunk, and also bounds the wait for response headers, so a
hung backend still fails a small metadata request promptly.
- `upstream.read_timeout` — the same guard on the Hub side.
`server.shutdown_grace` is the one bound that is not idle-based, and
deliberately so. It caps how long in-flight responses may drain on SIGTERM
before the process stops regardless. A blob response is an in-flight request
lasting `size / bandwidth`, so an unbounded drain does not converge: systemd's
`TimeoutStopSec` expires first and escalates to `SIGABRT`. Cutting a download
deliberately is the better failure — the client retries, and nothing in the
bucket depends on the response, because a manifest entry is only ever written
once its blob is durable.
This applies to the object-storage client in particular. `object_store`
defaults to a 30-second cap on the whole request, body included, and then hides
its own consequences: it catches the resulting body errors and silently retries
with a resumed range, so a large read reconnects every 30 seconds until the
180-second `retry_timeout` is exhausted, at which point the body ends with a 200
already on the wire and nothing logged. The adapter must disable that cap
explicitly, or every proxied blob over `180s x bandwidth` fails deterministically
and forever -- at 8MB/s, anything over about 1.4GB.
A ranged request on a file the bucket does not hold is subject to the same
principle from the other side. The whole file is still fetched and stored, since
a fragment must never be stored, but the client is sent its slice as those bytes
pass rather than being made to wait in silence for the transfer to end. A
resumed `hf download` asks for exactly that shape on every retry, and silence
there is indistinguishable to it from a dead server.
### `policy.ref_resolution`
- `freeze` (default) — the first resolution of a mutable ref is recorded and
-1
View File
@@ -134,7 +134,6 @@ class Instance:
f"""
[server]
listen = "127.0.0.1:{self.port}"
request_timeout = "600s"
[storage]
{storage_lines}