morningman opened a new pull request, #66729:
URL: https://github.com/apache/doris/pull/66729
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
**Draft.** Opened for direction review. See "State of this PR" at the bottom
for what is
still missing.
Today every `be-java-extensions` module is flattened onto one classpath.
`bin/start_be.sh`
puts an 80MB `preload-extensions` fat jar plus `java-udf` in front of
everything, each
scanner is a second fat jar loaded by a child-first loader, and BE resolves
a scanner by
Java class name. Three consequences, all of which have caused production
incidents:
- **Silent version arbitration.** Between the shared layer and the scanners
there are
45,530 same-named classes, thousands of which differ in bytes (parquet
2.12 vs 2.10,
iceberg, hive UDF contracts, two log4j 1.x). Which one runs is decided by
classpath
order, with no warning. A scanner cannot upgrade a library without
changing what every
other scanner sees.
- **Implicit provider contracts.** libthrift comes from `java-udf`;
`org.apache.doris.thrift.*`
from `preload-extensions`; hadoop from `lib/hadoop_hdfs`. None of this is
declared
anywhere - the only record is pom comments left behind by past incidents,
written in the
providing module rather than the consuming one.
- **A JVM nobody asked for.** BE creates a JVM at startup whether or not any
Java code is
ever used, and a broken jar in the shared layer can stop BE from starting.
This PR replaces that with Trino-style hard isolation: a shared SPI, a
loader, and one
directory per plugin.
**Architecture**
```
be/lib/java/
spi/ doris-jni-spi.jar zero dependencies, the only classes BE
and plugins share
doris-jni-bootstrap.jar the loader and the registry BE calls
plugins/
paimon/ hudi/ iceberg/ jdbc/ max-compute/ trino-connector/
java-udf/ java-writer/
```
A plugin's classloader has the **platform** loader as its parent and
delegates only the
`org.apache.doris.jni.spi.` prefix. Nothing else is shared - each plugin
carries its own
hadoop, its own filesystem implementations, its own logging bridges. BE
addresses a plugin
by `(plugin name, factory name)` instead of by class name, so the wiring is
one table in
`be/src/util/jni_plugin_registry.h` rather than string literals spread over
the readers.
The system classpath is now `conf/` + `lib/java/spi` + `lib/hadoop_hdfs` +
JindoFS/JuiceFS.
`preload-extensions` and `java-common` are gone.
**Also in this PR**
- **The JVM bootstrap moves into Doris**
(`be/src/util/jvm_launcher.{h,cpp}`). It used to
belong to libhdfs, which meant BE's JNI ability depended on libhdfs being
linked. The
three-step bootstrap libhdfs performed is reproduced exactly, including
`FileSystem.loadFileSystems()` - hadoop records its providers once, under
the first
caller's TCCL, so skipping it defers that scan to whichever plugin thread
gets there
first. The JVM is now created lazily: no Java query and no HDFS access
means no JVM.
- **A plugin failing to load is a plugin-level FAILED state**, not `exit(1)`.
- **User Java UDFs see only the contract** - `org.joda.time.` and the Hive
UDF base classes -
and nothing else Doris happens to have on its classpath.
**Sizes** (fat jar -> plugin directory): jdbc 5.6MB -> 3.8MB, java-udf 80MB
-> 20MB,
trino-connector 110MB -> 73MB, max-compute 67MB -> 60MB.
**Behaviour changes worth calling out**
- `log/jni.log` becomes `log/jni.log.0` (JUL rotation naming, 10MB x 5).
- `log/trinoconnector0.log` is removed. Its FileHandler was attached to the
JUL **root**
logger, which is process-wide, so after isolation it would collect every
plugin's log.
Trino logs through `io.airlift.log.Logger` (JUL) and lands in `jni.log`
anyway.
- The `expiration_time` UDF property is not carried into the new module. It
has been
ignored since time-based eviction was removed; it stays accepted and stays
ineffective.
- Profile node names use the plugin name (`paimon`) instead of the class name
(`PaimonJniScanner`).
- Under hudi, `file://` now resolves to hive-apache's `ProxyLocalFileSystem`
rather than
hadoop's `LocalFileSystem` (a subclass overriding only `rename`).
Previously the static
registry was filled once by BE's TCCL; now each plugin fills its own.
**New CI check**
`tools/be-java-plugins/check_plugin_layout.py` runs against a built
`output/be/lib/java`
and enforces four rules: the SPI jar carries only SPI packages; no plugin
ships an SPI
class; no class inside a plugin directory resolves to different bytes in
different jars;
and every class referenced by Doris's own code in a plugin resolves inside
that directory.
The known-benign exceptions are allowlisted with the reason written down.
The file states
what it does **not** prove: it is static and starts from Doris's classes, so
anything
reached by ServiceLoader or reflection is invisible to it.
### Release note
None
### Check List (For Author)
- Test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
Per-module unit tests (716 tests across the 22-module reactor, 0 failures /
0 errors /
0 skipped), plus for each plugin an end-to-end test that builds a real table
and reads it
back through the plugin factory, and a **deployment-directory load probe**:
the plugin is
loaded out of the directory `build.sh` produces, using the real
`PluginRegistry`.
That last one is not redundant. Surefire puts `provided` dependencies on the
test
classpath, so a dependency wrongly marked `provided` leaves every unit test
green and only
fails when the plugin is loaded for real - verified three times during this
work by
mutation (hadoop-common under iceberg, hadoop-mapreduce-client-core under
hudi, guava
under trino-connector).
Each step was mutation tested; the mutations and which assertion each one
killed are
recorded per commit.
- Behavior changed:
- [x] Yes. See "Behaviour changes worth calling out" above.
- Does this need documentation?
- [x] Yes. Deployment layout, the `jni.log.0` name, and the removal of
`trinoconnector0.log` are user-visible. Doc PR to follow.
### State of this PR
Not ready to merge. Outstanding:
- **Rebase.** The branch is based on 5378480d492 and conflicts with #66564,
which touches
`build.sh`, `hadoop-deps/pom.xml` and `preload-extensions/pom.xml` - the
last of which
this PR deletes. There is also a semantic overlap: #66564 makes
`--exclude-obs-dependencies` prevent Huawei artifacts from resolving at
all, while the
plugins here declare `hadoop-huaweicloud` unconditionally, so they need
the same profile
treatment.
- **Full regression run.** The per-connector suites have not been run
against a deployed
cluster yet.
- **Three end-to-end assertions** that need a real BE: no libjvm mapping in
`/proc` when no
plugin is deployed; a query against an undeployed plugin reports `is not
deployed`; a
corrupt jar does not stop BE from starting.
- **One open experiment.** jindo-core carries a native library, and a JVM
binds one of those
to exactly one classloader. A BE reading `oss-hdfs://` natively through
libhdfs and a
plugin loading its own copy are mutually exclusive; this needs measuring
on a real
deployment.
--
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]
