924060929 commented on code in PR #66315:
URL: https://github.com/apache/doris/pull/66315#discussion_r3754831802
##########
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());
Review Comment:
[P1] Reject or fall back for API-compatible connector plugins that do not
carry the patched Hadoop jar. On this head, lines 252-262 only move
`hadoop-deps` to the front when it is already present; a directory plugin with
vanilla `hadoop-common` and no `hadoop-deps` is still loaded child-first. The
loader accepts any connector plugin with the current major
(`connector.plugin.api.version=5.0`), while this PR removes the blanket
`fs.<scheme>.impl.disable.cache=true` defaults. Such an API-5.x plugin
therefore ignores `doris.fs.cache.key.*` and can reuse the first FileSystem for
two catalogs with the same authority but different credentials. This is also
inconsistent with the POM contract that changing `fe-filesystem-api` requires
bumping the connector API version; this PR adds
`FileSystemProperties.fsCacheFingerprint()` without doing so. Please bump/gate
the plugin contract, add an explicit patched-Hadoop capability check, or retain
the disable-cache fallback for unpatched plugins, an
d cover 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]
