JingsongLi commented on code in PR #998:
URL: https://github.com/apache/paimon-rust/pull/998#discussion_r4173341701
##########
crates/paimon/src/io/file_io.rs:
##########
@@ -676,17 +826,40 @@ fn status_path(base_path: &str, entry_path: &str) ->
String {
}
}
-fn cache_object_path(op: &Operator, relative_path: &str) -> String {
+fn cache_object_path(namespace: &str, op: &Operator, relative_path: &str) ->
String {
let info = op.info();
format!(
- "{}\0{}\0{}\0{}",
+ "{}\0{}\0{}\0{}\0{}",
+ namespace,
info.scheme(),
info.name(),
info.root(),
relative_path.trim_start_matches('/')
)
}
+fn storage_cache_namespace(scheme: &str, props: &HashMap<String, String>) ->
Arc<str> {
+ // Credentials may rotate while the storage namespace stays unchanged.
+ // Endpoints identify storage without coupling cache reuse to credentials.
+ let mut endpoints = props
+ .iter()
+ .filter(|(key, _)| key.to_ascii_lowercase().ends_with("endpoint"))
+ .collect::<Vec<_>>();
+ endpoints.sort_unstable_by_key(|(key, _)| *key);
+
+ let mut digest = Sha256::new();
+ digest.update(scheme.to_ascii_lowercase());
Review Comment:
[P2] Canonicalize storage aliases before hashing the cache namespace
This hashes the raw builder scheme, although `Storage::build` maps `file`,
`fs`, and the empty scheme to the same Fs backend. Two FileIOs created with
`FileIOBuilder::new("file")` and `FileIOBuilder::new("fs")`, then attached to
the same `Arc<dyn FileBlockCache>` through the public `with_file_block_cache`
API, therefore use different keys for the same physical file. After the first
FileIO caches `snapshot-1`, a successful overwrite through the second
invalidates only its own namespace; a fresh read through the first still
returns the old metadata.
I reproduced this with an equal-length overwrite: the backing file contains
`new metadata`, but the fresh cached read returns `old metadata` on ee631f97.
The identical test passes on the exact d57e85a9 base, whose block-cache keys
use the canonical OpenDAL identity. This is a sequential invalidation failure,
not an in-flight-reader race. The existing overwrite/delete-recreate tests
establish that FileIO mutations must invalidate cached metadata.
Please derive this part of the namespace from the canonical storage identity
(including supported aliases), while retaining endpoint/account isolation, and
add a cross-FileIO shared-cache regression test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
