fix(ecstore): handle stalled recovery reads and listings (#3790)

* fix(ecstore): handle stalled recovery reads and listings

* fix(rio): start HTTP stall timeout on read

* fix(ecstore): handle stalled reads and partial lists

* fix(ecstore): retire stalled shards and list errors

* fix(ecstore): preserve list merge lookahead entries

* fix(ecstore): bound zero-copy shard reads

* fix(ecstore): hedge stalled shard reads

* fix(ecstore): retire abandoned shard reads

* fix(ecstore): include part identity in metadata quorum

* fix(ecstore): validate heal shard sources

* fix(ecstore): verify reconstructed read shards

* chore(ecstore): log slow object read stages

* fix(heal): throttle auto heal during recovery

* fix(scanner): yield to foreground reads

* fix(scanner): track streaming object reads

* fix(ecstore): avoid false read heal fanout

* fix(ecstore): verify codec streaming reconstruction sources

* fix(ecstore): preserve quorum progress on slow shards

* fix(storage): restore read timeout facade

* fix(ecstore): retain fallback readers after quorum

* chore: allow decode helper argument lists

---------

Co-authored-by: overtrue <anzhengchao@gmail.com>

crates/ecstore/src/erasure/coding/decode_reader.rs

@@ -479,9 +479,12 @@ mod tests {
     use crate::erasure::codec::bridge::{
         CodecStreamingDecodeEngine, ErasureDecodeEngine, LegacyEcDecodeEngine, RustfsCodecDecodeEngine,
     };
-    use crate::erasure::coding::Erasure;
+    use crate::erasure::coding::decode::ParallelReader;
+    use crate::erasure::coding::{BitrotReader, Erasure};
     use crate::set_disk::shard_source::{ShardSlot, StripeReadState};
+    use rustfs_utils::HashAlgorithm;
     use std::collections::VecDeque;
+    use std::io::Cursor;
     use std::sync::Arc;
     use std::sync::atomic::{AtomicUsize, Ordering};
     use tokio::io::AsyncReadExt;
@@ -611,6 +614,46 @@ mod tests {
         assert_eq!(decoded, data);
     }
 
+    #[tokio::test]
+    async fn erasure_decode_reader_rejects_inconsistent_reconstruction_sources() {
+        const DATA_SHARDS: usize = 2;
+        const PARITY_SHARDS: usize = 2;
+        const BLOCK_SIZE: usize = 64;
+
+        let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE);
+        let data = (0u8..64u8).collect::<Vec<_>>();
+        let encoded = erasure.encode_data(&data).expect("test stripe should encode");
+        let mut corrupt_parity = encoded[DATA_SHARDS].to_vec();
+        corrupt_parity[0] ^= 0x80;
+
+        let source = VecStripeSource {
+            stripes: VecDeque::from([StripeReadState::from_parts(
+                vec![
+                    None,
+                    Some(encoded[1].to_vec()),
+                    Some(corrupt_parity),
+                    Some(encoded[DATA_SHARDS + 1].to_vec()),
+                ],
+                Vec::new(),
+                DATA_SHARDS,
+            )]),
+            read_quorum: DATA_SHARDS,
+            read_count: None,
+        };
+        let engine = LegacyEcDecodeEngine::new(erasure);
+        let mut reader = ErasureDecodeReader::new(source, engine, data.len()).expect("reader should be constructed");
+        let mut decoded = Vec::new();
+
+        let err = reader
+            .read_to_end(&mut decoded)
+            .await
+            .expect_err("streaming reader must reject inconsistent reconstruction sources");
+
+        assert_eq!(err.kind(), ErrorKind::InvalidData);
+        assert!(err.to_string().contains("inconsistent read source shards"));
+        assert!(decoded.is_empty());
+    }
+
     #[tokio::test]
     async fn erasure_decode_reader_rustfs_engine_matches_legacy_with_missing_data() {
         let erasure = Erasure::new(4, 2, 32);
@@ -641,6 +684,55 @@ mod tests {
         assert!(decoded.is_empty());
     }
 
+    #[tokio::test]
+    async fn erasure_decode_reader_verifying_parallel_source_rejects_inconsistent_reconstruction_sources() {
+        const DATA_SHARDS: usize = 2;
+        const PARITY_SHARDS: usize = 2;
+        const BLOCK_SIZE: usize = 64;
+
+        let erasure = Erasure::new(DATA_SHARDS, PARITY_SHARDS, BLOCK_SIZE);
+        let data = (0u8..64u8).collect::<Vec<_>>();
+        let shard_size = erasure.shard_size();
+        let encoded = erasure.encode_data(&data).expect("test stripe should encode");
+        let mut corrupt_parity = encoded[DATA_SHARDS].to_vec();
+        corrupt_parity[0] ^= 0x80;
+        let readers = vec![
+            None,
+            Some(BitrotReader::new(
+                Cursor::new(encoded[1].to_vec()),
+                shard_size,
+                HashAlgorithm::None,
+                false,
+            )),
+            Some(BitrotReader::new(Cursor::new(corrupt_parity), shard_size, HashAlgorithm::None, false)),
+            Some(BitrotReader::new(
+                Cursor::new(encoded[DATA_SHARDS + 1].to_vec()),
+                shard_size,
+                HashAlgorithm::None,
+                false,
+            )),
+        ];
+        let source = ParallelReader::new_with_metrics_path_and_reconstruction_verification(
+            readers,
+            erasure.clone(),
+            0,
+            data.len(),
+            Some(GET_OBJECT_PATH_CODEC_STREAMING),
+        );
+        let engine = LegacyEcDecodeEngine::new(erasure);
+        let mut reader = ErasureDecodeReader::new(source, engine, data.len()).expect("reader should be constructed");
+        let mut decoded = Vec::new();
+
+        let err = reader
+            .read_to_end(&mut decoded)
+            .await
+            .expect_err("streaming reader must reject inconsistent reconstruction sources");
+
+        assert_eq!(err.kind(), ErrorKind::InvalidData);
+        assert!(err.to_string().contains("inconsistent read source shards"));
+        assert!(decoded.is_empty());
+    }
+
     #[tokio::test]
     async fn erasure_decode_reader_codec_streaming_engine_enum_matches_legacy() {
         let erasure = Erasure::new(4, 2, 32);

crates/ecstore/src/erasure/coding/erasure.rs

@@ -182,6 +182,34 @@ impl LegacyReedSolomonEncoder {
         self.encode_parity(shards)
     }
 
+    fn verify(&self, shards: &[&[u8]]) -> io::Result<bool> {
+        let expected_shards = self.data_shards + self.parity_shards;
+        if shards.len() != expected_shards {
+            return Err(io::Error::other(format!(
+                "invalid shard count: got {}, expected {}",
+                shards.len(),
+                expected_shards
+            )));
+        }
+        if shards.iter().all(|shard| shard.is_empty()) {
+            return Ok(true);
+        }
+
+        let mut expected = shards.iter().map(|shard| Some(shard.to_vec())).collect::<Vec<_>>();
+        self.encode_parity(&mut expected)?;
+
+        for index in self.data_shards..expected_shards {
+            let Some(expected_parity) = expected[index].as_ref() else {
+                return Err(io::Error::other(format!("missing parity shard {index} after verification encode")));
+            };
+            if expected_parity.as_slice() != shards[index] {
+                return Ok(false);
+            }
+        }
+
+        Ok(true)
+    }
+
     fn encode_parity(&self, shards: &mut [Option<Vec<u8>>]) -> io::Result<()> {
         encode_parity_shards(shards, self.data_shards, self.parity_shards, |shards| self.encode(shards))
     }
@@ -257,6 +285,19 @@ impl ReedSolomonEncoder {
         self.encode_parity(shards)
     }
 
+    pub fn verify(&self, shards: &[&[u8]]) -> io::Result<bool> {
+        if shards.iter().all(|shard| shard.is_empty()) {
+            return Ok(true);
+        }
+
+        if let Some(ref rs) = self.encoder {
+            rs.verify(shards)
+                .map_err(|e| io::Error::other(format!("Reed-Solomon verify failed: {e:?}")))
+        } else {
+            Ok(true)
+        }
+    }
+
     fn encode_parity(&self, shards: &mut [Option<Vec<u8>>]) -> io::Result<()> {
         encode_parity_shards(shards, self.data_shards, self.parity_shards, |shards| self.encode(shards))
     }
@@ -656,6 +697,92 @@ impl Erasure {
         Ok(())
     }
 
+    pub(crate) fn decode_data_with_reconstruction_verification(&self, shards: &mut [Option<Vec<u8>>]) -> io::Result<()> {
+        let missing_data_source = shards.iter().take(self.data_shards).any(|shard| shard.is_none());
+        let available_shards = shards.iter().filter(|shard| shard.is_some()).count();
+        let source_parity = if missing_data_source && available_shards > self.data_shards {
+            shards
+                .iter()
+                .enumerate()
+                .skip(self.data_shards)
+                .filter_map(|(index, shard)| shard.as_ref().map(|shard| (index, shard.clone())))
+                .collect::<Vec<_>>()
+        } else {
+            Vec::new()
+        };
+
+        if source_parity.is_empty() {
+            return self.decode_data(shards);
+        }
+
+        self.decode_data_and_parity(shards)?;
+        for (index, source) in source_parity {
+            let Some(rebuilt) = shards[index].as_ref() else {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidData,
+                    "missing rebuilt parity shard after read verification",
+                ));
+            };
+            if rebuilt != &source {
+                warn!(
+                    shard_index = index,
+                    data_shards = self.data_shards,
+                    parity_shards = self.parity_shards,
+                    "erasure decode rejected inconsistent read source shards"
+                );
+                return Err(io::Error::new(io::ErrorKind::InvalidData, "inconsistent read source shards"));
+            }
+        }
+
+        Ok(())
+    }
+
+    pub(crate) fn verify_data_and_parity(&self, shards: &[Option<Vec<u8>>]) -> io::Result<bool> {
+        let expected_shards = self.total_shard_count();
+        if shards.len() != expected_shards {
+            return Err(io::Error::other(format!(
+                "invalid shard count: got {}, expected {}",
+                shards.len(),
+                expected_shards
+            )));
+        }
+        if self.parity_shards == 0 {
+            return Ok(true);
+        }
+
+        let mut shard_refs = Vec::with_capacity(expected_shards);
+        let mut shard_len = None;
+        for (index, shard) in shards.iter().enumerate() {
+            let shard = shard
+                .as_deref()
+                .ok_or_else(|| io::Error::other(format!("missing shard {index} for data/parity verification")))?;
+            if let Some(expected_len) = shard_len {
+                if shard.len() != expected_len {
+                    return Err(io::Error::other(format!(
+                        "inconsistent shard length at index {index}: got {}, expected {}",
+                        shard.len(),
+                        expected_len
+                    )));
+                }
+            } else {
+                shard_len = Some(shard.len());
+            }
+            shard_refs.push(shard);
+        }
+
+        if self.uses_legacy {
+            if let Some(encoder) = self.legacy_encoder.as_ref() {
+                encoder.verify(&shard_refs)
+            } else {
+                Err(io::Error::other("parity_shards > 0, uses_legacy but legacy_encoder is None"))
+            }
+        } else if let Some(encoder) = self.encoder.as_ref() {
+            encoder.verify(&shard_refs)
+        } else {
+            Err(io::Error::other("parity_shards > 0, but encoder is None"))
+        }
+    }
+
     /// Get the total number of shards (data + parity).
     pub fn total_shard_count(&self) -> usize {
         self.data_shards + self.parity_shards

crates/ecstore/src/erasure/coding/heal.rs

@@ -12,15 +12,98 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use crate::disk::disk_store::get_object_disk_read_timeout;
 use crate::disk::error::{Error, Result};
 use crate::erasure::coding::BitrotReader;
 use crate::erasure::coding::BitrotWriterWrapper;
-use crate::erasure::coding::decode::ParallelReader;
 use crate::erasure::coding::encode::MultiWriter;
 use bytes::Bytes;
+use futures::StreamExt;
+use futures::stream::FuturesUnordered;
+use std::io;
+use std::io::ErrorKind;
+use std::time::Duration;
 use tokio::io::AsyncRead;
 use tracing::{info, warn};
 
+async fn read_heal_shards<R>(
+    readers: &mut [Option<BitrotReader<R>>],
+    shard_size: usize,
+    read_timeout: Duration,
+) -> (Vec<Option<Vec<u8>>>, Vec<Option<Error>>)
+where
+    R: AsyncRead + Unpin + Send + Sync,
+{
+    let num_readers = readers.len();
+    let mut shards = vec![None; num_readers];
+    let mut errs = vec![None; num_readers];
+    let mut retire_readers = Vec::new();
+
+    if shard_size == 0 {
+        return (shards, errs);
+    }
+
+    {
+        let mut futures = FuturesUnordered::new();
+        for (index, reader) in readers.iter_mut().enumerate() {
+            let Some(reader) = reader else {
+                errs[index] = Some(Error::FileNotFound);
+                continue;
+            };
+
+            futures.push(Box::pin(async move {
+                let mut buf = vec![0; shard_size];
+                let read_result = if read_timeout.is_zero() {
+                    reader.read(&mut buf).await
+                } else {
+                    match tokio::time::timeout(read_timeout, reader.read(&mut buf)).await {
+                        Ok(result) => result,
+                        Err(_) => {
+                            return (
+                                index,
+                                Err(Error::from(io::Error::new(ErrorKind::TimedOut, "heal shard read timed out"))),
+                                true,
+                            );
+                        }
+                    }
+                };
+
+                match read_result {
+                    Ok(n) => {
+                        buf.truncate(n);
+                        (index, Ok(buf), false)
+                    }
+                    Err(err) => {
+                        let should_retire = err.kind() == ErrorKind::TimedOut;
+                        (index, Err(Error::from(err)), should_retire)
+                    }
+                }
+            }));
+        }
+
+        while let Some((index, result, should_retire)) = futures.next().await {
+            match result {
+                Ok(shard) => {
+                    shards[index] = Some(shard);
+                }
+                Err(err) => {
+                    errs[index] = Some(err);
+                    if should_retire {
+                        retire_readers.push(index);
+                    }
+                }
+            }
+        }
+    }
+
+    for index in retire_readers {
+        readers[index] = None;
+        warn!(shard_index = index, "retiring timed-out heal shard reader");
+    }
+
+    (shards, errs)
+}
+
 impl super::Erasure {
     pub async fn heal<R>(
         &self,
@@ -41,7 +124,7 @@ impl super::Erasure {
         if writers.len() != self.parity_shards + self.data_shards {
             return Err(Error::other("invalid argument"));
         }
-        let mut reader = ParallelReader::new(readers, self.clone(), 0, total_length);
+        let mut readers = readers;
 
         let start_block = 0;
         let mut end_block = total_length / self.block_size;
@@ -52,12 +135,16 @@ impl super::Erasure {
         let available_writers = writers.iter().filter(|w| w.is_some()).count();
         let write_quorum = available_writers.max(1);
         let mut writers = MultiWriter::new(writers, write_quorum);
+        let read_timeout = get_object_disk_read_timeout();
+        let shard_file_size = self.shard_file_size(total_length as i64) as usize;
 
-        for _ in start_block..end_block {
-            let (mut shards, errs) = reader.read().await;
+        for block_index in start_block..end_block {
+            let shard_offset = block_index * self.shard_size();
+            let shard_size = self.shard_size().min(shard_file_size.saturating_sub(shard_offset));
+            let (mut shards, errs) = read_heal_shards(&mut readers, shard_size, read_timeout).await;
 
-            // Check if we have enough shards to reconstruct data
-            // We need at least data_shards available shards (data + parity combined)
+            // Data reads may use the first read quorum, but heal writes must only
+            // proceed when the source set is strong enough to validate itself.
             let available_shards = errs.iter().filter(|e| e.is_none()).count();
             if available_shards < self.data_shards {
                 warn!(
@@ -70,8 +157,38 @@ impl super::Erasure {
                 return Err(Error::ErasureReadQuorum);
             }
 
+            let missing_data_source = shards.iter().take(self.data_shards).any(|shard| shard.is_none());
+            let required_shards = if missing_data_source && self.parity_shards > 0 {
+                self.data_shards + 1
+            } else {
+                self.data_shards
+            };
+            if available_shards < required_shards {
+                return Err(Error::other(format!(
+                    "can not reconstruct data: not enough verified heal source shards (need {}, have {}) {errs:?}",
+                    required_shards, available_shards
+                )));
+            }
+
+            let source_parity = shards
+                .iter()
+                .enumerate()
+                .skip(self.data_shards)
+                .filter_map(|(index, shard)| shard.as_ref().map(|shard| (index, shard.clone())))
+                .collect::<Vec<_>>();
             if self.parity_shards > 0 {
                 self.decode_data_and_parity(&mut shards)?;
+                if !source_parity.is_empty() && !self.verify_data_and_parity(&shards)? {
+                    return Err(Error::other("can not reconstruct data: inconsistent heal source shards"));
+                }
+                for (index, source) in source_parity {
+                    let Some(rebuilt) = shards[index].as_ref() else {
+                        return Err(Error::other("can not reconstruct data: missing rebuilt parity shard"));
+                    };
+                    if rebuilt != &source {
+                        return Err(Error::other("can not reconstruct data: inconsistent heal source shards"));
+                    }
+                }
             }
 
             let shards = shards
@@ -229,4 +346,57 @@ mod tests {
 
         assert!(matches!(err, Error::ErasureReadQuorum));
     }
+
+    #[tokio::test]
+    async fn heal_rejects_inconsistent_sources_before_writing_data_shard() {
+        let erasure = Erasure::new(2, 2, 64);
+        let data = b"heal must not rebuild data from a stale parity shard";
+        let encoded = erasure.encode_data(data).expect("encode should succeed");
+        let missing_data = 1;
+        let corrupt_parity = erasure.data_shards;
+
+        let readers = encoded
+            .iter()
+            .enumerate()
+            .map(|(index, shard)| {
+                if index == missing_data {
+                    return None;
+                }
+
+                let mut shard = shard.to_vec();
+                if index == corrupt_parity {
+                    shard[0] ^= 0x5a;
+                }
+
+                Some(BitrotReader::new(Cursor::new(shard), erasure.shard_size(), HashAlgorithm::None, false))
+            })
+            .collect::<Vec<_>>();
+
+        let mut writers = (0..erasure.total_shard_count())
+            .map(|index| {
+                if index == missing_data {
+                    Some(BitrotWriterWrapper::new(
+                        CustomWriter::new_inline_buffer(),
+                        erasure.shard_size(),
+                        HashAlgorithm::None,
+                    ))
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<_>>();
+
+        let err = erasure
+            .heal(&mut writers, readers, data.len(), &[])
+            .await
+            .expect_err("heal should reject inconsistent source shards");
+        assert!(err.to_string().contains("inconsistent heal source shards"));
+
+        let written = writers[missing_data]
+            .take()
+            .expect("data writer should remain")
+            .into_inline_data()
+            .expect("inline writer should retain data");
+        assert!(written.is_empty(), "heal must fail before writing rebuilt data");
+    }
 }