[refactor](catalog) drop hadoop source imports from fe-core and fe-common - #66324
Open
morningman wants to merge 6 commits into
Open
[refactor](catalog) drop hadoop source imports from fe-core and fe-common#66324morningman wants to merge 6 commits into
morningman wants to merge 6 commits into
Conversation
…ource imports First step of decoupling fe-core and fe-common source from hadoop. Only source imports are in scope here; the pom dependencies stay as they are. - fe-common: delete CatalogConfigFileUtils. It has no caller anywhere in the tree; fe-filesystem-hdfs-base and fe-filesystem-hdfs already carry a faithful port as HdfsConfigFileLoader. This leaves fe-common free of any hadoop import. - LocationPath: delete getTempWritePath (no caller; fe-connector-hive reimplemented it in HiveWritePlanProvider) and its now-unused UUID import. - HdfsStorageVault: inline the literal of CommonConfigurationKeysPublic.HADOOP_SECURITY_AUTHENTICATION, verified against hadoop-common 3.4.2, and drop the import. It was a plain config-key string. - StageUtil: inline org.apache.hadoop.fs.Path.SEPARATOR as "/". The remaining GlobExpander/GlobFilter use is left for a later step. - RangerHiveAuditHandler: document the hadoop Configuration import as a sanctioned exception rather than removing it. The type is imposed by Ranger (RangerDefaultAuditHandler's constructor takes org.apache.hadoop.conf .Configuration, and RangerPluginConfig extends RangerConfiguration extends Configuration), and ranger-plugins-common pulls hadoop-client-api/runtime onto the classpath regardless, so narrowing the parameter would remove no dependency. Verified: full reactor `package -DskipTests` BUILD SUCCESS; `checkstyle:check` on fe-core BUILD SUCCESS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
…esystem-obs OBSFileSystem belongs with the OBS filesystem plugin, not with fe-core. The only reference to org.apache.hadoop.fs.obs.OBSFileSystem in the FE tree is the initialize=false Class.forName probe in ObsFileSystemProperties, which picks between fs.obs.impl=OBSFileSystem and the fs.obs.impl=S3AFileSystem fallback. That probe resolves against fe-filesystem-obs's own classloader, so the jar belongs in that plugin for the probe to report the truth. Consumers that actually instantiate the class already carry their own copy (fe-connector-paimon, be-java-extensions/hadoop-deps); fe-connector-iceberg needs none because it normalizes obs:// to s3 and goes through S3AFileSystem. The huawei-obs-sdk repository declaration moves along with the dependency: the -hw-46 artifact is not published to Maven Central, and it was the only artifact fe-core resolved from there. Two things deliberately left in place, with their comments corrected: - fe-core keeps commons-lang (2.x, runtime). OBSFileSystem calls org.apache.commons.lang.StringUtils and declares no commons-lang of its own, and neither fe-filesystem-obs nor fe-connector-paimon declares one either, so their child-first loaders still fall through to fe/lib for it. - fe-filesystem-obs excludes the transitive esdk-obs-java-optimised. The -hw-46 fat jar already inlines exactly those 762 com/obs/* classes, so it was a third redundant copy alongside the declared esdk-obs-java-bundle 3.21.11 that ObsObjStorage compiles against. The fat jar's inlined copy remains and cannot be excluded; DirectoryPluginRuntimeManager.collectJars sorts lib/ jars lexicographically, so esdk-obs-java-bundle-* still precedes hadoop-huaweicloud-* and the declared version wins. This is recorded in the pom comment so a future rename or version bump re-checks the ordering. Verified: full reactor `package -DskipTests` BUILD SUCCESS; the OBS plugin zip now carries lib/hadoop-huaweicloud-3.1.1-hw-46.jar without esdk-obs-java-optimised, and fe-core/target no longer ships the jar. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
… a fe-core type P2 of removing hadoop from fe-core source. FileSplitter took an org.apache.hadoop.fs.BlockLocation[] and read exactly three accessors from it: getOffset(), getLength() and getHosts(). Introduces FileBlockLocation carrying that subset so split planning owns its own type. Kept the parameter rather than dropping it. Nothing in the tree produces a BlockLocation[] (getFileBlockLocations has no caller) and the only production caller, TVFScanNode, always passes null, so the block-locality capability is in fact dead. Removing it, however, would also delete getBlockIndex, the multi-block clamping loop and six tests that assert per-block host preservation — a capability removal well beyond taking out an import. That is worth doing on its own terms, not smuggled into this change. FileBlockLocation normalizes null hosts to an empty array. This mirrors hadoop, verified by running it: new BlockLocation(null, null, 0, len).getHosts() returns String[0], never null. The null-block-locations path builds a synthetic whole-file block with null hosts, so the normalization is on the shipping path. Adds testNullBlockLocationsSplitLikeOneWholeFileBlock. The null path is what TVFScanNode actually exercises and it had no coverage at all: the existing tests either pass a non-null array or use a zero length that returns before the synthetic block is built. The test asserts null degrades to a single block spanning the file, pinning the split sizes a TVF scan sends to the BE. Verified: 10/10 FileSplitterTest green; mutation check (synthetic block length -> length/2) fails ONLY the new test with IndexOutOfBounds while the other nine stay green, confirming both that the new test guards the synthetic block and that the pre-existing tests never covered this path; full reactor `package -DskipTests` BUILD SUCCESS; `checkstyle:check` on fe-core BUILD SUCCESS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
…roup infos P3 of removing hadoop from fe-core source, following what Trino did rather than porting hadoop's behaviour across. Trino replaced hadoop Path with io.trino.filesystem.Location, which parses a location into scheme/userInfo/host/port/path and otherwise leaves the string alone: its toString() returns the input verbatim, and its javadoc states it "does not follow the format rules of a URI or URL". TestLocation pins that deliberately -- "scheme://host///path//" keeps the path "//path//", "file://///" keeps "//", and parentDirectory of ".../path///name" is ".../path//". The reason shows up in S3Location.key(), which returns location.path() directly: object stores key on the exact byte string, so a//b and a/b name different objects and collapsing them rewrites the key. Doris already has the same abstraction -- org.apache.doris.filesystem.Location in fe-filesystem-api, storing its uri verbatim with pure string accessors, already used by ~20 fe-filesystem files and ~10 fe-core files, one of which (DefaultConnectorContext) is a caller changed here. No new type is needed. - LocationPath.toStorageLocation() now returns that Location instead of a hadoop Path. Every caller already followed it with toString(), so no call site changes; the value returned is the normalized location verbatim. - LocationPath.getPath() is deleted. Its only production use built "scheme://authority" for TFileRangeDesc.fsName, which LocationPath already computes into fsIdentifier at construction, so FileQueryScanNode now reads that field. - FileGroupInfo and NereidsFileGroupInfo built the same string through new Path(path).toUri(); both now go through LocationPath.of(...) .getFsIdentifier(). - FederationBackendPolicyTest compared Path objects; it now compares the normalized location string. BEHAVIOUR CHANGE, intentional and signed off. The path sent to the BE is no longer rewritten on the way out. Locations containing repeated slashes, a trailing slash, or . / .. segments now reach the BE as written, and a location with no authority keeps its "scheme://" form instead of being shortened to "scheme:/". validateAndNormalizeUri only rewrites schemes (cos:// -> s3://, path-style handling) and LocationPath.of does not normalize at all, so the input domain genuinely can contain those forms. fsName for an authority-less location becomes "hdfs://" rather than the literal "hdfs://null" hadoop produced. Verified: full reactor `package -DskipTests` BUILD SUCCESS; LocationPathTest + FederationBackendPolicyTest + FileSplitterTest run 36 tests, 0 failures (the 1 skipped is the pre-existing @disabled testGSLocationConvert from d83289d); `checkstyle:check` on fe-core BUILD SUCCESS. Note for future verification runs: `mvn package` followed by `mvn test` in the same invocation silently skips surefire -- the build cache restores fe-core and logs "Skipping plugin execution (cached): surefire:test" while still reporting BUILD SUCCESS. Pass -Dmaven.build.cache.enabled=false when running tests after a package in the same session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
…e-core
StorageAdapter was the last fe-core holder of org.apache.hadoop.conf.Configuration.
It built one lazily and exposed it as getHadoopStorageConfig(), but that method has
no caller anywhere in the tree: the only consumer is the adapter's own
azureBackendConfigProperties(), and only when the binding is AZURE with
azure.auth_type=OAuth2. Every other arm of buildHadoopStorageConfig() -- BROKER/HTTP,
LOCAL, the S3 credential-provider chain, the GCS anonymous skip and the JFS auth
alignment -- is therefore unreachable, as is applyAzureAccountKeysFromConfig(), whose
guard (AZURE && !isAzureOauth2()) can never hold on the one path that reaches it.
The dump itself is load-bearing and stays byte-for-byte identical. Microsoft Fabric
OneLake locations (abfs[s]://...dfs.fabric.microsoft.com) are routed to FILE_HDFS by
LocationPath.getTFileTypeForBE(), and BE's hdfs_builder feeds every entry of the map
into its JNI hadoop builder -- that is how the fs.azure.account.oauth2.* keys reach
the ABFS connector. The map also carries whatever core-site.xml is reachable on the FE
classpath, which start_fe.sh populates from ${DORIS_HOME}/conf and ${HADOOP_CONF_DIR},
so it is a real operator-facing channel rather than just hadoop's built-in defaults.
So the logic moves to its rightful owner instead of being dropped: fe-filesystem-azure
already returned toHadoopConfigurationMap() for OAuth2, and now merges the hadoop
defaults, the user fs.* passthrough and the cache-disable normalization in the same
order the fe-core facade used. The module bundles hadoop-common the way
fe-filesystem-hdfs, -jfs and -oss-hdfs already do, so it does not depend on the host
shipping hadoop; the logging jars those plugins exclude are excluded here too.
That bundled copy is a fallback rather than the active one, which is also why it
introduces no classloader split-brain: FileSystemPluginManager lists
"org.apache.hadoop." among the filesystem family's parent-first prefixes, so
ChildFirstClassLoader keeps resolving hadoop from the host while the host has it and
only falls through to the plugin's own jars when it does not. Worth remembering if the
host ever stops shipping hadoop: Configuration resolves core-default.xml and
core-site.xml through the context classloader, not through its own, so that scenario
needs the TCCL pinned as well -- bundling the jars alone would not be enough.
fe-core keeps routing AZURE around the S3-family backend-map alignment exactly as
before; the arm now just returns the provider map for both auth types.
The OAuth2 map had no test coverage at all, so its contract is pinned where it now
lives, including the hadoop-resolved keys that a plain key-value map would silently
lack. The existing SharedKey assertion gains a size guard so the reverse regression --
applying the dump to both auth types -- also fails loudly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
StageUtil analyzes the glob of COPY INTO ... FROM @stage('<glob>') into the object-store prefixes to list. It reached into hadoop for two things: GlobExpander.expand for brace groups, and GlobFilter.hasPattern to tell a literal path component from the one where wildcard matching starts. Both are ported to a new package-private GlobPatterns, which leaves fe-core with a single hadoop import -- the Ranger audit handler that is meant to keep it. GlobFilter is easy to misread as a wildcard predicate. It is also a validator: its constructor rejects a[b, a{b, a trailing backslash and [z-a], and analyzeGlob turns that into a user-facing DdlException. Porting only the predicate would have accepted those globs and quietly listed some unintended prefix instead of failing the statement, so the validation is ported with it, including the re2j compile hadoop's GlobPattern performs for exactly that purpose. re2j is already a declared fe-core dependency and is the engine hadoop uses, so this needs no new dependency and keeps the error text identical. Note the existing GlobRegexUtil is a different dialect -- it escapes braces as literals and knows nothing about brace groups -- so it could not be reused here. Equivalence was established differentially against the hadoop classes while they are still on the test classpath: every example from their javadoc, realistic stage globs, malformed inputs, and 20k fuzzed strings over the glob metacharacter alphabet, comparing results, exception types and exception messages. That run is what caught the DOTALL flag being missing -- it never affects matching here, since the pattern is only compiled for validation, but re2j echoes the flags into the syntax-error text that reaches the user. The differential harness is deliberately not committed, as depending on hadoop is the thing being removed; StageGlobTest pins the same behaviour with literals instead, and analyzeGlob goes from no coverage at all to covering prefix derivation, brace expansion and each rejection. One of those tests documents a defect rather than a guarantee: x/{y,z} yields only x/y. The cause is StageUtil.splitByComma dropping a trailing alternative that is a single character, which predates this change and is unrelated to the ported code. It is pinned so this refactor is provably behaviour-preserving; fixing it changes which files a COPY INTO reads and belongs in its own commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RaupPraax1LQqUDu4Zq1aq
morningman
requested review from
924060929,
CalvinKirs,
englefly,
gavinchou,
morrySnow and
starocean999
as code owners
July 31, 2026 08:29
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29081 ms |
Contributor
TPC-DS: Total hot run time: 169431 ms |
Contributor
ClickBench: Total hot run time: 23.84 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
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
2 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.
What problem does this PR solve?
Issue Number: #65185
Related PR: #66004
Problem Summary:
Now that catalogs go through the connector/filesystem SPIs, fe-core and fe-common no
longer have a reason to compile against hadoop, but a handful of source imports were
still left over from the pre-SPI code. This removes them.
Scope is deliberately narrow: source imports only. The hadoop pom dependencies stay
exactly as they are, because they remain reachable at runtime —
ranger-plugins-commoninherits from
org.apache.hadoop.conf.Configuration,hive-execsupplies the UDF baseclass that
CREATE FUNCTIONresolves, andfe-kerberosusesUserGroupInformation.Dropping the jars is a separate, larger question and is not attempted here.
After this PR,
grep -rn "^import org.apache.hadoop" fe/fe-core/src fe/fe-common/srcreturns exactly one line:
RangerHiveAuditHandler, which is a sanctioned exception. Thetype is imposed by Ranger's own API, and
ranger-plugins-commonputs hadoop on theclasspath regardless, so removing that import would remove no dependency. It is
documented as such in the code rather than worked around.
The six commits are independent steps and are easiest to review one at a time:
4314ad2CatalogConfigFileUtils(no caller;fe-filesystem-hdfs*already carries the port) andLocationPath.getTempWritePath(no caller); inline two hadoop constants that were plain strings; document the Ranger exception29649e2hadoop-huaweicloudmoves from fe-core to thefe-filesystem-obsplugin, together with thehuawei-obs-sdkrepository declaration. The only FE reference is theClass.forNameprobe inObsFileSystemProperties, which must resolve against that plugin's classloader to report the truth90339cdFileSplittertakes a new fe-coreFileBlockLocationinstead of hadoop'sBlockLocation. OnlygetOffset/getLength/getHostswere ever readc55800aLocationPath.toStorageLocation()returns the existingorg.apache.doris.filesystem.Locationinstead of a hadoopPath;getPath()is deleted in favour of thefsIdentifierthe class already computes. See the behaviour-change note below592e8aaStorageAdapterintofe-filesystem-azure, which is where the legacyAzurePropertiesowned it. This deletes ~120 lines of fe-core code that turned out to be unreachable464efa4StageUtil'sGlobExpander/GlobFilteruse is ported to a package-privateGlobPatterns, compiled with re2j — the same engine hadoop'sGlobPatternuses, and already a declared fe-core dependencyTwo notes that are easy to miss on review:
hadoop.fs.GlobFilterreads like a wildcard predicate but its constructor is also avalidator: it rejects
a[b,a{b, a trailing backslash and[z-a], andanalyzeGlobsurfaces that as a
DdlException. Porting only the predicate would have accepted thoseglobs and quietly listed an unintended object-store prefix instead of failing the
statement, so the validation is ported with it.
StorageAdapter.getHadoopStorageConfig()had no caller anywhere in the tree; the onlyconsumer was the adapter's own Azure OAuth2 arm. That map is still load-bearing (BE
routes Microsoft Fabric OneLake locations to
FILE_HDFS, andhdfs_builderfeeds everyentry into its JNI hadoop builder), so it is preserved key-for-key rather than dropped —
it just lives in the Azure plugin now.
Release note
None
Check List (For Author)
Test
Unit tests were added where the code being moved had no coverage:
StageGlobTest(glob prefix derivation, brace expansion and each rejection —analyzeGlobhad none at all), the Azure OAuth2 backend map in
AzureFileSystemPropertiesTest(alsonone), and
FileSplitterTest.testNullBlockLocationsSplitLikeOneWholeFileBlock(the nullpath is the one
TVFScanNodeactually takes and it was uncovered).Equivalence for the glob port was established differentially against the hadoop classes
while they are still on the test classpath — their javadoc examples, realistic stage
globs, malformed inputs and 20k fuzzed strings, comparing results, exception types and
exception messages. That run caught a missing
Pattern.DOTALLflag, which never affectsmatching here but does change the syntax-error text the user sees. The harness itself is
not committed, since depending on hadoop is the thing being removed; the committed test
pins the same behaviour with literals.
Each behaviour-preserving claim was mutation-checked (removing the glob validation,
making
{stop counting as a wildcard, reverting the Azure arm to the plain map,applying the hadoop dump to both auth types, halving the synthetic block length) — every
mutation fails exactly the test that guards it.
Verified per commit: full reactor
package -DskipTestswith the build cache disabled,BUILD SUCCESS;
checkstyle:checkon each changed module, 0 violations.No regression tests: the paths involved need a cloud-mode cluster (COPY INTO) or a live
Azure/OneLake account, neither of which is reachable here. Azure OAuth2 has no existing
regression coverage either.
Behavior changed:
In
c55800a, the path Doris sends to the BE is no longer rewritten on the way out.Routing it through hadoop's
Pathused to collapse repeated slashes, drop a trailingslash, resolve
.and..segments, and turn an authority-less location fromscheme://xintoscheme:/x. None of that is wanted for object storage, wherea//band
a/bname different objects and collapsing them rewrites the key. This follows whatTrino did when it replaced hadoop
Pathwithio.trino.filesystem.Location, whosejavadoc states it "does not follow the format rules of a URI or URL" and whose tests pin
slash preservation deliberately. Doris already had the equivalent type
(
org.apache.doris.filesystem.Location), so this reuses it rather than adding one.Relatedly,
fsNamefor an authority-less location is nowhdfs://instead of theliteral string
hdfs://nullthat hadoop produced.Two smaller consequences worth flagging: the Azure OAuth2 backend map is now immutable
(it was a mutable
HashMap; every reachable consumer only reads it), andfe-filesystem-azurenow bundles hadoop-common the wayfe-filesystem-hdfs,-jfsand-oss-hdfsalready do, which grows that plugin zip.Does this need documentation?