mirror of
https://github.com/rustfs/rustfs.git
synced 2026-09-01 17:58:22 +00:00
fix(heal): resume remote rebuilds after target restart (#6941)
* fix(heal): retry unavailable recreate targets * fix(heal): refresh put-file epochs after target restart * test(e2e): harden heal restart evidence Co-Authored-By: heihutu <heihutu@gmail.com> * test(e2e): cancel competing heal before restart Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: houseme <housemecn@gmail.com> Co-authored-by: heihutu <heihutu@gmail.com>
This commit is contained in:
@@ -36,6 +36,7 @@ use rustfs_rio::{ChunkReaderBox, HttpChunkReader, HttpReader, HttpWriter};
|
||||
use sha2::{Digest, Sha256};
|
||||
use std::collections::HashMap;
|
||||
use std::future::Future;
|
||||
use std::io;
|
||||
use std::pin::Pin;
|
||||
use std::sync::{Arc, LazyLock, OnceLock};
|
||||
use std::task::{Context, Poll};
|
||||
@@ -105,9 +106,13 @@ struct PutFileCapabilityCacheState {
|
||||
cached: Option<PutFileCapabilityState>,
|
||||
generation: u64,
|
||||
in_flight: Option<PutFileCapabilityFlight>,
|
||||
rejected_server_epoch: Option<Uuid>,
|
||||
}
|
||||
|
||||
type PutFileCapabilityCacheEntry = Arc<tokio::sync::RwLock<PutFileCapabilityCacheState>>;
|
||||
// The registry lock is released before taking an entry lock. Entry guards cover
|
||||
// only cache transitions, never a probe or await; poll-based writers must be
|
||||
// able to reject an epoch atomically with those transitions.
|
||||
type PutFileCapabilityCacheEntry = Arc<parking_lot::RwLock<PutFileCapabilityCacheState>>;
|
||||
|
||||
static PUT_FILE_CAPABILITY_CACHE: LazyLock<parking_lot::RwLock<HashMap<String, PutFileCapabilityCacheEntry>>> =
|
||||
LazyLock::new(|| parking_lot::RwLock::new(HashMap::new()));
|
||||
@@ -119,7 +124,7 @@ fn put_file_capability_cache_entry(endpoint: &str) -> PutFileCapabilityCacheEntr
|
||||
PUT_FILE_CAPABILITY_CACHE
|
||||
.write()
|
||||
.entry(endpoint.to_owned())
|
||||
.or_insert_with(|| Arc::new(tokio::sync::RwLock::new(PutFileCapabilityCacheState::default())))
|
||||
.or_insert_with(|| Arc::new(parking_lot::RwLock::new(PutFileCapabilityCacheState::default())))
|
||||
.clone()
|
||||
}
|
||||
|
||||
@@ -134,6 +139,23 @@ fn fresh_put_file_capability(state: Option<PutFileCapabilityState>, now: Instant
|
||||
}
|
||||
}
|
||||
|
||||
fn reject_put_file_server_epoch(endpoint: &str, server_epoch: Uuid) {
|
||||
let entry = PUT_FILE_CAPABILITY_CACHE.read().get(endpoint).cloned();
|
||||
if let Some(entry) = entry {
|
||||
let mut state = entry.write();
|
||||
if matches!(state.cached, Some(PutFileCapabilityState::V1 { server_epoch: cached, .. }) if cached == server_epoch) {
|
||||
state.rejected_server_epoch = Some(server_epoch);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn usable_put_file_capability(state: &PutFileCapabilityCacheState, now: Instant) -> Option<Option<Uuid>> {
|
||||
match fresh_put_file_capability(state.cached, now)? {
|
||||
Some(server_epoch) if state.rejected_server_epoch == Some(server_epoch) => None,
|
||||
capability => Some(capability),
|
||||
}
|
||||
}
|
||||
|
||||
fn put_file_capability_status_is_legacy(status: u16) -> bool {
|
||||
status == 404
|
||||
}
|
||||
@@ -322,13 +344,14 @@ impl InternodeDataTransport for TcpHttpInternodeDataTransport {
|
||||
|
||||
async fn open_write(&self, request: WriteStreamRequest) -> Result<FileWriter> {
|
||||
let server_epoch = self.put_file_auth_capability(&request.endpoint).await?;
|
||||
let nonce = server_epoch.map(|_| Uuid::new_v4());
|
||||
let url = build_put_file_stream_url(&request, nonce.zip(server_epoch));
|
||||
let auth_scope = server_epoch.map(|server_epoch| (Uuid::new_v4(), server_epoch));
|
||||
let url = build_put_file_stream_url(&request, auth_scope);
|
||||
let endpoint = request.endpoint;
|
||||
let mut headers = json_headers();
|
||||
build_auth_headers(&url, &Method::PUT, &mut headers)?;
|
||||
let writer = HttpWriter::new(url.clone(), Method::PUT, headers).await?;
|
||||
match nonce {
|
||||
Some(nonce) => Ok(Box::new(PutFileAuthWriter::new(writer, url, nonce))),
|
||||
match auth_scope {
|
||||
Some((nonce, server_epoch)) => Ok(Box::new(PutFileAuthWriter::new(writer, url, nonce, endpoint, server_epoch))),
|
||||
None => Ok(Box::new(writer)),
|
||||
}
|
||||
}
|
||||
@@ -498,15 +521,15 @@ where
|
||||
{
|
||||
let entry = put_file_capability_cache_entry(endpoint);
|
||||
{
|
||||
let state = entry.read().await;
|
||||
if let Some(cached) = fresh_put_file_capability(state.cached, Instant::now()) {
|
||||
let state = entry.read();
|
||||
if let Some(cached) = usable_put_file_capability(&state, Instant::now()) {
|
||||
return Ok(cached);
|
||||
}
|
||||
}
|
||||
|
||||
let flight = {
|
||||
let mut state = entry.write().await;
|
||||
if let Some(cached) = fresh_put_file_capability(state.cached, Instant::now()) {
|
||||
let mut state = entry.write();
|
||||
if let Some(cached) = usable_put_file_capability(&state, Instant::now()) {
|
||||
return Ok(cached);
|
||||
}
|
||||
if let Some(flight) = state.in_flight.clone() {
|
||||
@@ -532,7 +555,7 @@ where
|
||||
.await;
|
||||
|
||||
{
|
||||
let mut state = entry.write().await;
|
||||
let mut state = entry.write();
|
||||
let is_current_flight = state
|
||||
.in_flight
|
||||
.as_ref()
|
||||
@@ -540,6 +563,9 @@ where
|
||||
if is_current_flight {
|
||||
match outcome {
|
||||
Ok(Some(server_epoch)) => {
|
||||
if state.rejected_server_epoch != Some(*server_epoch) {
|
||||
state.rejected_server_epoch = None;
|
||||
}
|
||||
state.cached = Some(PutFileCapabilityState::V1 {
|
||||
server_epoch: *server_epoch,
|
||||
revalidate_after: Instant::now() + PUT_FILE_V1_CAPABILITY_TTL,
|
||||
@@ -630,17 +656,23 @@ struct PutFileAuthWriter<W> {
|
||||
inner: W,
|
||||
url: String,
|
||||
nonce: Uuid,
|
||||
endpoint: String,
|
||||
server_epoch: Uuid,
|
||||
server_epoch_rejected: bool,
|
||||
hasher: Sha256,
|
||||
trailer: Option<Vec<u8>>,
|
||||
trailer_offset: usize,
|
||||
}
|
||||
|
||||
impl<W> PutFileAuthWriter<W> {
|
||||
fn new(inner: W, url: String, nonce: Uuid) -> Self {
|
||||
fn new(inner: W, url: String, nonce: Uuid, endpoint: String, server_epoch: Uuid) -> Self {
|
||||
Self {
|
||||
inner,
|
||||
url,
|
||||
nonce,
|
||||
endpoint,
|
||||
server_epoch,
|
||||
server_epoch_rejected: false,
|
||||
hasher: Sha256::new(),
|
||||
trailer: None,
|
||||
trailer_offset: 0,
|
||||
@@ -656,6 +688,14 @@ impl<W> PutFileAuthWriter<W> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn reject_server_epoch_on_conflict(&mut self, error: &io::Error) {
|
||||
if self.server_epoch_rejected || !io_error_has_put_file_epoch_conflict(error) {
|
||||
return;
|
||||
}
|
||||
reject_put_file_server_epoch(&self.endpoint, self.server_epoch);
|
||||
self.server_epoch_rejected = true;
|
||||
}
|
||||
|
||||
fn poll_write_trailer(&mut self, cx: &mut Context<'_>) -> Poll<std::io::Result<()>>
|
||||
where
|
||||
W: AsyncWrite + Unpin,
|
||||
@@ -673,7 +713,10 @@ impl<W> PutFileAuthWriter<W> {
|
||||
)));
|
||||
}
|
||||
Poll::Ready(Ok(written)) => written,
|
||||
Poll::Ready(Err(err)) => return Poll::Ready(Err(err)),
|
||||
Poll::Ready(Err(err)) => {
|
||||
self.reject_server_epoch_on_conflict(&err);
|
||||
return Poll::Ready(Err(err));
|
||||
}
|
||||
Poll::Pending => return Poll::Pending,
|
||||
};
|
||||
self.trailer_offset += written;
|
||||
@@ -682,6 +725,15 @@ impl<W> PutFileAuthWriter<W> {
|
||||
}
|
||||
}
|
||||
|
||||
fn io_error_has_put_file_epoch_conflict(error: &io::Error) -> bool {
|
||||
error
|
||||
.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<rustfs_rio::InternodeHttpError>())
|
||||
.is_some_and(
|
||||
|error| matches!(error.kind(), rustfs_rio::InternodeHttpErrorKind::HttpStatus(status) if status.as_u16() == 409),
|
||||
)
|
||||
}
|
||||
|
||||
impl<W> AsyncWrite for PutFileAuthWriter<W>
|
||||
where
|
||||
W: AsyncWrite + Unpin,
|
||||
@@ -698,12 +750,22 @@ where
|
||||
self.hasher.update(&buf[..written]);
|
||||
Poll::Ready(Ok(written))
|
||||
}
|
||||
other => other,
|
||||
Poll::Ready(Err(err)) => {
|
||||
self.reject_server_epoch_on_conflict(&err);
|
||||
Poll::Ready(Err(err))
|
||||
}
|
||||
Poll::Pending => Poll::Pending,
|
||||
}
|
||||
}
|
||||
|
||||
fn poll_flush(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<std::io::Result<()>> {
|
||||
Pin::new(&mut self.inner).poll_flush(cx)
|
||||
match Pin::new(&mut self.inner).poll_flush(cx) {
|
||||
Poll::Ready(Err(err)) => {
|
||||
self.reject_server_epoch_on_conflict(&err);
|
||||
Poll::Ready(Err(err))
|
||||
}
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
|
||||
fn poll_shutdown(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<std::io::Result<()>> {
|
||||
@@ -712,7 +774,13 @@ where
|
||||
Poll::Ready(Err(err)) => return Poll::Ready(Err(err)),
|
||||
Poll::Pending => return Poll::Pending,
|
||||
}
|
||||
Pin::new(&mut self.inner).poll_shutdown(cx)
|
||||
match Pin::new(&mut self.inner).poll_shutdown(cx) {
|
||||
Poll::Ready(Err(err)) => {
|
||||
self.reject_server_epoch_on_conflict(&err);
|
||||
Poll::Ready(Err(err))
|
||||
}
|
||||
other => other,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -840,7 +908,6 @@ mod tests {
|
||||
loop {
|
||||
let strong_count = entry
|
||||
.read()
|
||||
.await
|
||||
.in_flight
|
||||
.as_ref()
|
||||
.map(|flight| Arc::strong_count(&flight.outcome))
|
||||
@@ -858,6 +925,50 @@ mod tests {
|
||||
#[derive(Debug)]
|
||||
struct LegacyTestTransport;
|
||||
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
enum PutFileFailurePhase {
|
||||
Write,
|
||||
Flush,
|
||||
Shutdown,
|
||||
}
|
||||
|
||||
struct PutFileFailureWriter {
|
||||
phase: PutFileFailurePhase,
|
||||
status: reqwest::StatusCode,
|
||||
}
|
||||
|
||||
impl PutFileFailureWriter {
|
||||
fn error(&self) -> io::Error {
|
||||
rustfs_rio::new_test_internode_http_io_error(rustfs_rio::InternodeHttpErrorKind::HttpStatus(self.status))
|
||||
}
|
||||
}
|
||||
|
||||
impl tokio::io::AsyncWrite for PutFileFailureWriter {
|
||||
fn poll_write(self: Pin<&mut Self>, _cx: &mut Context<'_>, buf: &[u8]) -> Poll<std::io::Result<usize>> {
|
||||
Poll::Ready(if matches!(self.phase, PutFileFailurePhase::Write) {
|
||||
Err(self.error())
|
||||
} else {
|
||||
Ok(buf.len())
|
||||
})
|
||||
}
|
||||
|
||||
fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<std::io::Result<()>> {
|
||||
Poll::Ready(if matches!(self.phase, PutFileFailurePhase::Flush) {
|
||||
Err(self.error())
|
||||
} else {
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
|
||||
fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<std::io::Result<()>> {
|
||||
Poll::Ready(if matches!(self.phase, PutFileFailurePhase::Shutdown) {
|
||||
Err(self.error())
|
||||
} else {
|
||||
Ok(())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl InternodeDataTransport for LegacyTestTransport {
|
||||
async fn open_read(&self, _request: ReadStreamRequest) -> Result<FileReader> {
|
||||
@@ -1048,7 +1159,7 @@ mod tests {
|
||||
let v1_endpoint = format!("http://v1-{}.invalid", Uuid::new_v4());
|
||||
let v1_entry = put_file_capability_cache_entry(&v1_endpoint);
|
||||
let server_epoch = Uuid::new_v4();
|
||||
v1_entry.write().await.cached = Some(PutFileCapabilityState::V1 {
|
||||
v1_entry.write().cached = Some(PutFileCapabilityState::V1 {
|
||||
server_epoch,
|
||||
revalidate_after: Instant::now() + PUT_FILE_V1_CAPABILITY_TTL,
|
||||
});
|
||||
@@ -1067,7 +1178,7 @@ mod tests {
|
||||
Some(server_epoch)
|
||||
);
|
||||
assert!(!cache_probe_called.load(Ordering::SeqCst));
|
||||
v1_entry.write().await.cached = Some(PutFileCapabilityState::V1 {
|
||||
v1_entry.write().cached = Some(PutFileCapabilityState::V1 {
|
||||
server_epoch,
|
||||
revalidate_after: Instant::now(),
|
||||
});
|
||||
@@ -1086,8 +1197,7 @@ mod tests {
|
||||
|
||||
let legacy_endpoint = format!("http://legacy-{}.invalid", Uuid::new_v4());
|
||||
let legacy_entry = put_file_capability_cache_entry(&legacy_endpoint);
|
||||
legacy_entry.write().await.cached =
|
||||
Some(PutFileCapabilityState::LegacyUntil(Instant::now() + PUT_FILE_LEGACY_CAPABILITY_TTL));
|
||||
legacy_entry.write().cached = Some(PutFileCapabilityState::LegacyUntil(Instant::now() + PUT_FILE_LEGACY_CAPABILITY_TTL));
|
||||
assert!(
|
||||
transport
|
||||
.put_file_auth_capability(&legacy_endpoint)
|
||||
@@ -1098,7 +1208,7 @@ mod tests {
|
||||
|
||||
let expired_endpoint = format!("http://expired-legacy-{}.invalid", Uuid::new_v4());
|
||||
let expired_entry = put_file_capability_cache_entry(&expired_endpoint);
|
||||
expired_entry.write().await.cached = Some(PutFileCapabilityState::LegacyUntil(Instant::now()));
|
||||
expired_entry.write().cached = Some(PutFileCapabilityState::LegacyUntil(Instant::now()));
|
||||
let reprobed = std::sync::atomic::AtomicBool::new(false);
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&expired_endpoint, || async {
|
||||
@@ -1349,7 +1459,7 @@ mod tests {
|
||||
};
|
||||
probe_started.notified().await;
|
||||
{
|
||||
let mut state = entry.write().await;
|
||||
let mut state = entry.write();
|
||||
state.generation = state.generation.checked_add(1).expect("test generation should advance");
|
||||
state.cached = Some(PutFileCapabilityState::V1 {
|
||||
server_epoch: newer_epoch,
|
||||
@@ -1362,10 +1472,7 @@ mod tests {
|
||||
task.await.expect("stale task should finish").expect("stale probe result"),
|
||||
Some(stale_epoch)
|
||||
);
|
||||
assert_eq!(
|
||||
fresh_put_file_capability(entry.read().await.cached, Instant::now()),
|
||||
Some(Some(newer_epoch))
|
||||
);
|
||||
assert_eq!(fresh_put_file_capability(entry.read().cached, Instant::now()), Some(Some(newer_epoch)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1398,6 +1505,8 @@ mod tests {
|
||||
|
||||
let _ = rustfs_credentials::set_global_rpc_secret("put-file-auth-writer-test-secret".to_string());
|
||||
let nonce = Uuid::parse_str("11111111-2222-4333-8444-555555555555").expect("nonce");
|
||||
let server_epoch = Uuid::parse_str("aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee").expect("server epoch");
|
||||
let endpoint = "http://node1:9000".to_string();
|
||||
let url = concat!(
|
||||
"http://node1:9000/rustfs/rpc/put_file_stream?disk=disk-a&volume=bucket&path=object%2Fpart.1",
|
||||
"&append=false&size=11&put_file_auth=digest-trailer-v1&put_file_nonce=11111111-2222-4333-8444-555555555555"
|
||||
@@ -1406,7 +1515,7 @@ mod tests {
|
||||
let mut sink = Vec::new();
|
||||
|
||||
{
|
||||
let mut writer = PutFileAuthWriter::new(&mut sink, url.clone(), nonce);
|
||||
let mut writer = PutFileAuthWriter::new(&mut sink, url.clone(), nonce, endpoint, server_epoch);
|
||||
writer.write_all(b"hello world").await.expect("body write should succeed");
|
||||
writer.shutdown().await.expect("shutdown should append auth trailer");
|
||||
let err = writer
|
||||
@@ -1424,6 +1533,143 @@ mod tests {
|
||||
assert_eq!(verified, expected_digest);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn put_file_auth_writer_reprobes_after_server_epoch_conflict() {
|
||||
use tokio::io::AsyncWriteExt;
|
||||
|
||||
let _ = rustfs_credentials::set_global_rpc_secret("put-file-epoch-conflict-test-secret".to_string());
|
||||
for status in [reqwest::StatusCode::CONFLICT, reqwest::StatusCode::BAD_REQUEST] {
|
||||
for (phase, trailer_write) in [
|
||||
(PutFileFailurePhase::Write, false),
|
||||
(PutFileFailurePhase::Write, true),
|
||||
(PutFileFailurePhase::Flush, false),
|
||||
(PutFileFailurePhase::Shutdown, false),
|
||||
] {
|
||||
let endpoint = format!("http://epoch-conflict-{}.invalid", Uuid::new_v4());
|
||||
let stale_epoch = Uuid::new_v4();
|
||||
let replacement_epoch = Uuid::new_v4();
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(stale_epoch)) })
|
||||
.await
|
||||
.expect("initial capability should resolve");
|
||||
let mut writer = PutFileAuthWriter::new(
|
||||
PutFileFailureWriter { phase, status },
|
||||
format!("{endpoint}{PUT_FILE_AUTH_STREAM_PATH}"),
|
||||
Uuid::new_v4(),
|
||||
endpoint.clone(),
|
||||
stale_epoch,
|
||||
);
|
||||
let error = match (phase, trailer_write) {
|
||||
(PutFileFailurePhase::Write, false) => writer.write_all(b"body").await,
|
||||
(PutFileFailurePhase::Flush, _) => writer.flush().await,
|
||||
_ => writer.shutdown().await,
|
||||
}
|
||||
.expect_err("injected writer error must reach the caller");
|
||||
let conflict = status == reqwest::StatusCode::CONFLICT;
|
||||
assert_eq!(io_error_has_put_file_epoch_conflict(&error), conflict);
|
||||
|
||||
let probe_called = AtomicBool::new(false);
|
||||
let resolved = resolve_put_file_auth_capability(&endpoint, || async {
|
||||
probe_called.store(true, Ordering::SeqCst);
|
||||
Ok(Some(replacement_epoch))
|
||||
})
|
||||
.await
|
||||
.expect("capability should remain usable or be reprobed");
|
||||
assert_eq!(probe_called.load(Ordering::SeqCst), conflict, "phase={phase:?}, trailer={trailer_write}");
|
||||
assert_eq!(resolved, Some(if conflict { replacement_epoch } else { stale_epoch }));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn late_put_file_epoch_rejection_preserves_current_rejection() {
|
||||
let endpoint = format!("http://late-epoch-conflict-{}.invalid", Uuid::new_v4());
|
||||
let old_epoch = Uuid::new_v4();
|
||||
let current_epoch = Uuid::new_v4();
|
||||
let replacement_epoch = Uuid::new_v4();
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(old_epoch)) })
|
||||
.await
|
||||
.expect("initial epoch should be cached"),
|
||||
Some(old_epoch)
|
||||
);
|
||||
reject_put_file_server_epoch(&endpoint, old_epoch);
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(current_epoch)) })
|
||||
.await
|
||||
.expect("first restart should install a new epoch"),
|
||||
Some(current_epoch)
|
||||
);
|
||||
|
||||
reject_put_file_server_epoch(&endpoint, current_epoch);
|
||||
// A writer opened before the first restart can report its 409 after
|
||||
// a newer writer has already rejected the second server incarnation.
|
||||
reject_put_file_server_epoch(&endpoint, old_epoch);
|
||||
let probe_called = AtomicBool::new(false);
|
||||
let resolved = resolve_put_file_auth_capability(&endpoint, || async {
|
||||
probe_called.store(true, Ordering::SeqCst);
|
||||
Ok(Some(replacement_epoch))
|
||||
})
|
||||
.await
|
||||
.expect("late old-epoch rejection must preserve the current rejection");
|
||||
|
||||
assert!(probe_called.load(Ordering::SeqCst), "known-rejected current epoch must be reprobed");
|
||||
assert_eq!(resolved, Some(replacement_epoch));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn put_file_epoch_rejection_is_endpoint_and_epoch_scoped() {
|
||||
let endpoint = format!("http://scoped-epoch-{}.invalid", Uuid::new_v4());
|
||||
let other_endpoint = format!("http://other-epoch-{}.invalid", Uuid::new_v4());
|
||||
let current_epoch = Uuid::new_v4();
|
||||
for endpoint in [&endpoint, &other_endpoint] {
|
||||
resolve_put_file_auth_capability(endpoint, || async { Ok(Some(current_epoch)) })
|
||||
.await
|
||||
.expect("initial epoch should resolve");
|
||||
}
|
||||
reject_put_file_server_epoch(&endpoint, Uuid::new_v4());
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&endpoint, || async { panic!("old writer must not invalidate a new epoch") })
|
||||
.await
|
||||
.expect("new epoch must remain cached"),
|
||||
Some(current_epoch)
|
||||
);
|
||||
reject_put_file_server_epoch(&endpoint, current_epoch);
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&other_endpoint, || async { panic!("another endpoint must stay cached") })
|
||||
.await
|
||||
.expect("other endpoint must remain cached"),
|
||||
Some(current_epoch)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn put_file_rejected_epoch_survives_failed_stale_and_downgrade_probes() {
|
||||
let endpoint = format!("http://rejected-probe-{}.invalid", Uuid::new_v4());
|
||||
let rejected_epoch = Uuid::new_v4();
|
||||
let replacement_epoch = Uuid::new_v4();
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(rejected_epoch)) })
|
||||
.await
|
||||
.expect("initial epoch should resolve");
|
||||
reject_put_file_server_epoch(&endpoint, rejected_epoch);
|
||||
let failure = resolve_put_file_auth_capability(&endpoint, || async { Err(Error::other("injected probe failure")) })
|
||||
.await
|
||||
.expect_err("probe failure must be returned");
|
||||
assert!(failure.to_string().contains("injected probe failure"));
|
||||
let downgrade = resolve_put_file_auth_capability(&endpoint, || async { Ok(None) })
|
||||
.await
|
||||
.expect_err("rejection must not unpin authenticated v1");
|
||||
assert!(downgrade.to_string().contains("downgrade rejected"));
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(rejected_epoch)) })
|
||||
.await
|
||||
.expect("a probe racing a restart can still return the old epoch");
|
||||
assert_eq!(
|
||||
resolve_put_file_auth_capability(&endpoint, || async { Ok(Some(replacement_epoch)) })
|
||||
.await
|
||||
.expect("same-epoch probe must not clear known rejection"),
|
||||
Some(replacement_epoch)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn walk_dir_url_encodes_disk_ref() {
|
||||
let url = build_walk_dir_url(&WalkDirStreamRequest {
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
use rustfs_io_metrics::internode_metrics::INTERNODE_OPERATION_PUT_FILE_STREAM;
|
||||
use rustfs_rio::{InternodeHttpError, InternodeHttpErrorKind};
|
||||
use std::error::Error as StdError;
|
||||
use std::hash::{Hash, Hasher};
|
||||
@@ -229,6 +230,19 @@ fn classify_internode_missing_error(error: &InternodeHttpError) -> Option<DiskEr
|
||||
None
|
||||
}
|
||||
|
||||
fn internode_write_error_is_retryable(error: &InternodeHttpError) -> bool {
|
||||
error.kind().is_retryable()
|
||||
|| (matches!(error.kind(), InternodeHttpErrorKind::HttpStatus(status) if status.as_u16() == 409)
|
||||
&& error.context().operation() == Some(INTERNODE_OPERATION_PUT_FILE_STREAM))
|
||||
}
|
||||
|
||||
fn io_error_contains_retryable_internode_write(error: &io::Error) -> bool {
|
||||
error
|
||||
.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<InternodeHttpError>())
|
||||
.is_some_and(internode_write_error_is_retryable)
|
||||
}
|
||||
|
||||
/// Wrap a terminal shard-read failure without changing its typed
|
||||
/// classification. Timeout-like disk errors retain `TimedOut`; other errors
|
||||
/// retain their inner I/O kind or use `Other` when no more specific kind exists.
|
||||
@@ -336,10 +350,7 @@ impl DiskError {
|
||||
|
||||
pub fn is_retryable_internode_write_failure(&self) -> bool {
|
||||
match self {
|
||||
DiskError::Io(io_error) => io_error
|
||||
.get_ref()
|
||||
.and_then(|source| source.downcast_ref::<InternodeHttpError>())
|
||||
.is_some_and(|err| err.kind().is_retryable()),
|
||||
DiskError::Io(io_error) => io_error_contains_retryable_internode_write(io_error),
|
||||
_ => false,
|
||||
}
|
||||
}
|
||||
@@ -1240,6 +1251,68 @@ mod tests {
|
||||
assert!(!DiskError::FileNotFound.is_internode_http_status(429));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_put_file_server_epoch_conflict_is_retryable_write_failure() {
|
||||
let conflict = DiskError::from(rustfs_rio::new_test_internode_http_io_error(
|
||||
rustfs_rio::InternodeHttpErrorKind::HttpStatus(http::StatusCode::CONFLICT),
|
||||
));
|
||||
let bad_request = DiskError::from(rustfs_rio::new_test_internode_http_io_error(
|
||||
rustfs_rio::InternodeHttpErrorKind::HttpStatus(http::StatusCode::BAD_REQUEST),
|
||||
));
|
||||
|
||||
assert!(conflict.is_retryable_internode_write_failure());
|
||||
assert!(!bad_request.is_retryable_internode_write_failure());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn read_stream_conflict_is_not_a_retryable_put_file_failure() {
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
|
||||
tokio::time::timeout(std::time::Duration::from_secs(5), async {
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("bind isolated HTTP fixture");
|
||||
let address = listener.local_addr().expect("fixture address");
|
||||
let server = tokio::spawn(async move {
|
||||
let (mut stream, _) = listener.accept().await.expect("accept read request");
|
||||
let mut request = [0_u8; 4096];
|
||||
let mut read = 0;
|
||||
loop {
|
||||
let count = stream.read(&mut request[read..]).await.expect("read HTTP request");
|
||||
assert!(count > 0, "request ended before its complete headers");
|
||||
read += count;
|
||||
if request[..read].windows(4).any(|bytes| bytes == b"\r\n\r\n") {
|
||||
break;
|
||||
}
|
||||
assert!(read < request.len(), "fixture request headers exceed their budget");
|
||||
}
|
||||
stream
|
||||
.write_all(b"HTTP/1.1 409 Conflict\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")
|
||||
.await
|
||||
.expect("send typed conflict response");
|
||||
});
|
||||
let error = match rustfs_rio::HttpReader::new(
|
||||
format!("http://{address}/rustfs/rpc/read_file_stream"),
|
||||
http::Method::GET,
|
||||
http::HeaderMap::new(),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(_) => panic!("HTTP 409 must fail the read"),
|
||||
Err(error) => DiskError::from(error),
|
||||
};
|
||||
server.await.expect("fixture task should complete");
|
||||
assert!(error.is_internode_http_status(409));
|
||||
assert!(
|
||||
!error.is_retryable_internode_write_failure(),
|
||||
"read-operation 409 must not trigger put-file retry"
|
||||
);
|
||||
})
|
||||
.await
|
||||
.expect("isolated read-conflict test must finish within its budget");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_internode_missing_errors_preserve_disk_error_types() {
|
||||
let file_missing = DiskError::from(rustfs_rio::new_test_remote_file_not_found_http_io_error());
|
||||
|
||||
@@ -321,6 +321,13 @@ impl<'a> MultiWriter<'a> {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn take_retryable_internode_write_failure(&mut self) -> Option<Error> {
|
||||
self.errs
|
||||
.iter_mut()
|
||||
.find(|error| error.as_ref().is_some_and(Error::is_retryable_internode_write_failure))
|
||||
.and_then(Option::take)
|
||||
}
|
||||
|
||||
/// Effective budget for one shard operation: the smaller of the per-shard
|
||||
/// stall timeout and the time remaining until the object's absolute cap.
|
||||
/// Returns `None` when neither deadline is configured (wait indefinitely).
|
||||
|
||||
@@ -108,6 +108,13 @@ where
|
||||
(shards, errs)
|
||||
}
|
||||
|
||||
fn heal_writer_failure(writers: &mut MultiWriter<'_>, error: io::Error) -> Error {
|
||||
writers
|
||||
.take_retryable_internode_write_failure()
|
||||
.map(|error| Error::RemoteClientUnavailable(error.to_string()))
|
||||
.unwrap_or_else(|| error.into())
|
||||
}
|
||||
|
||||
impl super::Erasure {
|
||||
pub async fn heal<R>(
|
||||
&self,
|
||||
@@ -202,10 +209,14 @@ impl super::Erasure {
|
||||
.map(|s| Bytes::from(s.unwrap_or_default()))
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
writers.write(shards).await?;
|
||||
if let Err(error) = writers.write(shards).await {
|
||||
return Err(heal_writer_failure(&mut writers, error));
|
||||
}
|
||||
}
|
||||
|
||||
writers.shutdown().await?;
|
||||
if let Err(error) = writers.shutdown().await {
|
||||
return Err(heal_writer_failure(&mut writers, error));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
@@ -246,6 +257,35 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
struct InternodeFailureWriter {
|
||||
fail_on_write: bool,
|
||||
status: http::StatusCode,
|
||||
}
|
||||
|
||||
impl InternodeFailureWriter {
|
||||
fn error(&self) -> io::Error {
|
||||
rustfs_rio::new_test_internode_http_io_error(rustfs_rio::InternodeHttpErrorKind::HttpStatus(self.status))
|
||||
}
|
||||
}
|
||||
|
||||
impl AsyncWrite for InternodeFailureWriter {
|
||||
fn poll_write(self: Pin<&mut Self>, _cx: &mut Context<'_>, buf: &[u8]) -> Poll<io::Result<usize>> {
|
||||
Poll::Ready(if self.fail_on_write {
|
||||
Err(self.error())
|
||||
} else {
|
||||
Ok(buf.len())
|
||||
})
|
||||
}
|
||||
|
||||
fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<io::Result<()>> {
|
||||
Poll::Ready(Ok(()))
|
||||
}
|
||||
|
||||
fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<io::Result<()>> {
|
||||
Poll::Ready(Err(self.error()))
|
||||
}
|
||||
}
|
||||
|
||||
struct PendingReader;
|
||||
|
||||
impl AsyncRead for PendingReader {
|
||||
@@ -331,6 +371,94 @@ mod tests {
|
||||
assert!(writers.iter().all(Option::is_some));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn heal_maps_put_file_epoch_conflict_to_retryable_remote_unavailable() {
|
||||
for status in [http::StatusCode::CONFLICT, http::StatusCode::BAD_REQUEST] {
|
||||
for (fail_on_write, data) in [
|
||||
(false, b"".as_slice()),
|
||||
(false, b"payload".as_slice()),
|
||||
(true, b"payload".as_slice()),
|
||||
] {
|
||||
let erasure = Erasure::new(2, 1, 64);
|
||||
let encoded = erasure.encode_data(data).expect("source shards should encode");
|
||||
let readers = encoded
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, shard)| {
|
||||
(index < erasure.data_shards).then(|| {
|
||||
BitrotReader::new(Cursor::new(shard.to_vec()), erasure.shard_size(), HashAlgorithm::None, false)
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let mut writers = (0..erasure.total_shard_count())
|
||||
.map(|index| {
|
||||
(index == erasure.data_shards).then(|| {
|
||||
BitrotWriterWrapper::new(
|
||||
CustomWriter::new_tokio_writer(InternodeFailureWriter { fail_on_write, status }),
|
||||
erasure.shard_size(),
|
||||
HashAlgorithm::None,
|
||||
)
|
||||
})
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let error = erasure
|
||||
.heal(&mut writers, readers, data.len(), &[])
|
||||
.await
|
||||
.expect_err("failed sole target must not satisfy heal write quorum");
|
||||
assert_eq!(
|
||||
matches!(error, Error::RemoteClientUnavailable(_)),
|
||||
status == http::StatusCode::CONFLICT,
|
||||
"status={status}, fail_on_write={fail_on_write}, len={}, error={error:?}",
|
||||
data.len()
|
||||
);
|
||||
assert!(writers.iter().all(Option::is_none), "failed target must not be committed");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn heal_epoch_conflict_does_not_abort_healthy_target() {
|
||||
for fail_on_write in [false, true] {
|
||||
let erasure = Erasure::new(2, 2, 64);
|
||||
let data = b"healthy target must retain exact reconstructed bytes";
|
||||
let encoded = erasure.encode_data(data).expect("source shards should encode");
|
||||
let readers = encoded
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, shard)| {
|
||||
(index < erasure.data_shards)
|
||||
.then(|| BitrotReader::new(Cursor::new(shard.to_vec()), erasure.shard_size(), HashAlgorithm::None, false))
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let mut writers = vec![
|
||||
None,
|
||||
None,
|
||||
Some(BitrotWriterWrapper::new(
|
||||
CustomWriter::new_tokio_writer(InternodeFailureWriter {
|
||||
fail_on_write,
|
||||
status: http::StatusCode::CONFLICT,
|
||||
}),
|
||||
erasure.shard_size(),
|
||||
HashAlgorithm::None,
|
||||
)),
|
||||
Some(inline_writer(erasure.shard_size())),
|
||||
];
|
||||
erasure
|
||||
.heal(&mut writers, readers, data.len(), &[])
|
||||
.await
|
||||
.expect("one healthy target must still satisfy the existing heal quorum");
|
||||
assert!(writers[2].is_none(), "conflicting target must be dropped");
|
||||
assert_eq!(
|
||||
writers[3]
|
||||
.take()
|
||||
.expect("healthy target remains")
|
||||
.into_inline_data()
|
||||
.expect("inline target data"),
|
||||
encoded[3].to_vec()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn heal_reconstructs_missing_parity_shard() {
|
||||
let erasure = Erasure::new(2, 2, 64);
|
||||
|
||||
@@ -36,6 +36,20 @@ const EVENT_HEAL_OBJECT_RENAME: &str = "heal_object_rename";
|
||||
const HEAL_RENAME_INCOMPLETE: &str = "heal rename incomplete";
|
||||
const READ_REPAIR_DATA_PHASE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(60 * 60);
|
||||
|
||||
fn heal_drive_state_for_error(error: &DiskError) -> DriveState {
|
||||
match error {
|
||||
DiskError::DiskNotFound | DiskError::RemoteClientUnavailable(_) => DriveState::Offline,
|
||||
DiskError::FaultyDisk | DiskError::FaultyRemoteDisk => DriveState::Faulty,
|
||||
DiskError::FileNotFound
|
||||
| DiskError::FileVersionNotFound
|
||||
| DiskError::VolumeNotFound
|
||||
| DiskError::PartMissingOrCorrupt
|
||||
| DiskError::OutdatedXLMeta => DriveState::Missing,
|
||||
DiskError::FileCorrupt => DriveState::Corrupt,
|
||||
_ => DriveState::Unknown(error.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
static HEAL_RENAME_FAILURES: std::sync::Mutex<Vec<(String, String, usize)>> = std::sync::Mutex::new(Vec::new());
|
||||
|
||||
@@ -892,16 +906,7 @@ impl SetDisks {
|
||||
}
|
||||
|
||||
let drive_state = match reason {
|
||||
Some(err) => match err {
|
||||
DiskError::DiskNotFound => DriveState::Offline.to_string(),
|
||||
DiskError::FileNotFound
|
||||
| DiskError::FileVersionNotFound
|
||||
| DiskError::VolumeNotFound
|
||||
| DiskError::PartMissingOrCorrupt
|
||||
| DiskError::OutdatedXLMeta => DriveState::Missing.to_string(),
|
||||
DiskError::FileCorrupt => DriveState::Corrupt.to_string(),
|
||||
_ => DriveState::Unknown(err.to_string()).to_string(),
|
||||
},
|
||||
Some(err) => heal_drive_state_for_error(&err).to_string(),
|
||||
None => DriveState::Ok.to_string(),
|
||||
};
|
||||
result.before.drives.push(HealDriveInfo {
|
||||
@@ -2673,6 +2678,17 @@ mod heal_result_report_tests {
|
||||
assert!(!super::metadata_less_part_file("xl.meta"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unavailable_heal_errors_use_stable_drive_states() {
|
||||
for error in [DiskError::FaultyDisk, DiskError::FaultyRemoteDisk] {
|
||||
assert_eq!(super::heal_drive_state_for_error(&error).to_string(), DriveState::Faulty.to_string());
|
||||
}
|
||||
assert_eq!(
|
||||
super::heal_drive_state_for_error(&DiskError::RemoteClientUnavailable("peer restarting".to_string())).to_string(),
|
||||
DriveState::Offline.to_string()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_repair_commit_fingerprint_tracks_commit_identity_only() {
|
||||
let data_dir = Uuid::parse_str("11111111-1111-1111-1111-111111111111").expect("data dir should parse");
|
||||
|
||||
Reference in New Issue
Block a user