From 0713a723cd429d64fab583b4da073ff90f7ae5e4 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Thu, 3 Sep 2026 19:40:54 +0800 Subject: [PATCH] fix(odm): close three on-demand migration follow-ups from the e2e matrix (#7101) --- .../on_demand_migration/interaction_test.rs | 4 +- crates/ecstore/src/api/mod.rs | 2 +- .../src/bucket/on_demand_migration/mod.rs | 2 +- .../src/bucket/on_demand_migration/pull.rs | 51 ++++++++++++++++++- .../on_demand_migration/source_client.rs | 18 +++++-- .../src/bucket/on_demand_migration/stats.rs | 8 ++- .../fixtures/on_demand_migration/status.json | 2 +- .../status_with_backfill.json | 2 +- .../src/metrics/schema/on_demand_migration.rs | 3 +- docs/operations/on-demand-migration.md | 13 ++--- rustfs/src/app/object/get.rs | 14 ++++- .../src/app/object/on_demand_migration_put.rs | 4 +- rustfs/src/app/storage_api.rs | 3 +- 13 files changed, 96 insertions(+), 30 deletions(-) diff --git a/crates/e2e_test/src/on_demand_migration/interaction_test.rs b/crates/e2e_test/src/on_demand_migration/interaction_test.rs index c810c4f4b..93cafc05c 100644 --- a/crates/e2e_test/src/on_demand_migration/interaction_test.rs +++ b/crates/e2e_test/src/on_demand_migration/interaction_test.rs @@ -238,9 +238,7 @@ async fn test_odm_write_back_respects_the_bucket_quota() -> TestResult { assert_eq!(response.header(ODM_RESPONSE_HEADER), Some("source")); assert_eq!(response.body, body, "a full bucket still serves the client from the source"); - // ODM-05 fixed the failure-reason label set without a `quota` value, so a - // rejected admission is reported as a local write failure. - env.wait_for_status_counter(bucket, "/counters/pull_failures_total/local_write", 1, SETTLE) + env.wait_for_status_counter(bucket, "/counters/pull_failures_total/quota", 1, SETTLE) .await?; env.assert_local_absent(bucket, key).await; assert_eq!( diff --git a/crates/ecstore/src/api/mod.rs b/crates/ecstore/src/api/mod.rs index 46f91ebd5..1e63ed571 100644 --- a/crates/ecstore/src/api/mod.rs +++ b/crates/ecstore/src/api/mod.rs @@ -163,7 +163,7 @@ pub mod bucket { pub use crate::bucket::on_demand_migration::{ EnqueueOutcome, LocalObject, MAX_MULTIPART_PARTS, OdmWriteBack, PULL_MAX_RETRIES, PULL_RETRY_BASE_DELAYS, PullCompletion, PullQueue, PullReason, PullSource, QueuedPullOutcome, SourceBody, WriteBackBody, WriteBackError, - WriteBackOutcome, WriteBackPart, WriteBackRequest, commit_inline, commit_inline_with, + WriteBackOutcome, WriteBackPart, WriteBackRequest, commit_inline, commit_inline_with, idle_guarded_body, }; pub mod backfill { pub use crate::bucket::on_demand_migration::backfill::{ diff --git a/crates/ecstore/src/bucket/on_demand_migration/mod.rs b/crates/ecstore/src/bucket/on_demand_migration/mod.rs index e6c53bebb..705a3f51e 100644 --- a/crates/ecstore/src/bucket/on_demand_migration/mod.rs +++ b/crates/ecstore/src/bucket/on_demand_migration/mod.rs @@ -42,7 +42,7 @@ pub use negative_cache::{NEGATIVE_CACHE_MAX_ENTRIES, NegativeCache}; pub use pull::{ EnqueueOutcome, LocalObject, MAX_MULTIPART_PARTS, OdmWriteBack, PULL_MAX_RETRIES, PULL_RETRY_BASE_DELAYS, PullCompletion, PullQueue, PullReason, PullSource, QueuedPullOutcome, SourceBody, WriteBackBody, WriteBackError, WriteBackOutcome, - WriteBackPart, WriteBackRequest, commit_inline, commit_inline_with, + WriteBackPart, WriteBackRequest, commit_inline, commit_inline_with, idle_guarded_body, }; pub use stats::{ GaugeGuard, LastSourceError, LatencyBucketSnapshot, OdmOp, OdmOutcome, OdmStats, OdmStatsSnapshot, PullFailureReason, diff --git a/crates/ecstore/src/bucket/on_demand_migration/pull.rs b/crates/ecstore/src/bucket/on_demand_migration/pull.rs index 05c8ab542..f46b9c2b5 100644 --- a/crates/ecstore/src/bucket/on_demand_migration/pull.rs +++ b/crates/ecstore/src/bucket/on_demand_migration/pull.rs @@ -149,6 +149,30 @@ pub enum EnqueueOutcome { /// Body a source read produces; consumed inside the pump task only. pub type SourceBody = Pin> + Send + 'static>>; +/// Applies `source_timeout.idle_ms` to a source body chunk by chunk: a chunk +/// that does not arrive within `idle_timeout` ends the stream with a timeout +/// error. The background pump enforces the same budget itself; the inline tee +/// path in the app layer has no pump and wraps its body with this instead, so +/// a stalled source cannot hold an inline pull open past the configured +/// budget. +pub fn idle_guarded_body(body: SourceBody, idle_timeout: Duration) -> SourceBody { + Box::pin(futures::stream::unfold(Some(body), move |state| async move { + let mut body = state?; + match tokio::time::timeout(idle_timeout, body.next()).await { + Err(_elapsed) => Some(( + Err(io::Error::new( + io::ErrorKind::TimedOut, + format!("source body stalled for more than {}ms", idle_timeout.as_millis()), + )), + None, + )), + Ok(None) => None, + Ok(Some(Err(err))) => Some((Err(err), None)), + Ok(Some(Ok(chunk))) => Some((Ok(chunk), Some(body))), + } + })) +} + /// Body handed to the write-back; `Sync` because the app-layer put path /// wraps it into an S3 streaming blob. pub type WriteBackBody = Pin> + Send + Sync + 'static>>; @@ -262,7 +286,8 @@ impl WriteBackError { match self { WriteBackError::Integrity => PullFailureReason::EtagMismatch, WriteBackError::Unsupported(_) => PullFailureReason::SourceUnsupported, - WriteBackError::Quota(_) | WriteBackError::Local(_) => PullFailureReason::LocalWrite, + WriteBackError::Quota(_) => PullFailureReason::Quota, + WriteBackError::Local(_) => PullFailureReason::LocalWrite, } } } @@ -1487,6 +1512,28 @@ mod tests { assert_eq!(failures(&state).get("source_connect"), Some(&1)); } + #[tokio::test(start_paused = true)] + async fn idle_guarded_body_ends_a_stalled_stream_and_passes_chunks_through() { + let (tx, rx) = mpsc::channel::>(4); + let body: SourceBody = Box::pin(tokio_stream::wrappers::ReceiverStream::new(rx)); + let mut guarded = idle_guarded_body(body, Duration::from_secs(2)); + + tx.send(Ok(Bytes::from_static(b"chunk"))).await.expect("send a chunk"); + assert_eq!(guarded.next().await.expect("a chunk").expect("chunk is ok"), Bytes::from_static(b"chunk")); + + // The producer never sends again: the guard ends the stream itself. + let started = tokio::time::Instant::now(); + let err = guarded + .next() + .await + .expect("the guard yields the timeout") + .expect_err("a stalled body must time out"); + assert_eq!(err.kind(), io::ErrorKind::TimedOut); + assert!(started.elapsed() >= Duration::from_secs(2)); + assert!(guarded.next().await.is_none(), "the stream ends after the timeout"); + drop(tx); + } + #[tokio::test(start_paused = true)] async fn stalled_source_body_hits_the_idle_timeout() { let sys = OnDemandMigrationSys::new(); @@ -1744,7 +1791,7 @@ mod tests { assert_eq!(PullReason::Backfill.path(), PullPath::Backfill); assert_eq!(PullReason::RangeGet.as_str(), "range_get"); assert_eq!(WriteBackError::Integrity.reason(), PullFailureReason::EtagMismatch); - assert_eq!(WriteBackError::Quota("full".into()).reason(), PullFailureReason::LocalWrite); + assert_eq!(WriteBackError::Quota("full".into()).reason(), PullFailureReason::Quota); assert_eq!(WriteBackError::Local("x".into()).reason(), PullFailureReason::LocalWrite); assert_eq!(WriteBackError::Unsupported("x".into()).reason(), PullFailureReason::SourceUnsupported); for (retry, base) in PULL_RETRY_BASE_DELAYS.iter().enumerate() { diff --git a/crates/ecstore/src/bucket/on_demand_migration/source_client.rs b/crates/ecstore/src/bucket/on_demand_migration/source_client.rs index 4e23f3973..7164ef7f0 100644 --- a/crates/ecstore/src/bucket/on_demand_migration/source_client.rs +++ b/crates/ecstore/src/bucket/on_demand_migration/source_client.rs @@ -30,6 +30,7 @@ use crate::bucket::remote_s3_client::{ }; use crate::storage_api_contracts::range::HTTPRangeSpec; use aws_sdk_s3::Client as S3Client; +use aws_sdk_s3::config::retry::RetryConfig; use aws_sdk_s3::error::{ProvideErrorMetadata, SdkError}; use aws_sdk_s3::operation::get_object::GetObjectOutput; use aws_sdk_s3::operation::head_object::HeadObjectOutput; @@ -579,7 +580,18 @@ impl SourceClient { } fn from_config_builder(config: aws_sdk_s3::config::Builder, endpoint: String, spec: &SourceClientSpec) -> Self { - let client = S3Client::from_conf(config.interceptor(SourceProxyMarkerInterceptor::new()).build()); + // The pull pipeline and the backfill job own the retry budget for a + // source call (`pull.rs` PULL_MAX_RETRIES, `backfill.rs` + // LIST_MAX_RETRIES), and the breaker counts logical calls. Leaving the + // smithy standard policy on top would turn one counted failure into + // three wire requests against a source that is already struggling, so + // keep one logical call equal to one wire request. + let client = S3Client::from_conf( + config + .retry_config(RetryConfig::disabled()) + .interceptor(SourceProxyMarkerInterceptor::new()) + .build(), + ); Self { client, endpoint, @@ -736,7 +748,6 @@ impl SourceClient { #[cfg(test)] mod tests { use super::*; - use aws_sdk_s3::config::retry::RetryConfig; use aws_smithy_runtime_api::client::http::{HttpConnector, HttpConnectorFuture, SharedHttpConnector, http_client_fn}; use aws_smithy_runtime_api::client::orchestrator::HttpRequest; use aws_smithy_runtime_api::client::result::ConnectorError; @@ -867,8 +878,7 @@ mod tests { let config = build_remote_s3_config(&endpoint) .await .expect("test spec should build") - .http_client(http_client) - .retry_config(RetryConfig::disabled()); + .http_client(http_client); (SourceClient::from_config_builder(config, endpoint.endpoint_url(), spec), requests) } diff --git a/crates/ecstore/src/bucket/on_demand_migration/stats.rs b/crates/ecstore/src/bucket/on_demand_migration/stats.rs index d76d1c123..ed3a7de2c 100644 --- a/crates/ecstore/src/bucket/on_demand_migration/stats.rs +++ b/crates/ecstore/src/bucket/on_demand_migration/stats.rs @@ -123,6 +123,8 @@ pub enum PullFailureReason { EtagMismatch, /// The local write (internal PUT) failed. LocalWrite, + /// The bucket quota rejected the write-back. + Quota, /// The bucket state was removed or the process is shutting down. Canceled, /// The background pull queue was full. @@ -130,7 +132,7 @@ pub enum PullFailureReason { } impl PullFailureReason { - pub const ALL: [PullFailureReason; 12] = [ + pub const ALL: [PullFailureReason; 13] = [ PullFailureReason::SourceNotFound, PullFailureReason::SourceAccessDenied, PullFailureReason::SourceThrottled, @@ -141,6 +143,7 @@ impl PullFailureReason { PullFailureReason::SourceOther, PullFailureReason::EtagMismatch, PullFailureReason::LocalWrite, + PullFailureReason::Quota, PullFailureReason::Canceled, PullFailureReason::QueueFull, ]; @@ -157,6 +160,7 @@ impl PullFailureReason { PullFailureReason::SourceOther => "source_other", PullFailureReason::EtagMismatch => "etag_mismatch", PullFailureReason::LocalWrite => "local_write", + PullFailureReason::Quota => "quota", PullFailureReason::Canceled => "canceled", PullFailureReason::QueueFull => "queue_full", } @@ -457,7 +461,7 @@ mod tests { "pulled_bytes_total": 4096, "pulled_objects_total": { "backfill": 0, "background": 0, "inline": 1 }, "pull_failures_total": { - "canceled": 0, "etag_mismatch": 0, "local_write": 0, "queue_full": 0, + "canceled": 0, "etag_mismatch": 0, "local_write": 0, "queue_full": 0, "quota": 0, "source_access_denied": 0, "source_connect": 0, "source_not_found": 0, "source_other": 0, "source_server_error": 0, "source_throttled": 0, "source_timeout": 1, "source_unsupported": 0 }, diff --git a/crates/madmin/fixtures/on_demand_migration/status.json b/crates/madmin/fixtures/on_demand_migration/status.json index 6806d03b9..86acc06de 100644 --- a/crates/madmin/fixtures/on_demand_migration/status.json +++ b/crates/madmin/fixtures/on_demand_migration/status.json @@ -1 +1 @@ -{"configured":true,"enabled":true,"module_enabled":true,"provider":"minio","endpoint_host":"source.example.com","breaker":{"state":"half_open","opened_at":null},"counters":{"requests_total":{"get":{"breaker_open":0,"filtered":0,"negative_cached":0,"source_error":0,"source_hit":2,"source_miss":0,"unsupported":0},"head":{"breaker_open":0,"filtered":0,"negative_cached":1,"source_error":0,"source_hit":0,"source_miss":0,"unsupported":0}},"pulled_bytes_total":4096,"pulled_objects_total":{"backfill":0,"background":0,"inline":1},"pull_failures_total":{"canceled":0,"etag_mismatch":0,"local_write":0,"queue_full":0,"source_access_denied":0,"source_connect":0,"source_not_found":0,"source_other":0,"source_server_error":0,"source_throttled":0,"source_timeout":1,"source_unsupported":0},"source_latency":{"buckets":[{"le_ms":5,"count":1},{"le_ms":10,"count":1},{"le_ms":20,"count":1},{"le_ms":50,"count":1},{"le_ms":100,"count":1},{"le_ms":200,"count":1},{"le_ms":500,"count":1},{"le_ms":1000,"count":2},{"le_ms":2000,"count":2},{"le_ms":5000,"count":2},{"le_ms":10000,"count":2},{"le_ms":20000,"count":2},{"le_ms":30000,"count":2},{"le_ms":60000,"count":2}],"count":3,"sum_ms":90753}},"last_source_error":{"class":"server_error","at":"2026-09-02T10:00:00Z"},"inflight_pulls":1,"queue_depth":1,"served_by_source_ratio":null,"updated_at":"2026-09-02T10:00:00Z"} +{"configured":true,"enabled":true,"module_enabled":true,"provider":"minio","endpoint_host":"source.example.com","breaker":{"state":"half_open","opened_at":null},"counters":{"requests_total":{"get":{"breaker_open":0,"filtered":0,"negative_cached":0,"source_error":0,"source_hit":2,"source_miss":0,"unsupported":0},"head":{"breaker_open":0,"filtered":0,"negative_cached":1,"source_error":0,"source_hit":0,"source_miss":0,"unsupported":0}},"pulled_bytes_total":4096,"pulled_objects_total":{"backfill":0,"background":0,"inline":1},"pull_failures_total":{"canceled":0,"etag_mismatch":0,"local_write":0,"queue_full":0,"quota":0,"source_access_denied":0,"source_connect":0,"source_not_found":0,"source_other":0,"source_server_error":0,"source_throttled":0,"source_timeout":1,"source_unsupported":0},"source_latency":{"buckets":[{"le_ms":5,"count":1},{"le_ms":10,"count":1},{"le_ms":20,"count":1},{"le_ms":50,"count":1},{"le_ms":100,"count":1},{"le_ms":200,"count":1},{"le_ms":500,"count":1},{"le_ms":1000,"count":2},{"le_ms":2000,"count":2},{"le_ms":5000,"count":2},{"le_ms":10000,"count":2},{"le_ms":20000,"count":2},{"le_ms":30000,"count":2},{"le_ms":60000,"count":2}],"count":3,"sum_ms":90753}},"last_source_error":{"class":"server_error","at":"2026-09-02T10:00:00Z"},"inflight_pulls":1,"queue_depth":1,"served_by_source_ratio":null,"updated_at":"2026-09-02T10:00:00Z"} diff --git a/crates/madmin/fixtures/on_demand_migration/status_with_backfill.json b/crates/madmin/fixtures/on_demand_migration/status_with_backfill.json index ec14979bd..6dc1df255 100644 --- a/crates/madmin/fixtures/on_demand_migration/status_with_backfill.json +++ b/crates/madmin/fixtures/on_demand_migration/status_with_backfill.json @@ -1 +1 @@ -{"configured":true,"enabled":true,"module_enabled":true,"provider":"minio","endpoint_host":"source.example.com","breaker":{"state":"half_open","opened_at":null},"counters":{"requests_total":{"get":{"breaker_open":0,"filtered":0,"negative_cached":0,"source_error":0,"source_hit":2,"source_miss":0,"unsupported":0},"head":{"breaker_open":0,"filtered":0,"negative_cached":1,"source_error":0,"source_hit":0,"source_miss":0,"unsupported":0}},"pulled_bytes_total":4096,"pulled_objects_total":{"backfill":0,"background":0,"inline":1},"pull_failures_total":{"canceled":0,"etag_mismatch":0,"local_write":0,"queue_full":0,"source_access_denied":0,"source_connect":0,"source_not_found":0,"source_other":0,"source_server_error":0,"source_throttled":0,"source_timeout":1,"source_unsupported":0},"source_latency":{"buckets":[{"le_ms":5,"count":1},{"le_ms":10,"count":1},{"le_ms":20,"count":1},{"le_ms":50,"count":1},{"le_ms":100,"count":1},{"le_ms":200,"count":1},{"le_ms":500,"count":1},{"le_ms":1000,"count":2},{"le_ms":2000,"count":2},{"le_ms":5000,"count":2},{"le_ms":10000,"count":2},{"le_ms":20000,"count":2},{"le_ms":30000,"count":2},{"le_ms":60000,"count":2}],"count":3,"sum_ms":90753}},"last_source_error":{"class":"server_error","at":"2026-09-02T10:00:00Z"},"inflight_pulls":1,"queue_depth":1,"served_by_source_ratio":null,"updated_at":"2026-09-02T10:00:00Z","backfill":{"job_id":"11111111-1111-4111-8111-111111111111","state":"running","listed":2000,"enqueued":1500,"pulled":1400,"skipped_existing":500,"failed":3,"bytes":73400320,"updated_at":"2026-09-02T10:05:10Z"}} +{"configured":true,"enabled":true,"module_enabled":true,"provider":"minio","endpoint_host":"source.example.com","breaker":{"state":"half_open","opened_at":null},"counters":{"requests_total":{"get":{"breaker_open":0,"filtered":0,"negative_cached":0,"source_error":0,"source_hit":2,"source_miss":0,"unsupported":0},"head":{"breaker_open":0,"filtered":0,"negative_cached":1,"source_error":0,"source_hit":0,"source_miss":0,"unsupported":0}},"pulled_bytes_total":4096,"pulled_objects_total":{"backfill":0,"background":0,"inline":1},"pull_failures_total":{"canceled":0,"etag_mismatch":0,"local_write":0,"queue_full":0,"quota":0,"source_access_denied":0,"source_connect":0,"source_not_found":0,"source_other":0,"source_server_error":0,"source_throttled":0,"source_timeout":1,"source_unsupported":0},"source_latency":{"buckets":[{"le_ms":5,"count":1},{"le_ms":10,"count":1},{"le_ms":20,"count":1},{"le_ms":50,"count":1},{"le_ms":100,"count":1},{"le_ms":200,"count":1},{"le_ms":500,"count":1},{"le_ms":1000,"count":2},{"le_ms":2000,"count":2},{"le_ms":5000,"count":2},{"le_ms":10000,"count":2},{"le_ms":20000,"count":2},{"le_ms":30000,"count":2},{"le_ms":60000,"count":2}],"count":3,"sum_ms":90753}},"last_source_error":{"class":"server_error","at":"2026-09-02T10:00:00Z"},"inflight_pulls":1,"queue_depth":1,"served_by_source_ratio":null,"updated_at":"2026-09-02T10:00:00Z","backfill":{"job_id":"11111111-1111-4111-8111-111111111111","state":"running","listed":2000,"enqueued":1500,"pulled":1400,"skipped_existing":500,"failed":3,"bytes":73400320,"updated_at":"2026-09-02T10:05:10Z"}} diff --git a/crates/obs/src/metrics/schema/on_demand_migration.rs b/crates/obs/src/metrics/schema/on_demand_migration.rs index c8971bc5a..ab8d734e6 100644 --- a/crates/obs/src/metrics/schema/on_demand_migration.rs +++ b/crates/obs/src/metrics/schema/on_demand_migration.rs @@ -56,7 +56,7 @@ pub const REQUEST_OUTCOMES: [&str; 7] = [ /// Fixed `path` label values. pub const PULL_PATHS: [&str; 3] = ["inline", "background", "backfill"]; /// Fixed `reason` label values. -pub const PULL_FAILURE_REASONS: [&str; 12] = [ +pub const PULL_FAILURE_REASONS: [&str; 13] = [ "source_not_found", "source_access_denied", "source_throttled", @@ -67,6 +67,7 @@ pub const PULL_FAILURE_REASONS: [&str; 12] = [ "source_other", "etag_mismatch", "local_write", + "quota", "canceled", "queue_full", ]; diff --git a/docs/operations/on-demand-migration.md b/docs/operations/on-demand-migration.md index ba383b6ab..196d2e810 100644 --- a/docs/operations/on-demand-migration.md +++ b/docs/operations/on-demand-migration.md @@ -115,10 +115,10 @@ The persisted blob is `on-demand-migration.json` in the bucket's metadata. Unkno | `policy.pull_queue_capacity` | integer | `1024` | `1..=65536`; a full queue drops the *background* job, never the client response | | `policy.source_timeout.connect_ms` | integer | `5000` | `100..=600000` | | `policy.source_timeout.first_byte_ms` | integer | `15000` | `100..=600000`; also the window a follower waits for the singleflight leader before streaming through | -| `policy.source_timeout.idle_ms` | integer | `30000` | `100..=600000`; enforced by the background pump only (see Known limitations) | +| `policy.source_timeout.idle_ms` | integer | `30000` | `100..=600000`; enforced per body chunk on both the background pump and the inline tee | | `policy.bandwidth_limit_bytes_per_sec` | integer \| null | `null` | When set, at least `65536` | -Values that are **not** configurable: the breaker opens after 5 consecutive counted failures inside a 30 s window, stays open for 30 s and then admits one probe (`breaker.rs`); the negative cache holds at most 100 000 keys per bucket with LRU eviction (`negative_cache.rs`); a background pull retries a retryable source failure at most 3 times with 1 s / 4 s / 16 s base delays plus up to 25 % jitter (`pull.rs`). +Values that are **not** configurable: the breaker opens after 5 consecutive counted failures inside a 30 s window, stays open for 30 s and then admits one probe (`breaker.rs`); the negative cache holds at most 100 000 keys per bucket with LRU eviction (`negative_cache.rs`); a background pull retries a retryable source failure at most 3 times with 1 s / 4 s / 16 s base delays plus up to 25 % jitter (`pull.rs`). The SDK's own retry policy is disabled on the source client, so one logical source call is exactly one wire request and the retry budget above is the only one. Validation also rejects two shapes outright: a source whose endpoint and bucket name **this** bucket on this deployment (`SelfReference`), and a source that matches one of the bucket's own replication targets (`ReplicationLoop`) — that pairing would amplify a write-back into a loop. @@ -167,7 +167,7 @@ Behaviour a client can observe. The "Test" column names the case that pins it: ` | Repeated source 5xx / timeouts | The breaker opens, GETs answer per `source_error` and HEADs answer 404; a probe closes it again after the open window | `fault_test.rs::test_odm_repeated_source_errors_open_the_breaker_and_recover`, `head.rs::odm_head_source_errors_follow_policy_and_open_the_breaker` | | Bucket default SSE | The pulled object is stored encrypted and reads back in plaintext; the ETag override is dropped, and the source ETag is kept in metadata | `interaction_test.rs::test_odm_pulled_object_uses_bucket_default_encryption`, `on_demand_migration_put.rs::write_back_under_bucket_default_sse_stores_ciphertext_and_records_source_etag` | | Object Lock bucket | The pulled object inherits the bucket's default retention | `interaction_test.rs::test_odm_pulled_object_inherits_object_lock_retention` | -| Bucket quota exceeded | The client is still served from the source; the write-back is rejected, nothing is stored, and the failure counts under `local_write` | `interaction_test.rs::test_odm_write_back_respects_the_bucket_quota`, `on_demand_migration_put.rs::write_back_reports_a_full_bucket_quota` | +| Bucket quota exceeded | The client is still served from the source; the write-back is rejected, nothing is stored, and the failure counts under `quota` | `interaction_test.rs::test_odm_write_back_respects_the_bucket_quota`, `on_demand_migration_put.rs::write_back_reports_a_full_bucket_quota` | | Notifications | A write-back emits `ObjectCreated` with principal `rustfs-on-demand-migration`, unless `emit_events` is `false` | `interaction_test.rs::test_odm_pull_emits_object_created_events_unless_disabled` | | Replication configured on the local bucket | A pulled object replicates like any other write; configuring one of the bucket's replication targets as the source is rejected | `interaction_test.rs::test_odm_pulled_object_replicates_and_target_as_source_is_rejected`, `on_demand_migration_put.rs::write_back_schedules_replication_and_names_the_migration_principal` | | Source is itself a RustFS/MinIO deployment with its own source | The anti-loop marker stops the chain at the first hop; mutual configurations still terminate | `real_source_test.rs::test_odm_chained_sources_stop_at_the_loop_guard_real_single_node` | @@ -247,7 +247,7 @@ All series are bucket-scoped under `rustfs_on_demand_migration_*` and appear onl | `requests_total` | counter | `bucket`, `op` (`get`, `head`), `outcome` (`source_hit`, `source_miss`, `source_error`, `breaker_open`, `negative_cached`, `filtered`, `unsupported`) | | `pulled_bytes_total` | counter | `bucket` | | `pulled_objects_total` | counter | `bucket`, `path` (`inline`, `background`, `backfill`) | -| `pull_failures_total` | counter | `bucket`, `reason` (`source_not_found`, `source_access_denied`, `source_throttled`, `source_timeout`, `source_connect`, `source_server_error`, `source_unsupported`, `source_other`, `etag_mismatch`, `local_write`, `canceled`, `queue_full`) | +| `pull_failures_total` | counter | `bucket`, `reason` (`source_not_found`, `source_access_denied`, `source_throttled`, `source_timeout`, `source_connect`, `source_server_error`, `source_unsupported`, `source_other`, `etag_mismatch`, `local_write`, `quota`, `canceled`, `queue_full`) | | `inflight_pulls` | gauge | `bucket` | | `queue_depth` | gauge | `bucket` | | `source_latency_seconds_distribution` / `_sum` / `_count` | counter | `bucket`, plus `le` on the distribution | @@ -291,7 +291,7 @@ sum by (bucket, reason) (rate(rustfs_on_demand_migration_pull_failures_total[5m] **`NoSuchBucket` from the source, or a 301/307 redirect.** The addressing style is wrong: a virtual-host request against a path-style-only server looks like a missing bucket, and a path-style request against AWS gets redirected. Set `source.path_style` explicitly instead of relying on `auto`. -**Large objects are served but never stored.** Check `pull_failures_total`: `queue_full` means bursts exceed `pull_queue_capacity` (raise it, or raise `max_concurrent_pulls`); `local_write` covers both a genuine local write failure and a bucket quota rejection; `etag_mismatch` means the source body did not match the ETag the source advertised. The client response is served in all three cases — only the local copy is missing. +**Large objects are served but never stored.** Check `pull_failures_total`: `queue_full` means bursts exceed `pull_queue_capacity` (raise it, or raise `max_concurrent_pulls`); `quota` means the bucket quota rejected the write-back and `local_write` a genuine local write failure; `etag_mismatch` means the source body did not match the ETag the source advertised. The client response is served in all three cases — only the local copy is missing. **The first read of a key is slow, later ones are fast.** Expected: the first read pays a source HEAD plus a source GET. Watch `source_latency_seconds_*` for the source's contribution and lower `inline_max_bytes` if teeing large objects is hurting first-byte latency. @@ -306,10 +306,7 @@ sum by (bucket, reason) (rate(rustfs_on_demand_migration_pull_failures_total[5m] - **Azure Blob is not a supported source** (rustfs/backlog#2166). GCS is supported only through its XML interoperability API with HMAC keys. - **LIST does not merge the source** (rustfs/backlog#2164). Only local objects are listed, so a client that lists before reading will not see un-migrated keys. - **Write-through is undecided** (rustfs/backlog#2165). PUT and DELETE never reach the source in this version. -- **The source client inherits the SDK's default retry policy.** The standard smithy strategy retries up to 3 times, so one logical source call can be up to three wire requests; every breaker count therefore sits on top of a threefold request amplification against an already-struggling source. - **`pull_failures_total` counts abandoned pulls, not attempts.** A pull that failed twice and then succeeded contributes nothing; attempt-level failure needs a new counter. -- **A quota rejection is reported as `local_write`.** The failure-reason label set has no `quota` value, so a full bucket is indistinguishable from another local write failure in the metrics; the log line and the bucket's usage tell them apart. -- **`policy.source_timeout.idle_ms` applies to background pulls only.** The inline tee path does not enforce it, so an inline pull that stalls mid-body is bounded only by the SDK read timeout. - **The breaker's 30 s open window is a compile-time constant** with no environment override, which is why breaker-related tests have to wait it out. - **`breaker.opened_at` and `served_by_source_ratio` are always `null`** in the status response (see Observability). diff --git a/rustfs/src/app/object/get.rs b/rustfs/src/app/object/get.rs index 2e658ed8a..490394217 100644 --- a/rustfs/src/app/object/get.rs +++ b/rustfs/src/app/object/get.rs @@ -17,7 +17,7 @@ use super::*; use crate::app::storage_api::object_usecase::bucket::on_demand_migration::{ BucketOdmState, OdmLookup, OdmOp, OdmOutcome, OnDemandMigrationSys, PullError, PullLeader, PullOutcome, PullReason, PullSlot, - RangeGetPolicy, SourceClient, SourceError, SourceGet, SourceHead, commit_inline, + RangeGetPolicy, SourceBody, SourceClient, SourceError, SourceGet, SourceHead, commit_inline, idle_guarded_body, }; use crate::app::storage_api::object_usecase::on_demand_migration::WriteBackBody; use rustfs_rio::{TeeOptions, TeePrimary, tee_reader_with_options}; @@ -4703,7 +4703,17 @@ async fn odm_get_inline( drain_on_primary_drop: true, max_drain_bytes: usize::try_from(policy.inline_max_bytes).unwrap_or(usize::MAX), }; - let (primary, secondary) = tee_reader_with_options(Box::pin(body.into_async_read()), ODM_INLINE_TEE_BUFFER_BYTES, options); + // The inline path has no background pump, so `source_timeout.idle_ms` is + // applied to the teed body here; without it a stalled source would hold + // both the client stream and the write-back open until the SDK read + // timeout fires. + let source_body: SourceBody = Box::pin(tokio_util::io::ReaderStream::with_capacity( + body.into_async_read(), + ODM_SOURCE_BODY_CHUNK_BYTES, + )); + let guarded = idle_guarded_body(source_body, Duration::from_millis(policy.source_timeout.idle_ms)); + let (primary, secondary) = + tee_reader_with_options(Box::pin(tokio_util::io::StreamReader::new(guarded)), ODM_INLINE_TEE_BUFFER_BYTES, options); let output = Box::new(odm_get_output(&head, content_length, content_range, odm_inline_client_body(primary))); let commit_state = Arc::clone(state); let commit_key = key.to_string(); diff --git a/rustfs/src/app/object/on_demand_migration_put.rs b/rustfs/src/app/object/on_demand_migration_put.rs index c05fc5ef1..fc81009a1 100644 --- a/rustfs/src/app/object/on_demand_migration_put.rs +++ b/rustfs/src/app/object/on_demand_migration_put.rs @@ -677,9 +677,7 @@ mod tests { .await .expect_err("a full quota must reject the write-back"); assert!(matches!(err, WriteBackError::Quota(_)), "{err}"); - // ODM-05 fixed the failure label set without a quota label; quota - // failures are accounted as local writes until it grows one. - assert_eq!(err.reason(), PullFailureReason::LocalWrite); + assert_eq!(err.reason(), PullFailureReason::Quota); assert_nothing_left(&store, &bucket, "over.bin").await; } diff --git a/rustfs/src/app/storage_api.rs b/rustfs/src/app/storage_api.rs index 11a6c0f09..be8b0d9af 100644 --- a/rustfs/src/app/storage_api.rs +++ b/rustfs/src/app/storage_api.rs @@ -635,7 +635,8 @@ pub(crate) mod bucket { }; pub(crate) use crate::storage::storage_api::ecstore_bucket::on_demand_migration::{ BucketOdmState, HeadPolicy, OdmLookup, OdmOp, OdmOutcome, OdmStateError, OnDemandMigrationSys, PolicyConfig, - PullError, PullLeader, PullOutcome, PullReason, PullSlot, RangeGetPolicy, SourceErrorPolicy, commit_inline, + PullError, PullLeader, PullOutcome, PullReason, PullSlot, RangeGetPolicy, SourceBody, SourceErrorPolicy, + commit_inline, idle_guarded_body, }; }