yihua opened a new issue, #19736:
URL: https://github.com/apache/hudi/issues/19736
### Problem
`HoodieStorageUtils` offers a one-argument factory that reads as "build
storage from this configuration":
public static HoodieStorage getStorage(StorageConfiguration<?> conf) {
return getStorage(DEFAULT_URI, conf);
}
`DEFAULT_URI` is `"file:///"`, and `FileSystem.get(URI, conf)` selects the
implementation from the URI scheme, so the result is always bound to the local
filesystem no matter what the configuration carries. `HoodieHadoopStorage`
binds that filesystem once in its constructor and `open` / `openSeekable` /
`listDirectEntries` all go through the bound handle, so any non-local path
fails:
java.lang.IllegalArgumentException: Wrong FS: s3a://<bucket>/<path>,
expected: file:///
at org.apache.hadoop.fs.FileSystem.checkPath(FileSystem.java:807)
The signature is exactly what a caller shipping a serializable
`StorageConfiguration` to an executor reaches for, and it is not what the
method does. That mismatch has now produced two separate defects.
### Occurrence 1: rollback (fixed separately)
`RollbackHelperV1.addMissingLogFilesAndGetRollbackStats` used the overload
to list a partition whose full path it had just computed. Fixed by #19735.
### Occurrence 2: blob reads (open)
`BatchedBlobReader` builds executor-side storage with the same overload at
`BatchedBlobReader.scala:695` and `:754`, then uses it at `:393`
(`storage.open`) and `:432` (`storage.openSeekable`) against `new
StoragePath(rowInfo.filePath)`, where `filePath` is
`blob.reference.external_path` read straight out of row data.
Any out-of-line blob reference on `s3a` or `gs` therefore fails with the
same `Wrong FS` error. Blob references are absolute paths carried in row data,
so the filesystem a partition must read is not knowable before its rows arrive,
and one partition can legitimately reference more than one filesystem.
### Proposal
Remove the footgun rather than repairing call sites one defect at a time.
1. Give `BatchedBlobReader` the `StorageConfiguration` and have it resolve
`HoodieStorage` per referenced filesystem, cached by scheme and authority,
closed at task completion.
2. Delete `getStorage(StorageConfiguration<?>)` and `DEFAULT_URI` from
`HoodieStorageUtils` and move both to `HoodieTestUtils`, whose callers
genuinely do want a local filesystem for a table they created under a temp
directory.
After that every `HoodieStorageUtils` factory takes a path, so a caller
cannot accidentally obtain a local-filesystem handle for a remote table.
--
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]
