From 1d3d7f6b8791b895c8cf85e6a1409b7d8bacc5fb Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Wed, 26 Aug 2026 10:24:02 +0200 Subject: [PATCH 1/6] Optimize single-file reads from split bundles Reuse tail bytes when possible to avoid opening the full bundle and reduce object storage requests. --- .../quickwit-storage/src/bundle_storage.rs | 315 ++++++++++++++++-- 1 file changed, 280 insertions(+), 35 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index 99a17d47a3f..cb08cee8c62 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -93,6 +93,62 @@ impl BundleStorage { pub fn iter_files(&self) -> impl Iterator { self.file_ranges.files.keys() } + + /// Fetches a bundled file from a split. + /// + /// Use this only when retrieving a single file from the split. To retrieve multiple files, + /// prefer [`Self::open_from_storage`]. + /// + /// The split length is provided by the caller (e.g. from object listing metadata) to avoid a + /// separate metadata request. + pub async fn fetch_file_from_split( + storage: Arc, + bundle_filepath: PathBuf, + split_path: &Path, + split_len: u64, + ) -> anyhow::Result<(OwnedBytes, Range)> { + let (split_bytes, footer_range) = fetch_split_tail( + storage.as_ref(), + split_path, + split_len, + DEFAULT_SPLIT_TAIL_WINDOW_NUM_BYTES, + ) + .await?; + + // Parse the bundle file ranges from the split bytes. + let tail_start = split_len - split_bytes.len() as u64; + let (file_ranges, _hotcache) = + BundleFileRanges::open_from_split_bytes(split_bytes.clone())?; + let file_range = file_ranges.get(&bundle_filepath).ok_or_else(|| { + anyhow::anyhow!( + "missing file `{}` in split bundle", + bundle_filepath.display() + ) + })?; + ensure!( + file_range.start <= file_range.end, + "bundled file range starts after it ends" + ); + ensure!( + file_range.end <= footer_range.start, + "bundled file range overlaps split footer" + ); + + // If the initial tail also contains the file, reuse it and complete in one GET (at least). + // Otherwise, fetch the file with an additional GET. + let file_bytes = if file_range.start >= tail_start { + let relative_start = usize::try_from(file_range.start - tail_start)?; + let relative_end = usize::try_from(file_range.end - tail_start)?; + split_bytes.slice(relative_start..relative_end) + } else { + let relative_start = usize::try_from(file_range.start)?; + let relative_end = usize::try_from(file_range.end)?; + storage + .get_slice(split_path, relative_start..relative_end) + .await? + }; + Ok((file_bytes, footer_range)) + } } const HOTCACHE_LEN_NUM_BYTES: usize = std::mem::size_of::(); @@ -104,6 +160,7 @@ const SPLIT_FOOTER_TRAILER_VERSION_NUM_BYTES: usize = std::mem::size_of::() pub(crate) const SPLIT_FOOTER_TRAILER_NUM_BYTES: usize = SPLIT_FOOTER_START_NUM_BYTES + SPLIT_FOOTER_TRAILER_VERSION_NUM_BYTES + SPLIT_FOOTER_TRAILER_MAGIC.len(); +const DEFAULT_SPLIT_TAIL_WINDOW_NUM_BYTES: u64 = 1024 * 1024; pub(crate) fn serialize_split_footer_trailer( footer_start_inclusive: u64, @@ -145,44 +202,152 @@ pub async fn locate_split_footer_range( split_len >= SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, "split is too short to contain a footer" ); - let trailer_start = split_len - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64; - let trailer = storage - .get_slice( - split_path, - usize::try_from(trailer_start)?..usize::try_from(split_len)?, - ) - .await?; - if let Some(footer_start_inclusive) = deserialize_split_footer_trailer(&trailer)? { + let tail_start = split_len - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64; + let start = usize::try_from(tail_start)?; + let end = usize::try_from(split_len)?; + let tail_bytes = storage.get_slice(split_path, start..end).await?; + match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { + FooterLocation::Located(footer_range) => Ok(footer_range), + FooterLocation::ReadBundleMetadataLen(bundle_metadata_len_range) => { + let start = usize::try_from(bundle_metadata_len_range.start)?; + let end = usize::try_from(bundle_metadata_len_range.end)?; + let bundle_metadata_len_bytes = storage.get_slice(split_path, start..end).await?; + locate_split_footer_range_from_metadata_len( + split_len, + bundle_metadata_len_range.start, + bundle_metadata_len_bytes.as_slice(), + ) + } + } +} + +enum FooterLocation { + Located(Range), + /// The exact range containing the bundle-metadata length. + ReadBundleMetadataLen(Range), +} + +fn locate_split_footer_range_from_metadata_len( + split_len: u64, + bundle_metadata_len_start: u64, + bundle_metadata_len_bytes: &[u8], +) -> anyhow::Result> { + let bundle_metadata_len = u32::from_le_bytes(bundle_metadata_len_bytes.try_into()?) as u64; + let footer_start = bundle_metadata_len_start + .checked_sub(bundle_metadata_len) + .context("split footer exceeds split length")?; + Ok(footer_start..split_len) +} + +fn locate_split_footer_range_in_tail( + split_len: u64, + tail_bytes: &OwnedBytes, +) -> anyhow::Result { + // Legacy split layout: + // [body][bundle metadata][metadata len][hotcache][hotcache len] + ensure!( + tail_bytes.len() as u64 <= split_len, + "split tail is longer than the split itself" + ); + let bytes = tail_bytes.as_slice(); + let trailer_start = bytes + .len() + .checked_sub(SPLIT_FOOTER_TRAILER_NUM_BYTES) + .context("split tail is too short to contain a footer trailer")?; + if let Some(footer_start) = deserialize_split_footer_trailer(&bytes[trailer_start..])? { ensure!( - footer_start_inclusive <= trailer_start, + footer_start <= split_len - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, "split footer starts after its trailer" ); - return Ok(footer_start_inclusive..split_len); + return Ok(FooterLocation::Located(footer_start..split_len)); } - // Legacy split layout: - // [body][bundle metadata][metadata len][hotcache][hotcache len] - let hotcache_len = u32::from_le_bytes(trailer[12..].try_into().unwrap()) as u64; - let bundle_metadata_len_offset = split_len + let hotcache_len = + u32::from_le_bytes(bytes[bytes.len() - HOTCACHE_LEN_NUM_BYTES..].try_into()?) as u64; + let bundle_metadata_len_end = split_len .checked_sub(HOTCACHE_LEN_NUM_BYTES as u64) .and_then(|offset| offset.checked_sub(hotcache_len)) - .and_then(|offset| offset.checked_sub(BUNDLE_METADATA_LEN_NUM_BYTES as u64)) - .ok_or_else(|| anyhow::anyhow!("invalid legacy split footer lengths"))?; - let bundle_metadata_len_bytes = storage - .get_slice( - split_path, - usize::try_from(bundle_metadata_len_offset)? - ..usize::try_from( - bundle_metadata_len_offset + BUNDLE_METADATA_LEN_NUM_BYTES as u64, - )?, - ) - .await?; - let bundle_metadata_len = - u32::from_le_bytes(bundle_metadata_len_bytes.as_ref().try_into().unwrap()) as u64; - let footer_start_inclusive = bundle_metadata_len_offset - .checked_sub(bundle_metadata_len) - .ok_or_else(|| anyhow::anyhow!("invalid legacy split metadata length"))?; - Ok(footer_start_inclusive..split_len) + .context("split footer exceeds split length")?; + let bundle_metadata_len_start = bundle_metadata_len_end + .checked_sub(BUNDLE_METADATA_LEN_NUM_BYTES as u64) + .context("split footer exceeds split length")?; + let bundle_metadata_len_range = bundle_metadata_len_start..bundle_metadata_len_end; + let tail_start = split_len - tail_bytes.len() as u64; + if tail_start > bundle_metadata_len_start { + return Ok(FooterLocation::ReadBundleMetadataLen( + bundle_metadata_len_range, + )); + } + + let relative_start = usize::try_from(bundle_metadata_len_start - tail_start)?; + let relative_end = usize::try_from(bundle_metadata_len_end - tail_start)?; + let footer_range = locate_split_footer_range_from_metadata_len( + split_len, + bundle_metadata_len_start, + &bytes[relative_start..relative_end], + )?; + Ok(FooterLocation::Located(footer_range)) +} + +/// Reads the tail of a split until it holds the complete bundle footer. +/// +/// `initial_tail_window_num_bytes` is a jump-start hint: a larger window downloads more up front +/// but can save range GETs by covering the footer, and possibly the wanted file, in one read. +/// When the window falls short, the tail is re-read: once for a new split, whose trailer gives +/// the footer start, and up to twice for a legacy split, whose footer length is derived from the +/// trailing hotcache and bundle-metadata lengths. +/// +/// Returns the tail, which may start before the footer, and the footer range within the split. +async fn fetch_split_tail( + storage: &dyn Storage, + split_path: &Path, + split_len: u64, + initial_tail_window_num_bytes: u64, +) -> anyhow::Result<(OwnedBytes, Range)> { + ensure!( + split_len >= SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, + "split is too short to contain a footer" + ); + ensure!( + initial_tail_window_num_bytes > 0, + "split tail window must be positive" + ); + + // The tail always covers the fixed trailer so that footer parsing has a lower bound to work + // with, even when the caller asks for a very small window. + let mut tail_window_num_bytes = initial_tail_window_num_bytes + .max(SPLIT_FOOTER_TRAILER_NUM_BYTES as u64) + .min(split_len); + // Read the tail in a loop until we find the footer. + let (tail_bytes, footer_range) = loop { + let tail_start = split_len - tail_window_num_bytes; + let start = usize::try_from(tail_start)?; + let end = usize::try_from(split_len)?; + let tail_bytes = storage.get_slice(split_path, start..end).await?; + let required_tail_num_bytes = + match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { + FooterLocation::Located(footer_range) => { + let footer_num_bytes = footer_range.end - footer_range.start; + if tail_bytes.len() as u64 >= footer_num_bytes { + break (tail_bytes, footer_range); + } + footer_num_bytes + } + FooterLocation::ReadBundleMetadataLen(bundle_metadata_len_range) => { + split_len - bundle_metadata_len_range.start + } + }; + ensure!( + required_tail_num_bytes > tail_window_num_bytes, + "failed to locate split footer" + ); + ensure!( + required_tail_num_bytes <= split_len, + "split footer exceeds split length" + ); + tail_window_num_bytes = required_tail_num_bytes; + }; + Ok((tail_bytes, footer_range)) } /// Removes the fixed split footer trailer when it is present. @@ -418,7 +583,9 @@ mod tests { use std::io::Write; use super::*; - use crate::{PutPayload, RamStorageBuilder, SplitPayloadBuilder}; + use crate::{CountingStorage, PutPayload, RamStorageBuilder, SplitPayloadBuilder}; + + const DEFAULT_SPLIT_TAIL_WINDOW_NUM_BYTES: u64 = 1024 * 1024; #[tokio::test] async fn bundle_storage_locates_footer_from_object_storage() { @@ -447,24 +614,36 @@ mod tests { #[tokio::test] async fn bundle_storage_locates_legacy_footer_from_object_storage() { + let hotcache_bytes = vec![0u8; 1024]; let split_payload = - SplitPayloadBuilder::get_split_payload(&[], b"fields", None, b"hotcache").unwrap(); + SplitPayloadBuilder::get_split_payload(&[], b"fields", None, &hotcache_bytes).unwrap(); let expected_footer_range = split_payload.footer_range.clone(); let split_bytes = split_payload.read_all().await.unwrap(); let split_path = PathBuf::from("legacy-split"); - let storage = Arc::new( + let inner_storage: Arc = Arc::new( RamStorageBuilder::default() .put(&split_path.to_string_lossy(), &split_bytes) .build(), ); + let (storage, counters) = CountingStorage::instrument_storage(inner_storage); let (_bundle_storage, hotcache, footer_range) = BundleStorage::open_from_storage(storage, split_path) .await .unwrap(); - assert_eq!(hotcache.as_ref(), b"hotcache"); + assert_eq!(hotcache.as_ref(), hotcache_bytes.as_slice()); assert_eq!(footer_range, expected_footer_range); + let footer_num_bytes = footer_range.end - footer_range.start; + assert_eq!( + counters.snapshot(), + ( + SPLIT_FOOTER_TRAILER_NUM_BYTES as u64 + + BUNDLE_METADATA_LEN_NUM_BYTES as u64 + + footer_num_bytes, + 3, + ) + ); } #[tokio::test] @@ -508,6 +687,7 @@ mod tests { Ok(()) } + #[tokio::test] async fn bundle_storage_test() -> anyhow::Result<()> { let temp_dir = tempfile::tempdir()?; @@ -580,4 +760,69 @@ mod tests { Ok(()) } + + #[tokio::test] + async fn test_fetch_file_from_split_uses_one_tail_read() { + let mut split_builder = SplitPayloadBuilder::default(); + split_builder.add_payload( + "large-file".to_string(), + Box::new(vec![0u8; DEFAULT_SPLIT_TAIL_WINDOW_NUM_BYTES as usize + 1]), + ); + split_builder.add_payload("target".to_string(), Box::new(b"target-bytes".to_vec())); + let split_payload = split_builder + .finalize_with_footer_trailer(b"hotcache", true) + .unwrap(); + let expected_footer_range = split_payload.footer_range.clone(); + let split_bytes = split_payload.read_all().await.unwrap(); + let inner_storage: Arc = Arc::new( + RamStorageBuilder::default() + .put("split", &split_bytes) + .build(), + ); + let (storage, counters) = CountingStorage::instrument_storage(inner_storage); + + let (target_bytes, footer_range) = BundleStorage::fetch_file_from_split( + storage, + PathBuf::from("target"), + Path::new("split"), + split_bytes.len() as u64, + ) + .await + .unwrap(); + + assert_eq!(target_bytes.as_slice(), b"target-bytes"); + assert_eq!(footer_range, expected_footer_range); + assert_eq!(counters.snapshot().1, 1); + } + + #[tokio::test] + async fn test_fetch_file_from_split_widens_then_fetches_file_range() { + let mut split_builder = SplitPayloadBuilder::default(); + split_builder.add_payload("target".to_string(), Box::new(b"target-bytes".to_vec())); + let hotcache = vec![0u8; DEFAULT_SPLIT_TAIL_WINDOW_NUM_BYTES as usize + 1]; + let split_payload = split_builder + .finalize_with_footer_trailer(&hotcache, false) + .unwrap(); + let expected_footer_range = split_payload.footer_range.clone(); + let split_bytes = split_payload.read_all().await.unwrap(); + let inner_storage: Arc = Arc::new( + RamStorageBuilder::default() + .put("split", &split_bytes) + .build(), + ); + let (storage, counters) = CountingStorage::instrument_storage(inner_storage); + + let (target_bytes, footer_range) = BundleStorage::fetch_file_from_split( + storage, + PathBuf::from("target"), + Path::new("split"), + split_bytes.len() as u64, + ) + .await + .unwrap(); + + assert_eq!(target_bytes.as_slice(), b"target-bytes"); + assert_eq!(footer_range, expected_footer_range); + assert_eq!(counters.snapshot().1, 4); + } } From 9214ad9da2dd0c4591659673228d2d6b828d6ef2 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Wed, 26 Aug 2026 10:31:26 +0200 Subject: [PATCH 2/6] Cover both legacy footer read paths --- .../quickwit-storage/src/bundle_storage.rs | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index cb08cee8c62..d6a3c20a78d 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -614,6 +614,30 @@ mod tests { #[tokio::test] async fn bundle_storage_locates_legacy_footer_from_object_storage() { + let split_payload = + SplitPayloadBuilder::get_split_payload(&[], b"fields", None, b"hotcache").unwrap(); + let expected_footer_range = split_payload.footer_range.clone(); + let split_bytes = split_payload.read_all().await.unwrap(); + let split_path = PathBuf::from("legacy-split"); + let inner_storage: Arc = Arc::new( + RamStorageBuilder::default() + .put(&split_path.to_string_lossy(), &split_bytes) + .build(), + ); + let (storage, counters) = CountingStorage::instrument_storage(inner_storage); + + let (_bundle_storage, hotcache, footer_range) = + BundleStorage::open_from_storage(storage, split_path) + .await + .unwrap(); + + assert_eq!(hotcache.as_ref(), b"hotcache"); + assert_eq!(footer_range, expected_footer_range); + assert_eq!(counters.snapshot().1, 2); + } + + #[tokio::test] + async fn bundle_storage_locates_legacy_footer_with_large_hotcache() { let hotcache_bytes = vec![0u8; 1024]; let split_payload = SplitPayloadBuilder::get_split_payload(&[], b"fields", None, &hotcache_bytes).unwrap(); @@ -634,16 +658,7 @@ mod tests { assert_eq!(hotcache.as_ref(), hotcache_bytes.as_slice()); assert_eq!(footer_range, expected_footer_range); - let footer_num_bytes = footer_range.end - footer_range.start; - assert_eq!( - counters.snapshot(), - ( - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64 - + BUNDLE_METADATA_LEN_NUM_BYTES as u64 - + footer_num_bytes, - 3, - ) - ); + assert_eq!(counters.snapshot().1, 3); } #[tokio::test] From 5947c79e29ecca4d53d618064aaa6f3ad050e457 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Thu, 27 Aug 2026 17:20:18 +0200 Subject: [PATCH 3/6] Simplify split offset conversions for 64-bit targets --- .../quickwit-storage/src/bundle_storage.rs | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index d6a3c20a78d..fcaf12e071a 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -56,9 +56,8 @@ impl BundleStorage { let split_len = storage.file_num_bytes(&bundle_filepath).await?; let split_footer_range = locate_split_footer_range(storage.as_ref(), &bundle_filepath, split_len).await?; - let split_footer_start = usize::try_from(split_footer_range.start)?; - let split_footer_end = usize::try_from(split_footer_range.end)?; - let split_footer_range_usize = split_footer_start..split_footer_end; + let split_footer_range_usize = + split_footer_range.start as usize..split_footer_range.end as usize; let split_footer_bytes = storage .get_slice(&bundle_filepath, split_footer_range_usize) .await?; @@ -137,12 +136,12 @@ impl BundleStorage { // If the initial tail also contains the file, reuse it and complete in one GET (at least). // Otherwise, fetch the file with an additional GET. let file_bytes = if file_range.start >= tail_start { - let relative_start = usize::try_from(file_range.start - tail_start)?; - let relative_end = usize::try_from(file_range.end - tail_start)?; + let relative_start = (file_range.start - tail_start) as usize; + let relative_end = (file_range.end - tail_start) as usize; split_bytes.slice(relative_start..relative_end) } else { - let relative_start = usize::try_from(file_range.start)?; - let relative_end = usize::try_from(file_range.end)?; + let relative_start = file_range.start as usize; + let relative_end = file_range.end as usize; storage .get_slice(split_path, relative_start..relative_end) .await? @@ -202,15 +201,14 @@ pub async fn locate_split_footer_range( split_len >= SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, "split is too short to contain a footer" ); - let tail_start = split_len - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64; - let start = usize::try_from(tail_start)?; - let end = usize::try_from(split_len)?; + let end = split_len as usize; + let start = end - SPLIT_FOOTER_TRAILER_NUM_BYTES; let tail_bytes = storage.get_slice(split_path, start..end).await?; match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { FooterLocation::Located(footer_range) => Ok(footer_range), FooterLocation::ReadBundleMetadataLen(bundle_metadata_len_range) => { - let start = usize::try_from(bundle_metadata_len_range.start)?; - let end = usize::try_from(bundle_metadata_len_range.end)?; + let start = bundle_metadata_len_range.start as usize; + let end = bundle_metadata_len_range.end as usize; let bundle_metadata_len_bytes = storage.get_slice(split_path, start..end).await?; locate_split_footer_range_from_metadata_len( split_len, @@ -279,8 +277,8 @@ fn locate_split_footer_range_in_tail( )); } - let relative_start = usize::try_from(bundle_metadata_len_start - tail_start)?; - let relative_end = usize::try_from(bundle_metadata_len_end - tail_start)?; + let relative_start = (bundle_metadata_len_start - tail_start) as usize; + let relative_end = (bundle_metadata_len_end - tail_start) as usize; let footer_range = locate_split_footer_range_from_metadata_len( split_len, bundle_metadata_len_start, @@ -321,8 +319,8 @@ async fn fetch_split_tail( // Read the tail in a loop until we find the footer. let (tail_bytes, footer_range) = loop { let tail_start = split_len - tail_window_num_bytes; - let start = usize::try_from(tail_start)?; - let end = usize::try_from(split_len)?; + let start = tail_start as usize; + let end = split_len as usize; let tail_bytes = storage.get_slice(split_path, start..end).await?; let required_tail_num_bytes = match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { From 904fcfb9388bf2089eb0f6734c31f8244851ac37 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Thu, 27 Aug 2026 17:48:38 +0200 Subject: [PATCH 4/6] Make split tail read sequence explicit --- .../quickwit-storage/src/bundle_storage.rs | 82 +++++++++++-------- 1 file changed, 48 insertions(+), 34 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index fcaf12e071a..03fdc34e60a 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -306,48 +306,62 @@ async fn fetch_split_tail( split_len >= SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, "split is too short to contain a footer" ); - ensure!( - initial_tail_window_num_bytes > 0, - "split tail window must be positive" - ); - // The tail always covers the fixed trailer so that footer parsing has a lower bound to work - // with, even when the caller asks for a very small window. - let mut tail_window_num_bytes = initial_tail_window_num_bytes + // Start with the requested window, but always cover the fixed trailer and never read before + // the beginning of the split. + let initial_tail_num_bytes = initial_tail_window_num_bytes .max(SPLIT_FOOTER_TRAILER_NUM_BYTES as u64) .min(split_len); - // Read the tail in a loop until we find the footer. - let (tail_bytes, footer_range) = loop { - let tail_start = split_len - tail_window_num_bytes; - let start = tail_start as usize; - let end = split_len as usize; - let tail_bytes = storage.get_slice(split_path, start..end).await?; - let required_tail_num_bytes = - match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { - FooterLocation::Located(footer_range) => { - let footer_num_bytes = footer_range.end - footer_range.start; - if tail_bytes.len() as u64 >= footer_num_bytes { - break (tail_bytes, footer_range); - } - footer_num_bytes - } - FooterLocation::ReadBundleMetadataLen(bundle_metadata_len_range) => { - split_len - bundle_metadata_len_range.start + let mut tail_bytes = + read_split_tail(storage, split_path, split_len, initial_tail_num_bytes).await?; + + // Locate the footer range. + let footer_range = match locate_split_footer_range_in_tail(split_len, &tail_bytes)? { + FooterLocation::Located(footer_range) => { + // The range is known, but the initial tail may not contain the complete footer. + footer_range + } + FooterLocation::ReadBundleMetadataLen(bundle_metadata_len_range) => { + // Extend the tail through the legacy bundle-metadata length field, then use that + // length to locate the beginning of the footer. + let required_tail_num_bytes = split_len - bundle_metadata_len_range.start; + let metadata_len_tail_bytes = + read_split_tail(storage, split_path, split_len, required_tail_num_bytes).await?; + match locate_split_footer_range_in_tail(split_len, &metadata_len_tail_bytes)? { + FooterLocation::Located(footer_range) => footer_range, + FooterLocation::ReadBundleMetadataLen(_) => { + bail!("failed to locate split footer after reading bundle metadata length"); } - }; - ensure!( - required_tail_num_bytes > tail_window_num_bytes, - "failed to locate split footer" - ); - ensure!( - required_tail_num_bytes <= split_len, - "split footer exceeds split length" - ); - tail_window_num_bytes = required_tail_num_bytes; + } + } }; + + // If the initial tail does not contain the entire footer, fetch the exact footer range now + // that its boundaries are known. + let footer_num_bytes = footer_range.end - footer_range.start; + if (tail_bytes.len() as u64) < footer_num_bytes { + tail_bytes = read_split_tail(storage, split_path, split_len, footer_num_bytes).await?; + } Ok((tail_bytes, footer_range)) } +async fn read_split_tail( + storage: &dyn Storage, + split_path: &Path, + split_len: u64, + tail_num_bytes: u64, +) -> anyhow::Result { + ensure!( + tail_num_bytes <= split_len, + "split tail exceeds split length" + ); + let start = (split_len - tail_num_bytes) as usize; + let tail_bytes = storage + .get_slice(split_path, start..split_len as usize) + .await?; + Ok(tail_bytes) +} + /// Removes the fixed split footer trailer when it is present. pub fn strip_split_footer_trailer(split_slice: FileSlice) -> anyhow::Result { if split_slice.len() < SPLIT_FOOTER_TRAILER_NUM_BYTES { From 5eb77e45224c2ef5992d3afb61deaab58117b14c Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Fri, 28 Aug 2026 09:21:14 +0200 Subject: [PATCH 5/6] nit: accept slice in locate_split_footer_range_in_tail --- quickwit/quickwit-storage/src/bundle_storage.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index 03fdc34e60a..a3b289b5d5c 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -239,7 +239,7 @@ fn locate_split_footer_range_from_metadata_len( fn locate_split_footer_range_in_tail( split_len: u64, - tail_bytes: &OwnedBytes, + tail_bytes: &[u8], ) -> anyhow::Result { // Legacy split layout: // [body][bundle metadata][metadata len][hotcache][hotcache len] @@ -247,12 +247,11 @@ fn locate_split_footer_range_in_tail( tail_bytes.len() as u64 <= split_len, "split tail is longer than the split itself" ); - let bytes = tail_bytes.as_slice(); - let trailer_start = bytes + let trailer_start = tail_bytes .len() .checked_sub(SPLIT_FOOTER_TRAILER_NUM_BYTES) .context("split tail is too short to contain a footer trailer")?; - if let Some(footer_start) = deserialize_split_footer_trailer(&bytes[trailer_start..])? { + if let Some(footer_start) = deserialize_split_footer_trailer(&tail_bytes[trailer_start..])? { ensure!( footer_start <= split_len - SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, "split footer starts after its trailer" @@ -261,7 +260,8 @@ fn locate_split_footer_range_in_tail( } let hotcache_len = - u32::from_le_bytes(bytes[bytes.len() - HOTCACHE_LEN_NUM_BYTES..].try_into()?) as u64; + u32::from_le_bytes(tail_bytes[tail_bytes.len() - HOTCACHE_LEN_NUM_BYTES..].try_into()?) + as u64; let bundle_metadata_len_end = split_len .checked_sub(HOTCACHE_LEN_NUM_BYTES as u64) .and_then(|offset| offset.checked_sub(hotcache_len)) @@ -282,7 +282,7 @@ fn locate_split_footer_range_in_tail( let footer_range = locate_split_footer_range_from_metadata_len( split_len, bundle_metadata_len_start, - &bytes[relative_start..relative_end], + &tail_bytes[relative_start..relative_end], )?; Ok(FooterLocation::Located(footer_range)) } From 575229abd605119d64dfe88afa6619548e7a38a6 Mon Sep 17 00:00:00 2001 From: Luca Cominardi Date: Fri, 28 Aug 2026 12:35:06 +0200 Subject: [PATCH 6/6] Simplify split tail window bounds --- quickwit/quickwit-storage/src/bundle_storage.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/quickwit/quickwit-storage/src/bundle_storage.rs b/quickwit/quickwit-storage/src/bundle_storage.rs index a3b289b5d5c..4ed0d5534bf 100644 --- a/quickwit/quickwit-storage/src/bundle_storage.rs +++ b/quickwit/quickwit-storage/src/bundle_storage.rs @@ -309,9 +309,8 @@ async fn fetch_split_tail( // Start with the requested window, but always cover the fixed trailer and never read before // the beginning of the split. - let initial_tail_num_bytes = initial_tail_window_num_bytes - .max(SPLIT_FOOTER_TRAILER_NUM_BYTES as u64) - .min(split_len); + let initial_tail_num_bytes = + initial_tail_window_num_bytes.clamp(SPLIT_FOOTER_TRAILER_NUM_BYTES as u64, split_len); let mut tail_bytes = read_split_tail(storage, split_path, split_len, initial_tail_num_bytes).await?;