clawhdf5-remote: a failed first fetch is Error::Remote
cached() mapped an error of its open-time prefetch (a network error, a changed file) to Error::Hdf5(Format(Storage)), misclassifying it for callers that match on the variant. It is now Error::Remote (RemoteError::Backend with the backend's message). open_object and the s3/gs/az URLs fetch the first block of an ObjectStoreStorage directly, so their errors keep their kind (FileChanged, ObjectStore). Test: cached() over a backend whose reads fail gives Error::Remote(Backend) (Error::Hdf5 before). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -126,7 +126,7 @@ fn http_storage(url: &str, _options: &Options) -> Result<Arc<RemoteStorage>, Err
|
|||||||
fn cloud_storage(url: &str, _scheme: &str, options: &Options) -> Result<Arc<RemoteStorage>, Error> {
|
fn cloud_storage(url: &str, _scheme: &str, options: &Options) -> Result<Arc<RemoteStorage>, Error> {
|
||||||
let (store, path) = object::store_for_url(url)?;
|
let (store, path) = object::store_for_url(url)?;
|
||||||
let storage = ObjectStoreStorage::new(store, path)?;
|
let storage = ObjectStoreStorage::new(store, path)?;
|
||||||
Ok(Arc::new(cached(Box::new(storage), options)?))
|
Ok(Arc::new(object_cached(storage, options)?))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(not(any(feature = "s3", feature = "gcs", feature = "azure")))]
|
#[cfg(not(any(feature = "s3", feature = "gcs", feature = "azure")))]
|
||||||
@@ -144,13 +144,27 @@ fn cloud_storage(url: &str, scheme: &str, _options: &Options) -> Result<Arc<Remo
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// A [`BlockCache`] over `backend` with its first block fetched (readahead
|
/// A [`BlockCache`] over `backend` with its first block fetched (readahead
|
||||||
/// of the superblock and the metadata usually written next to it).
|
/// of the superblock and the metadata usually written next to it). A
|
||||||
|
/// failure of that fetch is [`Error::Remote`] ([`RemoteError::Backend`],
|
||||||
|
/// with the backend's message).
|
||||||
pub fn cached(backend: Backend, options: &Options) -> Result<RemoteStorage, Error> {
|
pub fn cached(backend: Backend, options: &Options) -> Result<RemoteStorage, Error> {
|
||||||
let cache = BlockCache::new(backend, options.cache.clone());
|
let cache = BlockCache::new(backend, options.cache.clone());
|
||||||
let first = cache.config().block_size;
|
let first = cache.config().block_size;
|
||||||
cache
|
cache
|
||||||
.prefetch(0, first)
|
.prefetch(0, first)
|
||||||
.map_err(|e| Error::Hdf5(clawhdf5::Error::Format(e)))?;
|
.map_err(|e| RemoteError::Backend(e.to_string()))?;
|
||||||
|
Ok(cache)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// [`cached`] for an [`ObjectStoreStorage`]: its first block is fetched
|
||||||
|
/// directly, so a failure keeps its kind (`FileChanged`, `ObjectStore`).
|
||||||
|
#[cfg(feature = "object-store")]
|
||||||
|
fn object_cached(storage: ObjectStoreStorage, options: &Options) -> Result<RemoteStorage, Error> {
|
||||||
|
// The block size BlockCache::new will use.
|
||||||
|
let block = options.cache.block_size.max(512);
|
||||||
|
let first = storage.fetch_first(block)?;
|
||||||
|
let cache = BlockCache::new(Box::new(storage) as Backend, options.cache.clone());
|
||||||
|
cache.insert(0, &first);
|
||||||
Ok(cache)
|
Ok(cache)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -205,8 +219,8 @@ pub fn open_object(
|
|||||||
) -> Result<(File, Arc<RemoteStorage>), Error> {
|
) -> Result<(File, Arc<RemoteStorage>), Error> {
|
||||||
let path = object_store::path::Path::parse(path)
|
let path = object_store::path::Path::parse(path)
|
||||||
.map_err(|e| RemoteError::InvalidUrl(format!("{path}: {e}")))?;
|
.map_err(|e| RemoteError::InvalidUrl(format!("{path}: {e}")))?;
|
||||||
let storage = Arc::new(cached(
|
let storage = Arc::new(object_cached(
|
||||||
Box::new(ObjectStoreStorage::new(store, path)?),
|
ObjectStoreStorage::new(store, path)?,
|
||||||
options,
|
options,
|
||||||
)?);
|
)?);
|
||||||
let file = File::open_storage(storage.clone())?;
|
let file = File::open_storage(storage.clone())?;
|
||||||
|
|||||||
@@ -138,6 +138,11 @@ impl ObjectStoreStorage {
|
|||||||
o
|
o
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The object's first `n` bytes (fewer if it is shorter).
|
||||||
|
pub(crate) fn fetch_first(&self, n: u64) -> Result<Vec<u8>, RemoteError> {
|
||||||
|
Ok(self.fetch_all(&[0..n])?.pop().unwrap_or_default())
|
||||||
|
}
|
||||||
|
|
||||||
fn fetch_all(&self, ranges: &[Range<u64>]) -> Result<Vec<Vec<u8>>, RemoteError> {
|
fn fetch_all(&self, ranges: &[Range<u64>]) -> Result<Vec<Vec<u8>>, RemoteError> {
|
||||||
let len = self.meta.size;
|
let len = self.meta.size;
|
||||||
let jobs: Vec<(usize, Range<u64>)> = ranges
|
let jobs: Vec<(usize, Range<u64>)> = ranges
|
||||||
|
|||||||
@@ -834,3 +834,27 @@ fn slow_links_read_and_stalled_ones_fail() {
|
|||||||
assert!(matches!(e, Error::Remote(RemoteError::Transport(_))), "{e}");
|
assert!(matches!(e, Error::Remote(RemoteError::Transport(_))), "{e}");
|
||||||
assert!(t.elapsed() < Duration::from_secs(5), "{:?}", t.elapsed());
|
assert!(t.elapsed() < Duration::from_secs(5), "{:?}", t.elapsed());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `cached` reports a failed first fetch as a remote error, not as an
|
||||||
|
/// HDF5 format error.
|
||||||
|
#[test]
|
||||||
|
fn cached_reports_a_failed_prefetch_as_remote() {
|
||||||
|
use clawhdf5_format::error::FormatError;
|
||||||
|
use std::borrow::Cow;
|
||||||
|
struct Down;
|
||||||
|
impl clawhdf5_format::storage::Storage for Down {
|
||||||
|
fn read_at(&self, _: u64, _: usize) -> Result<Cow<'_, [u8]>, FormatError> {
|
||||||
|
Err(RemoteError::Transport("connection refused".into()).into())
|
||||||
|
}
|
||||||
|
fn len(&self) -> u64 {
|
||||||
|
1 << 20
|
||||||
|
}
|
||||||
|
}
|
||||||
|
let e = clawhdf5_remote::cached(Box::new(Down), &Options::default())
|
||||||
|
.map(|_| ())
|
||||||
|
.unwrap_err();
|
||||||
|
assert!(
|
||||||
|
matches!(&e, Error::Remote(RemoteError::Backend(m)) if m.contains("connection refused")),
|
||||||
|
"{e:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user