fix(fs): stop depending on the optional FileSystem#getScheme() - #19470
Merged
voonhous merged 3 commits intoAug 3, 2026
Conversation
FileSystem#getScheme() is optional in Hadoop: the base implementation throws UnsupportedOperationException, and proxy implementations such as Presto's PrestoS3FileSystem do not override it. Hudi called it unguarded on filesystems it did not implement, so opening a log file on such a filesystem failed with "Not implemented by the PrestoS3FileSystem FileSystem implementation" instead of reading anything (HUDI-4602). Adds HadoopFSUtils#getScheme(FileSystem), which returns fs.getScheme() and falls 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. This is the same conclusion as apache#793, which stopped HoodieWrapperFileSystem calling getScheme() on the filesystem it wraps. Routes the seven unguarded call sites through it: isGCSFileSystem and isCHDFileSystem (the reported read path), registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath, HoodieRetryWrapperFileSystem#getScheme, WriteMarkersFactory's HDFS gate, and HoodieHadoopStorage#getScheme, which is what the seven HoodieStorage#getScheme callers reach. isGCSFileSystem's comparison is also flipped to put the constant first, matching isCHDFileSystem, so a filesystem whose URI carries no scheme returns false rather than throwing NullPointerException.
hudi-agent
reviewed
Aug 3, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this! This PR stops Hudi from depending on Hadoop's optional FileSystem#getScheme() by adding HadoopFSUtils.getScheme(FileSystem), which falls back to getUri().getScheme() when getScheme() is unimplemented, fixing the _rt MOR read failure on proxy filesystems like PrestoS3FileSystem (HUDI-4602). I traced the helper, its null-return handling, and all callers, and the change looks correct and behavior-preserving on the normal path. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. One minor nit on the test assertion message; the production changes and overall structure look clean.
cc @yihua
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19470 +/- ##
============================================
- Coverage 77.00% 76.98% -0.02%
+ Complexity 33873 33865 -8
============================================
Files 2576 2575 -1
Lines 143463 143386 -77
Branches 17589 17642 +53
============================================
- Hits 110476 110390 -86
- Misses 24716 24732 +16
+ Partials 8271 8264 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Review nit: the assertion messages did not make clear what had gone wrong. Each now names the filesystem and the branch of the helper it pins - LocalFileSystem overriding getScheme() so the helper returns what it reports, FilterFileSystem not overriding it so the helper falls back to getUri().getScheme().
voonhous
reviewed
Aug 3, 2026
voonhous
left a comment
voonhous
left a comment
Member
There was a problem hiding this comment.
Reviewed the change against the history of these files rather than just the diff. The production change is sound -- the try branch is byte-identical to the old code at all seven call sites, so the only behavioural deltas are the UOE fallback and the null-safe equals flip.
Things I checked and am explicitly not raising, so nobody re-derives them:
- No accidental revert. #793 (
83dab21ae131) is intact --HoodieWrapperFileSystem#getScheme()still returnsuri.getScheme()and the instanceconvertToHoodiePath/convertToDefaultPathstill route through it. HUDI-5286's override is modified, not removed. Worth adding to the PR description: #793 fixed only the instance methods and left the staticconvertToHoodiePath(StoragePath, Configuration)callinggetFs(...).getScheme()-- that is the line this PR finally fixes, six years later. - No prior attempt was reverted.
git log -i --grep=getSchemereturns only #793 and this PR;--grep=PrestoS3and--grep="Not implemented by the"return nothing. - The fix is complete at the
FileSystemlevel. The only rawfs.getScheme()left in main code is inside the new helper andFileSystem.getLocal(getConf()).getScheme()(HoodieWrapperFileSystem.java:991, andLocalFileSystemimplements it). EveryHoodieStorage#getScheme()consumer routes through the now-guardedHoodieHadoopStorage:113. - It reaches the reporter.
packaging/hudi-presto-bundle/pom.xml:69andpackaging/hudi-hadoop-mr-bundle/pom.xml:69both shadehudi-hadoop-common. - Case sensitivity is a non-issue.
URI#getScheme()preserves case, butFileSystem.getFileSystemClassdoes a case-sensitive lookup, soS3A://bucket/xdies in Hadoop withUnsupportedFileSystemExceptionbefore Hudi ever sees it. - Returning
nullfrom the Hadoop@Overrides is safe. The only Hadoop internal consuming an FS's owngetScheme()isloadFileSystems(), and there is noMETA-INF/services/org.apache.hadoop.fs.FileSystemanywhere in the repo. - The
equalsflip is an improvement, not a revert --e93c6a569310wrote it the throwing way round;isCHDFileSystemhas had the constant first sinceeaa2f8ed3bb3.
One thing worth a sentence in the PR body: for a scheme outside the StorageSchemes enum the failure moves rather than disappears -- HoodieLogFileReader.java:258 and RollbackHelperV1.java:190 will throw IllegalArgumentException: Unsupported scheme instead of the UOE. Not a regression (both cases failed before), and the reported s3 is in the enum, so the fix works end to end.
The substance of my review is inline: two coverage gaps I'd like closed before merge, one error-reporting fix, one perf nit, and four optional cleanliness nits.
hudi-agent
reviewed
Aug 3, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this. The PR stops Hudi from calling the optional FileSystem getScheme unguarded by adding a helper that falls back to the URI scheme, fixing the Presto MOR read crash in HUDI-4602. I traced the helper and the reported read path and found no new correctness issue this round. The substantive open items were already raised in prior rounds. A Hudi committer or PMC member can take it from here for a final review.
…his reroutes
Review feedback, all of it well founded.
The fallback no longer returns null. InLineFileSystem is the counter-example in this
module: getScheme() is "inlinefs" while getUri() is URI.create("inlinefs"), which has no
colon and so no scheme, so the two are not interchangeable and the javadoc claim that
they agree was simply wrong. A null surfaced far from the cause as "does not support
scheme null" or "Unsupported scheme :null" with the UnsupportedOperationException
discarded; it now throws with that exception chained. HoodieException rather than
HoodieIOException, since the latter only accepts an IOException cause.
HoodieHadoopStorage memoizes the scheme. On a filesystem without getScheme() the
fallback costs a thrown-and-caught exception, and this is called once per log block via
StorageSchemes.isWriteTransactional and three times per immutable-file write via
needCreateTempFile. A lazy field keeps all five constructors untouched.
Test coverage for what this actually reroutes, none of which any test reached:
- registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath (the write path) and
HoodieHadoopStorage#getScheme, via a LocalFileSystem subclass whose getScheme() throws,
registered as fs.file.impl so it is reached through FileSystem.get.
- isGCSFileSystem and isCHDFileSystem, which become reachable for proxy filesystems for
the first time here and select different stream wrappers: a scheme-less filesystem
reporting gs:// now yields SchemeAwareFSDataInputStream and ofs:// yields
BoundedFsDataInputStream.
- the new unresolvable-scheme failure.
TestFSUtilsWithRetryWrapperEnable#testGetSchema has been inert since HUDI-5286 added it:
it asserted on HoodieWrapperFileSystem#getScheme, which is uri.getScheme() and never
dispatches into the retry wrapper, and FakeRemoteFileSystem overrode getScheme() to
delegate to a real LocalFileSystem so it could not throw. Dropping that override gives
the fake the PrestoS3FileSystem shape and the assertion now targets the retry wrapper,
so it guards both HUDI-5286 and this change. Verified: it fails with the pre-PR helper.
Also drops the try/catch in convertToHoodiePath that only rethrew HoodieIOException
unchanged, dead since ef70de2, and the duplicated fixture and redundant nested
close in TestHadoopFSUtils.
hudi-agent
reviewed
Aug 3, 2026
hudi-agent
left a comment
hudi-agent
left a comment
Contributor
There was a problem hiding this comment.
Thanks for working on this! The PR stops Hudi from calling the optional FileSystem#getScheme() unguarded by adding HadoopFSUtils.getScheme(fs), which falls back to the URI scheme when getScheme() is unimplemented (the PrestoS3FileSystem/HUDI-4602 case), and reroutes the affected call sites. I traced the helper, the Lazy scheme memoization in HoodieHadoopStorage (thread-safe, fs is final), and the wrapper call sites for recursion and behavior regressions, and found no new correctness issues beyond what prior rounds already covered and the author addressed. No issues flagged from this automated pass — a Hudi committer or PMC member can take it from here for a final review.
cc @yihua
Member
|
Went through the head commit and every inline thread is addressed, so I have resolved them. Thanks for the A/B on each one, that made them quick to check. Three things left, all in the PR description rather than the code:
No code changes needed from my side after that. |
Member
|
Applied these to the description myself, no action needed on your end -- the helper bullet now matches what the code does, the #793 and |
voonhous
approved these changes
Aug 3, 2026
github-actions
Bot
added
size:M
voonhous
enabled auto-merge (squash)
August 3, 2026 17:50
voonhous
pushed a commit
that referenced
this pull request
Aug 6, 2026
* fix(fs): stop depending on the optional FileSystem#getScheme() FileSystem#getScheme() is optional in Hadoop: the base implementation throws UnsupportedOperationException, and proxy implementations such as Presto's PrestoS3FileSystem do not override it. Hudi called it unguarded on filesystems it did not implement, so opening a log file on such a filesystem failed with "Not implemented by the PrestoS3FileSystem FileSystem implementation" instead of reading anything (HUDI-4602). Adds HadoopFSUtils#getScheme(FileSystem), which returns fs.getScheme() and falls 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. This is the same conclusion as #793, which stopped HoodieWrapperFileSystem calling getScheme() on the filesystem it wraps. Routes the seven unguarded call sites through it: isGCSFileSystem and isCHDFileSystem (the reported read path), registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath, HoodieRetryWrapperFileSystem#getScheme, WriteMarkersFactory's HDFS gate, and HoodieHadoopStorage#getScheme, which is what the seven HoodieStorage#getScheme callers reach. isGCSFileSystem's comparison is also flipped to put the constant first, matching isCHDFileSystem, so a filesystem whose URI carries no scheme returns false rather than throwing NullPointerException. * test(fs): say which branch of the helper each assertion covers Review nit: the assertion messages did not make clear what had gone wrong. Each now names the filesystem and the branch of the helper it pins - LocalFileSystem overriding getScheme() so the helper returns what it reports, FilterFileSystem not overriding it so the helper falls back to getUri().getScheme(). * fix(fs): fail loudly on an unresolvable scheme, and cover the sites this reroutes Review feedback, all of it well founded. The fallback no longer returns null. InLineFileSystem is the counter-example in this module: getScheme() is "inlinefs" while getUri() is URI.create("inlinefs"), which has no colon and so no scheme, so the two are not interchangeable and the javadoc claim that they agree was simply wrong. A null surfaced far from the cause as "does not support scheme null" or "Unsupported scheme :null" with the UnsupportedOperationException discarded; it now throws with that exception chained. HoodieException rather than HoodieIOException, since the latter only accepts an IOException cause. HoodieHadoopStorage memoizes the scheme. On a filesystem without getScheme() the fallback costs a thrown-and-caught exception, and this is called once per log block via StorageSchemes.isWriteTransactional and three times per immutable-file write via needCreateTempFile. A lazy field keeps all five constructors untouched. Test coverage for what this actually reroutes, none of which any test reached: - registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath (the write path) and HoodieHadoopStorage#getScheme, via a LocalFileSystem subclass whose getScheme() throws, registered as fs.file.impl so it is reached through FileSystem.get. - isGCSFileSystem and isCHDFileSystem, which become reachable for proxy filesystems for the first time here and select different stream wrappers: a scheme-less filesystem reporting gs:// now yields SchemeAwareFSDataInputStream and ofs:// yields BoundedFsDataInputStream. - the new unresolvable-scheme failure. TestFSUtilsWithRetryWrapperEnable#testGetSchema has been inert since HUDI-5286 added it: it asserted on HoodieWrapperFileSystem#getScheme, which is uri.getScheme() and never dispatches into the retry wrapper, and FakeRemoteFileSystem overrode getScheme() to delegate to a real LocalFileSystem so it could not throw. Dropping that override gives the fake the PrestoS3FileSystem shape and the assertion now targets the retry wrapper, so it guards both HUDI-5286 and this change. Verified: it fails with the pre-PR helper. Also drops the try/catch in convertToHoodiePath that only rethrew HoodieIOException unchanged, dead since ef70de2, and the duplicated fixture and redundant nested close in TestHadoopFSUtils. (cherry picked from commit 70a5a4d)
voonhous
added a commit
that referenced
this pull request
Aug 6, 2026
…opStorage The cherry-pick of #19470 (b8a09bf) brought master's org.apache.hudi.common.util.Lazy import, but the hudi-common core package reorganization (#19195) that moved Lazy there is not on this branch. Lazy is still org.apache.hudi.util.Lazy here, so hudi-hadoop-common failed to compile with "cannot find symbol: class Lazy", breaking every downstream module. Note: 14 files under hudi-trino carry the same master-only Lazy import. That module is profile-gated off by default (-Phudi-trino) so it does not break the default build, and is left alone here.
voonhous
pushed a commit
that referenced
this pull request
Aug 7, 2026
* fix(fs): stop depending on the optional FileSystem#getScheme() FileSystem#getScheme() is optional in Hadoop: the base implementation throws UnsupportedOperationException, and proxy implementations such as Presto's PrestoS3FileSystem do not override it. Hudi called it unguarded on filesystems it did not implement, so opening a log file on such a filesystem failed with "Not implemented by the PrestoS3FileSystem FileSystem implementation" instead of reading anything (HUDI-4602). Adds HadoopFSUtils#getScheme(FileSystem), which returns fs.getScheme() and falls 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. This is the same conclusion as #793, which stopped HoodieWrapperFileSystem calling getScheme() on the filesystem it wraps. Routes the seven unguarded call sites through it: isGCSFileSystem and isCHDFileSystem (the reported read path), registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath, HoodieRetryWrapperFileSystem#getScheme, WriteMarkersFactory's HDFS gate, and HoodieHadoopStorage#getScheme, which is what the seven HoodieStorage#getScheme callers reach. isGCSFileSystem's comparison is also flipped to put the constant first, matching isCHDFileSystem, so a filesystem whose URI carries no scheme returns false rather than throwing NullPointerException. * test(fs): say which branch of the helper each assertion covers Review nit: the assertion messages did not make clear what had gone wrong. Each now names the filesystem and the branch of the helper it pins - LocalFileSystem overriding getScheme() so the helper returns what it reports, FilterFileSystem not overriding it so the helper falls back to getUri().getScheme(). * fix(fs): fail loudly on an unresolvable scheme, and cover the sites this reroutes Review feedback, all of it well founded. The fallback no longer returns null. InLineFileSystem is the counter-example in this module: getScheme() is "inlinefs" while getUri() is URI.create("inlinefs"), which has no colon and so no scheme, so the two are not interchangeable and the javadoc claim that they agree was simply wrong. A null surfaced far from the cause as "does not support scheme null" or "Unsupported scheme :null" with the UnsupportedOperationException discarded; it now throws with that exception chained. HoodieException rather than HoodieIOException, since the latter only accepts an IOException cause. HoodieHadoopStorage memoizes the scheme. On a filesystem without getScheme() the fallback costs a thrown-and-caught exception, and this is called once per log block via StorageSchemes.isWriteTransactional and three times per immutable-file write via needCreateTempFile. A lazy field keeps all five constructors untouched. Test coverage for what this actually reroutes, none of which any test reached: - registerFileSystem, HoodieWrapperFileSystem#convertToHoodiePath (the write path) and HoodieHadoopStorage#getScheme, via a LocalFileSystem subclass whose getScheme() throws, registered as fs.file.impl so it is reached through FileSystem.get. - isGCSFileSystem and isCHDFileSystem, which become reachable for proxy filesystems for the first time here and select different stream wrappers: a scheme-less filesystem reporting gs:// now yields SchemeAwareFSDataInputStream and ofs:// yields BoundedFsDataInputStream. - the new unresolvable-scheme failure. TestFSUtilsWithRetryWrapperEnable#testGetSchema has been inert since HUDI-5286 added it: it asserted on HoodieWrapperFileSystem#getScheme, which is uri.getScheme() and never dispatches into the retry wrapper, and FakeRemoteFileSystem overrode getScheme() to delegate to a real LocalFileSystem so it could not throw. Dropping that override gives the fake the PrestoS3FileSystem shape and the assertion now targets the retry wrapper, so it guards both HUDI-5286 and this change. Verified: it fails with the pre-PR helper. Also drops the try/catch in convertToHoodiePath that only rethrew HoodieIOException unchanged, dead since ef70de2, and the duplicated fixture and redundant nested close in TestHadoopFSUtils. (cherry picked from commit 70a5a4d)
voonhous
added a commit
that referenced
this pull request
Aug 7, 2026
…opStorage The cherry-pick of #19470 (b8a09bf) brought master's org.apache.hudi.common.util.Lazy import, but the hudi-common core package reorganization (#19195) that moved Lazy there is not on this branch. Lazy is still org.apache.hudi.util.Lazy here, so hudi-hadoop-common failed to compile with "cannot find symbol: class Lazy", breaking every downstream module. Note: 14 files under hudi-trino carry the same master-only Lazy import. That module is profile-gated off by default (-Phudi-trino) so it does not break the default build, and is left alone here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe the issue this Pull Request addresses
Closes #15331 (HUDI-4602).
A MOR
_rtquery through Presto fails before reading anything:FileSystem#getScheme()is optional in Hadoop —FileSystem's own implementation throwsUnsupportedOperationException— and proxy implementations such asPrestoS3FileSystemdo not overrideit. Hudi called it unguarded, so a filesystem that declines to implement an optional API took down an
unrelated read. The issue title asks for
getSchemeto be implemented inPrestoS3FileSystem, which isnot 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
FSUtilstoHadoopFSUtils.isGCSFileSystem(line 280) reached fromHadoopFSUtils.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
HoodieWrapperFileSystemcalling
getScheme()on the filesystem it wraps, deriving the scheme from the URI instead. That fixcovered only the instance methods, though. The static
HoodieWrapperFileSystem#convertToHoodiePath(StoragePath, Configuration)kept callinggetFs(...).getScheme()directly, and it is one of the lines this PR fixes, six years later.Summary and Changelog
Adds
HadoopFSUtils#getScheme(FileSystem): returnsfs.getScheme(), falling back tofs.getUri().getScheme()when it is unimplemented, and failing with aHoodieExceptionthat chains theoriginal
UnsupportedOperationExceptionwhen neither source yields a scheme.getUri()is abstract, soevery implementation supplies one to fall back on, but the two are not interchangeable, which is why
getScheme()is tried first:InLineFileSystemreturns"inlinefs"fromgetScheme()while itsgetUri()isURI.create("inlinefs"), which has no colon and so carries no scheme at all. A URI with noscheme is a resolution failure rather than a value to pass on, since returning null would surface much
later as
does not support scheme nullwith the original failure discarded. Behaviour is unchanged forevery filesystem that implements
getScheme(); only the throwing case is new.Routes the seven unguarded call sites through it:
HadoopFSUtils#isGCSFileSystemgetFSDataInputStream, i.e. every log-file openHadoopFSUtils#isCHDFileSystemHadoopFSUtils#registerFileSystemHoodieWrapperFileSystem#convertToHoodiePathHoodieRetryWrapperFileSystem#getSchemeWriteMarkersFactoryHDFS gateHoodieHadoopStorage#getSchemeHoodieStorage#getSchemeentry point — 7 callers, includingHoodieLogFileReader'sisWriteTransactionalcheck,HoodieLogFormatWriter,RollbackHelperV1andFileSystemBasedLockProviderisGCSFileSystem's comparison is flipped to put the constant first, matching its neighbourisCHDFileSystem, so a filesystem whose URI carries no scheme returns false instead of throwingNullPointerException.Three further changes came out of review:
HoodieHadoopStoragememoizes the scheme. On a filesystem withoutgetScheme()the fallback costs athrown-and-caught exception, and this is called once per log block via
StorageSchemes.isWriteTransactionaland three times per immutable-file write vianeedCreateTempFile(measured at 321-1923 ns/call depending on stack depth).
fsis final, so aLazyfield resolves it oncewithout touching any of the five constructors.
HoodieWrapperFileSystem#convertToHoodiePathdrops a deadtry/catchthat only caughtHoodieIOExceptionto rethrow it unchanged, dead sinceef70de2bba7b.TestFSUtilsWithRetryWrapperEnable#testGetSchemabecomes a real guard. It has been inert sinceHUDI-5286 added it: it asserted on
HoodieWrapperFileSystem#getScheme, which isuri.getScheme()and neverdispatches into the retry wrapper, and
FakeRemoteFileSystemoverrodegetScheme()to delegate to a realLocalFileSystemso it could not throw. Dropping that override gives the fake thePrestoS3FileSystemshape and the assertion now targets the retry wrapper, so it guards both HUDI-5286 and this change --
verified by confirming it fails with the pre-PR helper.
Verification
Reproduced first, on the real code path, with no fabrication:
FilterFileSystemis a Hadoop-providedclass with exactly the reported shape — it leaves
getScheme()to the throwing base implementation whileoverriding
getUri(). Opening a file throughHadoopFSUtils.getFSDataInputStreamwith one wrapped arounda local filesystem fails on master with the same exception and the same Hudi frames as the report:
Five tests in
TestHadoopFSUtils, each run against a filesystem with the reported shape:getUri()works,
getScheme()throws.testGetFSDataInputStreamWhenGetSchemeIsUnimplemented-- the read completes and returns the file's bytes.testGetSchemeFallsBackToTheUriWhenUnimplemented-- the helper returnsfileboth for animplementation that overrides
getScheme()and for one that does not.testGetSchemeFailsLoudlyWhenNeitherSourceHasOne-- agetUri()ofURI.create("inlinefs")raises aHoodieExceptionnaming the filesystem, with the originalUnsupportedOperationExceptionchained as thecause rather than discarded.
testCallSitesWorkOnAFileSystemWithoutGetScheme-- oneLocalFileSystemsubclass whosegetScheme()throws, registered as
fs.file.impl, covering the three rerouted sites no test in the repo reached:registerFileSystem,convertToHoodiePath(the write path, viaHoodieBaseParquetWriterand friends)and
HoodieHadoopStorage#getScheme.testSchemeSpecificStreamIsSelectedWithoutGetScheme-- with a URI ofgs://bucketandofs://cluster,getFSDataInputStreamselectsSchemeAwareFSDataInputStreamandBoundedFsDataInputStream. This is theassertion that pins the helper returning what
fs.getScheme()would have, rather than merely notthrowing. Neither predicate had a test before.
TestFSUtilsWithRetryWrapperEnable#testGetSchemais retargeted atHoodieRetryWrapperFileSystem, andFakeRemoteFileSystem'sgetScheme()override is deleted so the throwing base implementation is reached.It previously asserted on
HoodieWrapperFileSystem#getScheme(), which isreturn uri.getScheme()andnever dispatches into the retry wrapper, so it could not fail for HUDI-5286 either.
All are red with the guard removed and green with it, so none passes vacuously.
Regression runs:
hudi-hadoop-commonfull unit suiteTestHoodieActiveTimeline#testParseDateFromInstantTime, which fails identically on unmodified master (a 5.5-hour delta, i.e. the machine's local timezone)TestWriteMarkersFactory,TestMarkerBasedRollbackUtilsTestFlinkWriteClients(coverstestMarkerType)TestHoodieLogFormatWriter,TestFSUtilsWithRetryWrapperEnable,TestInLineFileSystem,TestStorageSchemes,TestHadoopFSUtilscheckstyle:checkandapache-rat:checkclean.Impact
Restores MOR
_rtreads on any filesystem that does not implementgetScheme()— Presto'sPrestoS3FileSystemas 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.
For a scheme outside the
StorageSchemesenum the failure moves rather than disappears:HoodieLogFileReader:258andRollbackHelperV1:190throwIllegalArgumentException: Unsupported schemeinstead of the
UnsupportedOperationException. That is not a regression, since both cases failed before,and the reported
s3is in the enum, so the fix works end to end for the reported case.Not addressed here: whether
PrestoS3FileSystemshould also implementgetScheme(). It should, but thatis a change in Presto, and Hudi should not fall over on an optional API either way.
Risk Level
low -- one new helper plus seven call-site changes, all one-liners except
HoodieHadoopStorage, wherethe scheme is resolved once into a
Lazyfield because it is read once per log block. No change inresolved scheme for any filesystem that implements
getScheme(), and the previously-throwing paths arecovered 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