mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-25 21:46:50 +00:00
fix(get-object): harden GET fast path against mid-stream regressions (#2472)
This commit is contained in:
@@ -17,6 +17,24 @@
|
||||
//! This module defines environment variables and default values for zero-copy
|
||||
//! read operations, which use memory mapping (mmap) to avoid data copying.
|
||||
|
||||
// =============================================================================
|
||||
// GET Fast Path Configuration
|
||||
// =============================================================================
|
||||
|
||||
/// Environment variable for the GetObject chunk fast path master switch.
|
||||
///
|
||||
/// When disabled, `GetObject` bypasses the chunk-streaming fast path entirely and
|
||||
/// always uses the legacy reader path. This provides an operational stopgap for
|
||||
/// regressions in the streaming data plane while keeping zero-copy internals
|
||||
/// configurable independently for future opt-in validation.
|
||||
pub const ENV_OBJECT_GET_CHUNK_FAST_PATH_ENABLE: &str = "RUSTFS_OBJECT_GET_CHUNK_FAST_PATH_ENABLE";
|
||||
|
||||
/// Default: GetObject chunk fast path is disabled.
|
||||
///
|
||||
/// The legacy reader path remains the safe default until the chunk-streaming
|
||||
/// path has sufficient regression coverage for full-body delivery semantics.
|
||||
pub const DEFAULT_OBJECT_GET_CHUNK_FAST_PATH_ENABLE: bool = false;
|
||||
|
||||
// =============================================================================
|
||||
// Zero-Copy Configuration
|
||||
// =============================================================================
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::common::{RustFSTestEnvironment, init_logging, rustfs_binary_path};
|
||||
use crate::common::{RustFSTestEnvironment, init_logging, local_http_client, rustfs_binary_path};
|
||||
use aws_sdk_s3::primitives::ByteStream;
|
||||
use aws_sdk_s3::types::{CompletedMultipartUpload, CompletedPart};
|
||||
use http::header::{CONTENT_TYPE, HOST};
|
||||
@@ -28,6 +28,8 @@ mod tests {
|
||||
use std::io::{Cursor, Write};
|
||||
use std::process::Command;
|
||||
use time::OffsetDateTime;
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
use tokio::net::TcpListener;
|
||||
use zip::{CompressionMethod, ZipWriter, write::SimpleFileOptions};
|
||||
|
||||
const ARCHIVE_TEST_BUCKET: &str = "archive-download-integrity";
|
||||
@@ -116,6 +118,75 @@ mod tests {
|
||||
.await?)
|
||||
}
|
||||
|
||||
fn find_header_terminator(buf: &[u8]) -> Option<usize> {
|
||||
buf.windows(4).position(|window| window == b"\r\n\r\n")
|
||||
}
|
||||
|
||||
async fn read_proxy_request(stream: &mut tokio::net::TcpStream) -> Result<(), Box<dyn Error + Send + Sync>> {
|
||||
let mut buffer = Vec::new();
|
||||
let mut chunk = [0_u8; 4096];
|
||||
|
||||
loop {
|
||||
let read = stream.read(&mut chunk).await?;
|
||||
if read == 0 {
|
||||
return Err("proxy request ended before headers were fully received".into());
|
||||
}
|
||||
buffer.extend_from_slice(&chunk[..read]);
|
||||
if find_header_terminator(&buffer).is_some() {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn spawn_reverse_proxy_to_presigned_url(
|
||||
target_url: String,
|
||||
) -> Result<(String, tokio::task::JoinHandle<Result<(), Box<dyn Error + Send + Sync>>>), Box<dyn Error + Send + Sync>> {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let address = listener.local_addr()?;
|
||||
let proxy_url = format!("http://{address}/");
|
||||
|
||||
let handle = tokio::spawn(async move {
|
||||
let (mut downstream, _) = listener.accept().await?;
|
||||
read_proxy_request(&mut downstream).await?;
|
||||
|
||||
let upstream_response: Result<reqwest::Response, reqwest::Error> = local_http_client().get(&target_url).send().await;
|
||||
let (status, body, content_type) = match upstream_response {
|
||||
Ok(response) => {
|
||||
let status = response.status();
|
||||
let content_type = response
|
||||
.headers()
|
||||
.get("content-type")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.map(str::to_string);
|
||||
match response.bytes().await {
|
||||
Ok(body) => (status, body.to_vec(), content_type),
|
||||
Err(err) => {
|
||||
let body = format!("upstream body read failed: {err}").into_bytes();
|
||||
(StatusCode::BAD_GATEWAY, body, Some("text/plain".to_string()))
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
let body = format!("upstream request failed: {err}").into_bytes();
|
||||
(StatusCode::BAD_GATEWAY, body, Some("text/plain".to_string()))
|
||||
}
|
||||
};
|
||||
|
||||
let mut response_head = format!("HTTP/1.1 {}\r\ncontent-length: {}\r\nconnection: close\r\n", status, body.len());
|
||||
if let Some(content_type) = content_type {
|
||||
response_head.push_str(&format!("content-type: {content_type}\r\n"));
|
||||
}
|
||||
response_head.push_str("\r\n");
|
||||
|
||||
downstream.write_all(response_head.as_bytes()).await?;
|
||||
downstream.write_all(&body).await?;
|
||||
downstream.shutdown().await?;
|
||||
Ok(())
|
||||
});
|
||||
|
||||
Ok((proxy_url, handle))
|
||||
}
|
||||
|
||||
async fn signed_put_request_with_headers(
|
||||
url: &str,
|
||||
access_key: &str,
|
||||
@@ -326,4 +397,130 @@ mod tests {
|
||||
env.stop_server();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[serial]
|
||||
async fn test_presigned_get_and_reverse_proxy_preserve_multipart_bytes_with_fast_path()
|
||||
-> Result<(), Box<dyn Error + Send + Sync>> {
|
||||
init_logging();
|
||||
let mut env = RustFSTestEnvironment::new().await?;
|
||||
start_rustfs_server_with_env(&mut env, &[("RUSTFS_OBJECT_GET_CHUNK_FAST_PATH_ENABLE", "true")]).await?;
|
||||
env.create_test_bucket(MULTIPART_ARCHIVE_TEST_BUCKET).await?;
|
||||
|
||||
let client = env.create_s3_client();
|
||||
let payload = random_bytes(MULTIPART_PART_SIZE + 768 * 1024);
|
||||
let zip_bytes = build_zip_bytes(&[("payload.bin", payload.as_slice())])?;
|
||||
assert!(zip_bytes.len() > MULTIPART_PART_SIZE, "zip payload must exceed multipart threshold");
|
||||
|
||||
let create_output = client
|
||||
.create_multipart_upload()
|
||||
.bucket(MULTIPART_ARCHIVE_TEST_BUCKET)
|
||||
.key("presigned-multipart-bundle.zip")
|
||||
.content_type("application/zip")
|
||||
.send()
|
||||
.await?;
|
||||
let upload_id = create_output.upload_id().expect("multipart upload id");
|
||||
|
||||
let first_part = zip_bytes[..MULTIPART_PART_SIZE].to_vec();
|
||||
let second_part = zip_bytes[MULTIPART_PART_SIZE..].to_vec();
|
||||
|
||||
let upload_part_1 = client
|
||||
.upload_part()
|
||||
.bucket(MULTIPART_ARCHIVE_TEST_BUCKET)
|
||||
.key("presigned-multipart-bundle.zip")
|
||||
.upload_id(upload_id)
|
||||
.part_number(1)
|
||||
.body(ByteStream::from(first_part))
|
||||
.send()
|
||||
.await?;
|
||||
|
||||
let upload_part_2 = client
|
||||
.upload_part()
|
||||
.bucket(MULTIPART_ARCHIVE_TEST_BUCKET)
|
||||
.key("presigned-multipart-bundle.zip")
|
||||
.upload_id(upload_id)
|
||||
.part_number(2)
|
||||
.body(ByteStream::from(second_part))
|
||||
.send()
|
||||
.await?;
|
||||
|
||||
let completed_upload = CompletedMultipartUpload::builder()
|
||||
.parts(
|
||||
CompletedPart::builder()
|
||||
.part_number(1)
|
||||
.e_tag(upload_part_1.e_tag().unwrap_or_default())
|
||||
.build(),
|
||||
)
|
||||
.parts(
|
||||
CompletedPart::builder()
|
||||
.part_number(2)
|
||||
.e_tag(upload_part_2.e_tag().unwrap_or_default())
|
||||
.build(),
|
||||
)
|
||||
.build();
|
||||
|
||||
client
|
||||
.complete_multipart_upload()
|
||||
.bucket(MULTIPART_ARCHIVE_TEST_BUCKET)
|
||||
.key("presigned-multipart-bundle.zip")
|
||||
.upload_id(upload_id)
|
||||
.multipart_upload(completed_upload)
|
||||
.send()
|
||||
.await?;
|
||||
|
||||
let object_url = format!("{}/{}/{}", env.url, MULTIPART_ARCHIVE_TEST_BUCKET, "presigned-multipart-bundle.zip");
|
||||
let direct_response =
|
||||
presigned_get_request_with_accept_encoding(&object_url, &env.access_key, &env.secret_key, "identity").await?;
|
||||
assert_eq!(direct_response.status(), StatusCode::OK);
|
||||
assert_eq!(
|
||||
direct_response
|
||||
.headers()
|
||||
.get("content-length")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.and_then(|value| value.parse::<usize>().ok()),
|
||||
Some(zip_bytes.len())
|
||||
);
|
||||
let direct_body = direct_response.bytes().await?;
|
||||
assert_eq!(direct_body.len(), zip_bytes.len());
|
||||
assert_eq!(direct_body.as_ref(), zip_bytes.as_slice());
|
||||
|
||||
let signed = pre_sign_v4(
|
||||
http::Request::builder()
|
||||
.method(http::Method::GET)
|
||||
.uri(object_url.parse::<http::Uri>()?)
|
||||
.header(
|
||||
HOST,
|
||||
object_url
|
||||
.parse::<http::Uri>()?
|
||||
.authority()
|
||||
.ok_or("request URL missing authority")?
|
||||
.to_string(),
|
||||
)
|
||||
.body(Body::empty())?,
|
||||
&env.access_key,
|
||||
&env.secret_key,
|
||||
"",
|
||||
"us-east-1",
|
||||
600,
|
||||
OffsetDateTime::now_utc(),
|
||||
);
|
||||
let (proxy_url, proxy_handle) = spawn_reverse_proxy_to_presigned_url(signed.uri().to_string()).await?;
|
||||
let proxied_response: reqwest::Response = local_http_client().get(&proxy_url).send().await?;
|
||||
assert_eq!(proxied_response.status(), StatusCode::OK);
|
||||
assert_eq!(
|
||||
proxied_response
|
||||
.headers()
|
||||
.get("content-length")
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.and_then(|value| value.parse::<usize>().ok()),
|
||||
Some(zip_bytes.len())
|
||||
);
|
||||
let proxied_body: bytes::Bytes = proxied_response.bytes().await?;
|
||||
assert_eq!(proxied_body.len(), zip_bytes.len());
|
||||
assert_eq!(proxied_body.as_ref(), zip_bytes.as_slice());
|
||||
|
||||
proxy_handle.await??;
|
||||
env.stop_server();
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -193,6 +193,8 @@ impl IoStage {
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum FallbackReason {
|
||||
Unknown,
|
||||
FeatureDisabled,
|
||||
ProbeFailed,
|
||||
MmapDisabled,
|
||||
MmapUnavailable,
|
||||
SmallObject,
|
||||
@@ -213,6 +215,8 @@ impl FallbackReason {
|
||||
pub const fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Unknown => "unknown",
|
||||
Self::FeatureDisabled => "feature_disabled",
|
||||
Self::ProbeFailed => "probe_failed",
|
||||
Self::MmapDisabled => "mmap_disabled",
|
||||
Self::MmapUnavailable => "mmap_unavailable",
|
||||
Self::SmallObject => "small_object",
|
||||
@@ -347,6 +351,59 @@ pub fn record_io_fallback(stage: IoStage, reason: FallbackReason) {
|
||||
.increment(1);
|
||||
}
|
||||
|
||||
/// Record a selected GET chunk fast path.
|
||||
#[inline(always)]
|
||||
pub fn record_get_object_fast_path_selected(path: &'static str, copy_mode: CopyMode, promised_bytes: i64) {
|
||||
counter!(
|
||||
metric_names::data_plane::GET_FAST_PATH_SELECTED_TOTAL,
|
||||
"path" => path.to_string(),
|
||||
"copy_mode" => copy_mode.as_str().to_string()
|
||||
)
|
||||
.increment(1);
|
||||
|
||||
if promised_bytes >= 0 {
|
||||
histogram!(metric_names::data_plane::GET_FAST_PATH_PROMISED_BYTES).record(promised_bytes as f64);
|
||||
}
|
||||
}
|
||||
|
||||
/// Record a failed GET chunk fast path probe before the response is committed.
|
||||
#[inline(always)]
|
||||
pub fn record_get_object_fast_path_probe_failed(path: &'static str, copy_mode: CopyMode, promised_bytes: i64) {
|
||||
counter!(
|
||||
metric_names::data_plane::GET_FAST_PATH_PROBE_FAILED_TOTAL,
|
||||
"path" => path.to_string(),
|
||||
"copy_mode" => copy_mode.as_str().to_string()
|
||||
)
|
||||
.increment(1);
|
||||
|
||||
if promised_bytes >= 0 {
|
||||
histogram!(metric_names::data_plane::GET_FAST_PATH_PROMISED_BYTES).record(promised_bytes as f64);
|
||||
}
|
||||
}
|
||||
|
||||
/// Record a GET chunk fast path mid-stream error after headers have already been committed.
|
||||
#[inline(always)]
|
||||
pub fn record_get_object_fast_path_midstream_error(
|
||||
path: &'static str,
|
||||
copy_mode: CopyMode,
|
||||
error_kind: &'static str,
|
||||
sent_bytes: usize,
|
||||
promised_bytes: i64,
|
||||
) {
|
||||
counter!(
|
||||
metric_names::data_plane::GET_FAST_PATH_MIDSTREAM_ERROR_TOTAL,
|
||||
"path" => path.to_string(),
|
||||
"copy_mode" => copy_mode.as_str().to_string(),
|
||||
"error_kind" => error_kind.to_string()
|
||||
)
|
||||
.increment(1);
|
||||
|
||||
histogram!(metric_names::data_plane::GET_FAST_PATH_MIDSTREAM_SENT_BYTES).record(sent_bytes as f64);
|
||||
if promised_bytes >= 0 {
|
||||
histogram!(metric_names::data_plane::GET_FAST_PATH_PROMISED_BYTES).record(promised_bytes as f64);
|
||||
}
|
||||
}
|
||||
|
||||
/// Record the currently active mmap bytes held by LocalDisk chunk streams.
|
||||
#[inline(always)]
|
||||
pub fn record_local_disk_active_mmap_bytes(active_bytes: usize) {
|
||||
@@ -875,6 +932,8 @@ mod tests {
|
||||
#[test]
|
||||
fn test_fallback_reason_as_str_values_stable() {
|
||||
assert_eq!(FallbackReason::Unknown.as_str(), "unknown");
|
||||
assert_eq!(FallbackReason::FeatureDisabled.as_str(), "feature_disabled");
|
||||
assert_eq!(FallbackReason::ProbeFailed.as_str(), "probe_failed");
|
||||
assert_eq!(FallbackReason::MmapDisabled.as_str(), "mmap_disabled");
|
||||
assert_eq!(FallbackReason::MmapUnavailable.as_str(), "mmap_unavailable");
|
||||
assert_eq!(FallbackReason::SmallObject.as_str(), "small_object");
|
||||
@@ -928,6 +987,13 @@ mod tests {
|
||||
record_local_disk_compat_collect(3, 16384);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_record_get_object_fast_path_metrics() {
|
||||
record_get_object_fast_path_selected("direct", CopyMode::TrueZeroCopy, 8192);
|
||||
record_get_object_fast_path_probe_failed("bridge", CopyMode::SingleCopy, 4096);
|
||||
record_get_object_fast_path_midstream_error("direct", CopyMode::Reconstructed, "unexpected_eof", 2048, 8192);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_record_put_object_attempted_fast_path() {
|
||||
record_put_object_attempted_fast_path(1024 * 1024);
|
||||
|
||||
@@ -54,4 +54,19 @@ pub mod data_plane {
|
||||
|
||||
/// Size distribution for transformed PUT selections.
|
||||
pub const PUT_TRANSFORM_SIZE_BYTES: &str = "rustfs.io.put.transform.size.bytes";
|
||||
|
||||
/// Total number of selected GET chunk fast paths.
|
||||
pub const GET_FAST_PATH_SELECTED_TOTAL: &str = "rustfs.io.get.fast_path.selected_total";
|
||||
|
||||
/// Total number of GET chunk fast path probe failures before response commit.
|
||||
pub const GET_FAST_PATH_PROBE_FAILED_TOTAL: &str = "rustfs.io.get.fast_path.probe_failed_total";
|
||||
|
||||
/// Total number of GET chunk fast path mid-stream errors after response commit.
|
||||
pub const GET_FAST_PATH_MIDSTREAM_ERROR_TOTAL: &str = "rustfs.io.get.fast_path.midstream_error_total";
|
||||
|
||||
/// Byte distribution promised by GET chunk fast path selections or failures.
|
||||
pub const GET_FAST_PATH_PROMISED_BYTES: &str = "rustfs.io.get.fast_path.promised.bytes";
|
||||
|
||||
/// Byte distribution already sent when a GET chunk fast path fails mid-stream.
|
||||
pub const GET_FAST_PATH_MIDSTREAM_SENT_BYTES: &str = "rustfs.io.get.fast_path.midstream_sent.bytes";
|
||||
}
|
||||
|
||||
@@ -143,6 +143,14 @@ pub fn chunk_body_data_plane_labels(
|
||||
)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub const fn get_object_chunk_path_label(path: GetObjectChunkPath) -> &'static str {
|
||||
match path {
|
||||
GetObjectChunkPath::Direct => "direct",
|
||||
GetObjectChunkPath::Bridge => "bridge",
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_object_chunk_fast_path_guard(
|
||||
has_sse_customer_key: bool,
|
||||
has_sse_customer_key_md5: bool,
|
||||
|
||||
Reference in New Issue
Block a user