gortiz opened a new pull request, #18459:
URL: https://github.com/apache/pinot/pull/18459
## Summary
Apache Pinot has all the ingredients of a clean plugin architecture —
`PluginManager` with `ClassRealm` isolation, a `pinot-plugin.properties`
activation file, a separate `plugins/` directory in the distribution — but the
build and runtime contradicted that design. This PR replaces the current "shade
everywhere" strategy with classloader-realm-based isolation, keeping shading
only where it has a real purpose (driver libraries and external-process
connectors).
### Problems addressed
1. **Plugin code was baked into the main service jar.**
`pinot-distribution`'s shade pulled in plugin modules transitively via
`pinot-tools`, so `pinot-all-VERSION.jar` contained all plugin classes with
relocations applied.
2. **The startup script put every plugin jar onto the JVM classpath.**
`appAssemblerScriptTemplate` walked `plugins/**/*.jar` and appended them to
`CLASSPATH`. `PluginManager`/`ClassRealm` ran, but its isolation was a no-op
because the parent classloader already had the same classes.
3. **All plugin shading used the same prefix as the distribution**
(`org.apache.pinot.shaded.*`). Two plugins shading to the same prefix collided
with each other; a plugin shading to the same prefix as the distribution had
its own bytecode overridden by the distribution's classes.
4. **Extension-point discovery was fragmented.**
`ServiceLoader.load(Iface)`, `Class.forName(FQCN)`, and a reflection scan for
`@MetricsFactory` all used the system classloader, so they were invisible to
plugin realms.
### What changed
**Phase 1 — Discovery mechanisms made realm-aware**
- `PluginManager.loadServices(Class<T>)`: new helper that walks all plugin
realms and the boot classloader. Replaces 9 `ServiceLoader.load(X.class)` call
sites across `pinot-segment-spi`, `pinot-spi`, `pinot-query-runtime`,
`pinot-core`, `pinot-common`, `pinot-broker`, `pinot-clients`, and
`pinot-plugins/pinot-metrics`.
- `AccessControlFactory`: replaced `Class.forName(FQCN)` with
`PluginManager.get().loadClass(FQCN)`.
- `PinotMetricUtils`: replaced `@MetricsFactory` reflection scan with
`PluginManager.get().loadServices(PinotMetricsFactory.class)`.
- `pinot-tools`: depends on `pinot-avro-base` (library) instead of
`pinot-avro` (plugin) so the Avro base classes are on the classpath without
pulling in the full plugin.
**Phase 2 — Every plugin is a realm**
- Added `pinot-plugin.properties` to every plugin module under
`pinot-plugins/` and `pinot-connectors/` that didn't already have one.
- `PluginManager`: added realm-walk fallback for `createInstance(FQCN)`,
jar-resident properties discovery, and a strong-dep stop list.
**Phase 3 — Plugin-zip layout replaces shaded fat-jars**
- Removed `org.apache.pinot.shaded.*` relocations from the global shade
config in the root `pom.xml`.
- Switched `pinot-distribution` from shaded plugin fat-jars to plugin-zip
layout (classes + dep jars side-by-side, no relocation).
- Hoisted shared Avro, JSON, Parquet, and Protobuf utilities into `-base`
library modules so plugins don't have to bundle duplicates.
- Simplified plugin-zip directory layout: dropped the intermediate
`plugin.type` level.
**Bug fixes (verified against the new distribution)**
- `PluginManager.loadClassFromAnyPlugin`: added `catch
(NoClassDefFoundError)` in the realm walk — previously this error escaped the
catch block and aborted the entire walk; now it is logged at DEBUG and the walk
continues to the next classloader.
- `InputFormatCheck`: fixed wrong FQCN for `pinot-confluent-protobuf`
(removed spurious `.confluent.` sub-package).
- `pinot-hdfs`: use `${hadoop.dependencies.scope}` (compile by default) so
`hadoop-common` is bundled in the plugin-zip assembly.
- `pinot-distribution`: explicit shade `<executions>` block with hardcoded
`package` phase — the `${shade.phase.prop}` property was not overriding the
inherited root-pom execution's phase.
- `pinot-thrift`: remove explicit `provided` scope on `libthrift` so it is
bundled in the plugin-zip.
- Deleted `pinot-plugin.properties` from `pinot-batch-ingestion-spark-3` and
`pinot-batch-ingestion-hadoop` — these run inside external processes (Spark,
Hadoop MR) and must not be loaded as Pinot realms.
**New tooling**
- `pinot-plugin-verifier`: standalone CLI tool that loads every known plugin
through `PluginManager` and reports pass/fail. Achieves 21/21 passes on the
built distribution.
### Where shading is kept
| Module | Why |
|---|---|
| `pinot-clients/pinot-java-client`, `pinot-clients/pinot-jdbc-client` |
User-embedded driver libraries — relocation keeps our jackson/guava off the
user's classpath |
| `pinot-connectors/pinot-spark-3-connector` | Runs inside Spark's process —
relocation avoids classpath conflict with Spark's own jackson/guava |
### Backward compatibility
- `pinot-spi` bytecode level (Java 11) and dependency surface are unchanged.
- Wire protocols and serialization formats are untouched.
- Third-party plugins using `PluginManager.createInstance(FQCN)` continue to
work — drop the plugin directory into `plugins/<name>/` with a
`pinot-plugin.properties` next to it.
- Third-party plugins using `ServiceLoader`-based extension points continue
to work as long as they ship `META-INF/services/<iface-fqcn>` (the standard
`@AutoService` output).
- The `@MetricsFactory` annotation is preserved but is no longer
load-bearing; plugins must add `@AutoService(PinotMetricsFactory.class)` (or
hand-write a service file) to be discovered.
## Test plan
- [ ] `pinot-plugin-verifier` reports 21/21 PASS on the built distribution
(`./mvnw install -DskipTests && java -jar
pinot-plugin-verifier/target/pinot-plugin-verifier-*-jar-with-dependencies.jar
--plugins-dir build/plugins`)
- [ ] `PluginManagerTest#testRealmWalkContinuesPastNoClassDefFoundError` —
regression test for the NCDFE walk-continuation fix
- [ ]
`PluginManagerTest#testLoadServicesFindsImplementationsViaServiceLoader` —
realm-aware service discovery
- [ ] Existing `PluginManagerTest` suite passes
- [ ] `quick-start-batch.sh` runs end-to-end from the built binary
distribution
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
