924060929 commented on code in PR #66315:
URL: https://github.com/apache/doris/pull/66315#discussion_r3754818809
##########
fe/fe-extension-loader/src/main/java/org/apache/doris/extension/loader/DirectoryPluginRuntimeManager.java:
##########
@@ -231,9 +239,27 @@ private PluginHandle<F> loadFromPluginDir(Path pluginDir,
ClassLoader parent, Cl
List<Path> libJars = new ArrayList<>();
resolveJars(normalizedDir, rootJars, libJars);
- List<Path> allJars = new ArrayList<>(rootJars.size() + libJars.size());
- allJars.addAll(rootJars);
- allJars.addAll(libJars);
+ // hadoop-deps carries the Doris-patched
org.apache.hadoop.fs.FileSystem (credential-aware
+ // doris.fs.cache.key, see FsCacheKeys). A plugin that bundles its own
hadoop-common is
+ // loaded child-first for org.apache.hadoop.* -- the connector
families deliberately keep
+ // their own hadoop copy -- so the patched class only wins if its jar
precedes hadoop-common
+ // on the plugin's own classpath. Neither directory position nor the
lexicographic order of
+ // collectJars() delivers that ("hadoop-common-x.y.z.jar" sorts BEFORE
"hadoop-deps-*.jar"),
+ // so the promotion is explicit here. Mirrors start_fe.sh and the BE,
which both prepend the
+ // same jar. Deliberately excluded from the discovery set below: it
registers no SPI service,
+ // and its mere presence at the plugin root would otherwise make
rootJars non-empty and
+ // narrow discovery to a jar that carries no factory.
+ List<Path> patchedHadoopJars = new ArrayList<>();
+ extractPatchedHadoopJars(rootJars, patchedHadoopJars);
+ extractPatchedHadoopJars(libJars, patchedHadoopJars);
+
+ List<Path> discoveryJars = new ArrayList<>(rootJars.size() +
libJars.size());
+ discoveryJars.addAll(rootJars);
+ discoveryJars.addAll(libJars);
+
+ List<Path> allJars = new ArrayList<>(patchedHadoopJars.size() +
discoveryJars.size());
+ allJars.addAll(patchedHadoopJars);
Review Comment:
[P1][recheck at `e9a515c0f3f059b8b2c127e1e4c85ac90c2484e9`] This is still
reachable on the current head. `loadFromPluginDir` only promotes `hadoop-deps`
when the plugin already contains it (lines 252-262); it never rejects a
child-first plugin that bundles vanilla `hadoop-common` without the patch. The
production connector loader accepts any plugin with the current major, and
`connector.plugin.api.version` is still `5.0`. In fact, the POM's own contract
says the connector API spans `fe-filesystem-api` and changing it requires
bumping this version, but this PR adds
`FileSystemProperties.fsCacheFingerprint()`/`FsCacheKeys` without a bump.
Therefore an API-5.x plugin built before this change is admitted, ignores
`doris.fs.cache.key.*`, and receives current storage maps after the blanket
`disable.cache` defaults were removed; two catalogs with the same authority and
different credentials can again reuse the first plugin-side FileSystem. Please
either bump/gate the plugin contract (or
add an explicit patched-Hadoop capability check) or retain the disable-cache
fallback for unpatched plugins, with a negative test for a 5.x plugin
containing `hadoop-common` but no `hadoop-deps`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
