rangareddy opened a new pull request, #19470:
URL: https://github.com/apache/hudi/pull/19470
### Describe the issue this Pull Request addresses
Closes #15331 (HUDI-4602).
A MOR `_rt` query through Presto fails before reading anything:
```
java.lang.UnsupportedOperationException: Not implemented by the
PrestoS3FileSystem FileSystem implementation
at org.apache.hadoop.fs.FileSystem.getScheme(FileSystem.java:219)
at
org.apache.hadoop.fs.HadoopExtendedFileSystem.getScheme(HadoopExtendedFileSystem.java:71)
at org.apache.hudi.common.fs.FSUtils.isGCSFileSystem(FSUtils.java:592)
at
org.apache.hudi.common.table.log.HoodieLogFileReader.getFSDataInputStream(HoodieLogFileReader.java:119)
...
at
org.apache.hudi.hadoop.realtime.HoodieParquetRealtimeInputFormat.getRecordReader
```
`FileSystem#getScheme()` is **optional** in Hadoop — `FileSystem`'s own
implementation throws
`UnsupportedOperationException` — and proxy implementations such as
`PrestoS3FileSystem` do not override
it. Hudi called it unguarded, so a filesystem that declines to implement an
optional API took down an
unrelated read. The issue title asks for `getScheme` to be implemented in
`PrestoS3FileSystem`, which is
not a change Hudi can make; what Hudi can fix is the dependency on it, and
that is the actual defect.
Still live on current master, just relocated from `FSUtils` to
`HadoopFSUtils.isGCSFileSystem` (line 280) reached from
`HadoopFSUtils.getFSDataInputStream` (line 223).
There is precedent for the conclusion: #793 ("Allow HoodieWrapperFileSystem
to wrap other proxy
file-system implementations with no getScheme implementation") already
stopped `HoodieWrapperFileSystem`
calling `getScheme()` on the filesystem it wraps, deriving the scheme from
the URI instead.
### Summary and Changelog
- Adds `HadoopFSUtils#getScheme(FileSystem)`: returns `fs.getScheme()`,
falling back to
`fs.getUri().getScheme()` when it is unimplemented. `getUri()` is
abstract, so every implementation
supplies it, and its scheme is what `getScheme()` returns wherever both
are present. Behaviour is
therefore unchanged for every filesystem that implements `getScheme()`;
only the throwing case is new.
- Routes the seven unguarded call sites through it:
| call site | reached from |
| --- | --- |
| `HadoopFSUtils#isGCSFileSystem` | **the reported crash** —
`getFSDataInputStream`, i.e. every log-file open |
| `HadoopFSUtils#isCHDFileSystem` | same method, same read path |
| `HadoopFSUtils#registerFileSystem` | |
| `HoodieWrapperFileSystem#convertToHoodiePath` | |
| `HoodieRetryWrapperFileSystem#getScheme` | delegates to the wrapped
filesystem |
| `WriteMarkersFactory` HDFS gate | marker-type selection |
| `HoodieHadoopStorage#getScheme` | the `HoodieStorage#getScheme` entry
point — 7 callers, including `HoodieLogFileReader`'s `isWriteTransactional`
check, `HoodieLogFormatWriter`, `RollbackHelperV1` and
`FileSystemBasedLockProvider` |
- `isGCSFileSystem`'s comparison is flipped to put the constant first,
matching its neighbour
`isCHDFileSystem`, so a filesystem whose URI carries no scheme returns
false instead of throwing
`NullPointerException`.
### Verification
Reproduced first, on the real code path, with no fabrication:
`FilterFileSystem` is a Hadoop-provided
class with exactly the reported shape — it leaves `getScheme()` to the
throwing base implementation while
overriding `getUri()`. Opening a file through
`HadoopFSUtils.getFSDataInputStream` with one wrapped around
a local filesystem fails on master with the same exception and the same Hudi
frames as the report:
```
java.lang.UnsupportedOperationException: Not implemented by the
FilterFileSystem FileSystem implementation
at
org.apache.hudi.hadoop.fs.HadoopFSUtils.isGCSFileSystem(HadoopFSUtils.java:280)
at
org.apache.hudi.hadoop.fs.HadoopFSUtils.getFSDataInputStream(HadoopFSUtils.java:223)
```
Two tests in `TestHadoopFSUtils`, both of which pin the premise with
`assertThrows(UnsupportedOperationException.class, fs::getScheme)` before
asserting anything:
- `testGetFSDataInputStreamWhenGetSchemeIsUnimplemented` — the read
completes and returns the file's bytes.
- `testGetSchemeFallsBackToTheUriWhenUnimplemented` — the helper returns
`file` both for an
implementation that overrides `getScheme()` and for one that does not.
Both are red with the guard removed and green with it, so neither passes
vacuously.
Regression runs:
| suite | result |
| --- | --- |
| `hudi-hadoop-common` full unit suite | 1084 tests, 1 failure —
`TestHoodieActiveTimeline#testParseDateFromInstantTime`, which fails
identically on unmodified master (a 5.5-hour delta, i.e. the machine's local
timezone) |
| `TestWriteMarkersFactory`, `TestMarkerBasedRollbackUtils` | 10 pass |
| `TestFlinkWriteClients` (covers `testMarkerType`) | 20 pass |
| `TestHoodieLogFormatWriter`, `TestFSUtilsWithRetryWrapperEnable`,
`TestInLineFileSystem`, `TestStorageSchemes`, `TestHadoopFSUtils` | 78 pass |
`checkstyle:check` and `apache-rat:check` clean.
### Impact
Restores MOR `_rt` reads on any filesystem that does not implement
`getScheme()` — Presto's
`PrestoS3FileSystem` as reported, and any other proxy or vendor
implementation with the same gap.
No behaviour change for filesystems that do implement it, since the helper
calls it first.
Not addressed here: whether `PrestoS3FileSystem` should also implement
`getScheme()`. It should, but that
is a change in Presto, and Hudi should not fall over on an optional API
either way.
### Risk Level
low — one new helper plus seven one-line call-site changes, no change in
resolved scheme for any
filesystem that implements `getScheme()`, and the previously-throwing paths
are covered by tests that were
shown to fail without the change.
### Documentation Update
none — no new config and no user-facing behaviour change beyond the reads
that used to fail.
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
- [x] CI passes on my PR
--
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]
