apache / apache/pinot

Replace global shading with classloader-realm isolation for plugins

Open
#18,386 1 comment 1 reaction 0 assignees View on GitHub
design-review PEP-Request
Dominant language
Java
Stars
6.1k
Forks
1.5k
Avg merge
2d 55m
Merged PRs (30d)
182

Description

# Proposal: Replace global shading with classloader-realm isolation for plugins

## 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 the runtime contradict that design. As a
result, the project applies maven-shade-plugin in 30+ modules, accumulates ~100–200 MB of
duplicate library copies in the distribution, and never gets the version-isolation guarantees
the shading was supposed to provide.

This issue proposes replacing the current "shade everywhere" strategy with a single
classloader-realm-based isolation model, keeping shading only where it has a real purpose
(driver libraries and external-process connectors).

## Current behavior

Three independent things conspire to defeat the existing isolation design:

### 1. Plugin code is baked into the main service jar

`pinot-distribution`'s shade pulls in plugin modules transitively via `pinot-tools`.
`pinot-tools/pom.xml` lists `pinot-avro`, `pinot-csv`, `pinot-json`, `pinot-orc`, `pinot-parquet`,
`pinot-thrift`, `pinot-protobuf`, `pinot-yammer`, `pinot-dropwizard`, `pinot-compound-metrics`,
`pinot-confluent-avro` (compile scope) and `pinot-kafka-3.0`, `pinot-kinesis`, `pinot-pulsar`,
`pinot-batch-ingestion-standalone`, `pinot-s3` (runtime scope) as direct `` entries.
The shade plugin in the root pom (`pom.xml` lines 3084–3152) has no `` filter, so
the resulting `pinot-all-VERSION-jar-with-dependencies.jar` includes all of those plugins'
classes with the relocations applied.

### 2. The startup script puts every plugin jar onto the JVM `-classpath`

`pinot-tools/src/main/resources/appAssemblerScriptTemplate` lines 110–163 walk
`plugins/**/*.jar` and append them to `CLASSPATH`. The system classloader sees every plugin
class. `PluginManager`/`ClassRealm` runs, but its isolation is a no-op because the parent
classloader already has the same classes.

### 3. All plugin shading uses the same prefix as the distribution

Every plugin inherits the root pom's relocations to `org.apache.pinot.shaded.*` — identical to
what `pinot-distribution` produces. When the `lib/` jar and a `plugins/` jar both expose
`org.apache.pinot.shaded.com.fasterxml.jackson.databind.ObjectMapper`, the first one on the
classpath wins. `lib/*` precedes `plugins/*` in the script, so the plugin jar's bundled jackson
is dead code.

The stated design intent — *"pinot core relocates so plugins can use their own jackson/guava/scala
versions"* — is **not** what the build produces. Two plugins shading to the same prefix collide
with each other; a plugin shading to the same prefix as the distribution gets its own bytecode
overridden by the distribution's classes.

### Extension-point discovery is fragmented across four mechanisms

There are four different ways a plugin implementation gets discovered today; only one of them
is realm-aware:

| Mechanism | Used by | Realm-aware? |
|---|---|---|
| `PluginManager.createInstance(FQCN)` | `StreamConsumerFactoryProvider`, `RecordReaderFactory`, `PinotFSFactory` | Yes |
| `ServiceLoader.load(Iface)` (default thread-context CL) | `IndexService`, `ExecutorServiceUtils`, `ResponseStoreService`, `RecordEnricherRegistry`, `OpChainConverterDispatcher`, `TimeBoundaryStrategyService`, `LogicalTableConfigSerDeProvider`, `SslContextProviderFactory`, `KafkaStarterUtils`, `CompoundPinotMetricsFactory` | No |
| `Class.forName(FQCN)` | `AccessControlFactory` | No |
| Reflection scan for `@MetricsFactory` | `PinotMetricUtils` | No |

These all "work" today only because plugin classes are on the system classpath. The moment
plugins move into isolated realms, three of these four paths stop seeing them.

## Proposed change

End state:

1. **`lib/` contains only core service code** — broker, server, controller, minion, common,
spi, segment-spi, segment-local, query-planner, query-runtime, pinot-tools' admin code.
No plugin modules, no shaded plugin classifiers.
2. **Plugins live exclusively in `plugins///`**, each in its own subdirectory
with the plugin's primary jar plus its runtime dep jars side-by-side. **No plugin jar is
on the JVM `-classpath`.**
3. **Every plugin has a `pinot-plugin.properties` file**, so `PluginManager` always loads it
into a `ClassRealm`. Properties files import `org.apache.pinot.spi` (and any other
parent-realm packages the plugin needs) explicitly.
4. **No relocation anywhere in the service or in plugins.** Each plugin is in its own
classloader; conflict resolution is by classloader isolation, not by package renaming.
5. **All four current discovery mechanisms become realm-aware** (see migration section
below).

### Where shading still has a real purpose

Two narrow categories of code keep `maven-shade-plugin` with relocations, because they don't
ship inside a Pinot service process and therefore don't have realms to fall back on:

| Module | Why it still shades |
|---|---|
| `pinot-clients/pinot-java-client`, `pinot-clients/pinot-jdbc-client` | User-embedded driver libraries. Consumers depend on them inside their own applications; relocation is the only way to keep our jackson/guava out of the user's classpath. |
| `pinot-connectors/pinot-spark-3-connector` (and any future external connector) | Runs inside someone else's process (Spark). That process owns the system classloader and has its own jackson/guava/protobuf. |

`pinot-cli` and `pinot-perf` keep building uber-jars but no longer relocate — they need
bundling, not renaming.

`pinot-distribution` itself stops relocating. It can keep the shade plugin for uber-jar
packaging or switch to `maven-assembly-plugin`'s `jar-with-dependencies` descriptor; either
way, no relocations.

The unused `build-shaded-jar` profiles in `pinot-spi`, `pinot-common`, `pinot-core`,
`pinot-jdbc-client` are deleted — they produce classifier artifacts that nothing in the repo
consumes (verified with `grep` across all `pom.xml`).

## Phased migration

Each phase is independently testable. Phases 1 and 2 must ship together (either alone breaks
startup); Phase 3 follows; Phase 4 is per-call-site and can be tackled incrementally.

### Phase 1 — Get plugin code out of the main jar and off the system classpath

- Remove plugin module deps from `pinot-tools/pom.xml`. Quickstart commands that today rely
on plugin classes being on the system classpath need to load them through `PluginManager`.
- Update `appAssemblerScriptTemplate` (lines 110–163) to **not** put plugin jars on
`CLASSPATH`. Keep `-Dplugins.dir` so `PluginManager` can still find them.
- Introduce a `quickstart-dev` Maven profile (described in the next subsection) so quickstarts
remain runnable from Maven and IDEs without re-coupling `pinot-tools` to plugins.

**Verification**:
- `jar tf .../lib/pinot-all-*.jar | grep -E 'KafkaConsumer|S3PinotFS|AvroRecordReader'` returns
nothing.
- `find .../plugins -name '*.jar' -exec jar tf {} \; | grep KafkaConsumer | sort -u` returns
the kafka plugin jar(s) only.
- `build/bin/quick-start-batch.sh` still works.
- `mvn -pl pinot-tools -Pquickstart-dev exec:java -Dexec.mainClass=org.apache.pinot.tools.Quickstart`
still works.

#### Running quickstarts after Phase 1

Quickstarts (`Quickstart`, `HybridQuickstart`, `RealtimeQuickStart`, `UpsertQuickStart`, etc.)
need plugins for their input formats and stream sources. Today they happen to work in dev
because `pinot-tools` lists every plugin as a `` and the IDE / `mvn exec:java`
puts those on the main classpath. After Phase 1 removes those deps, three distinct ways to
run quickstarts:

##### 1. From the built distribution (production layout)

```
mvn -Pbin-dist install -DskipTests
build/bin/quick-start-batch.sh
```

`plugins/` is already populated with realm-loadable plugin directories; the startup script
sets `-Dplugins.dir=$BASEDIR/plugins`; `PluginManager` discovers them as realms. **No
behavioral change for end users running prebuilt binaries.**

##### 2. From Maven (`exec:java` or `exec:exec`)

Add a `quickstart-dev` profile to `pinot-tools/pom.xml` that re-introduces the plugin module
deps **only when explicitly activated**:

```xml

quickstart-dev

org.apache.pinotpinot-kafka-3.0
org.apache.pinotpinot-avro
org.apache.pinotpinot-csv
org.apache.pinotpinot-json
org.apache.pinotpinot-parquet
org.apache.pinotpinot-orc

```

Then:

```
mvnd -pl pinot-tools -Pquickstart-dev exec:java \
-Dexec.mainClass=org.apache.pinot.tools.Quickstart
```

The profile is **off by default**, so it does not affect the production build. The `bin-dist`
profile and the `quickstart-dev` profile can both be active when packaging if a developer
wants the binary distribution to also have plugins on the dev classpath, but neither needs
the other.

`PluginManager.createInstance(FQCN)` will resolve plugin classes from the system classloader
(via its `Class.forName` fallback) when the profile is active and `plugins/` is empty — same
behavior as today. If both the profile and a built `plugins/` directory are present,
`PluginManager` finds the realm copy first; the system-CL copy is unused.

##### 3. From the IDE (IntelliJ / VS Code)

The IDE picks up Maven profiles from the project's profile selection panel. Enabling
`quickstart-dev` in the IDE's Maven view adds the plugin modules to the IDE classpath of
`pinot-tools`, after which the existing run configurations for `Quickstart` /
`HybridQuickstart` / `RealtimeQuickStart` continue to work unchanged.

For users who prefer not to enable the profile globally, an alternative is to point
quickstarts at a built plugin tree:

```
# JVM args on the run config
-Dplugins.dir=/abs/path/to/pinot/build/plugins
```

This requires having run `mvnd -Pbin-dist install` at least once so `build/plugins/` exists.
`PluginManager` loads from there as realms, the IDE classpath only contains `pinot-tools`'
real (non-plugin) deps, and the dev run mirrors production exactly. This is the recommended
path for changes that touch the plugin loader itself, since it actually exercises the realm
code path.

##### 4. From integration tests

`pinot-integration-tests` already pulls in plugin modules transitively via its test deps
(kafka, avro, etc.). Tests run `Quickstart.main(...)` against the test classpath, which
mirrors the `quickstart-dev` profile shape. **No change required for integration tests.**

#### What goes in the `quickstart-dev` profile

Only plugins that an in-tree quickstart actually invokes:
- Stream: `pinot-kafka-3.0`
- Input format: `pinot-avro`, `pinot-csv`, `pinot-json`, `pinot-parquet`, `pinot-orc`,
`pinot-confluent-avro`, `pinot-thrift`, `pinot-protobuf`
- Batch ingestion: `pinot-batch-ingestion-standalone`
- Filesystem: `pinot-s3` (used by S3-aware quickstarts)
- Metrics: `pinot-yammer` or `pinot-dropwizard`

Plugins that no quickstart exercises (`pinot-pulsar`, `pinot-kinesis`, `pinot-azure`,
`pinot-hdfs`, `pinot-adls`, `pinot-gcs`, `pinot-arrow`, `pinot-clp-log`, `pinot-kafka-4.0`,
`pinot-batch-ingestion-{spark-3,hadoop}`, `pinot-timeseries-m3ql`) are NOT in the profile —
they're only useful when run from a real distribution that has them in `plugins/`.

### Phase 2 — Make every plugin a realm

- Add `src/main/resources/pinot-plugin.properties` to every module under `pinot-plugins/` and
`pinot-connectors/` (existing examples: `pinot-dropwizard`, `pinot-yammer`).
- The existing `plugin-assembly` profile in `pinot-plugins/pom.xml` is already gated on the
presence of this file; adding it activates the proper plugin-zip layout (classes + dep jars
side-by-side).
- Update `pinot-distribution/pinot-assembly.xml` to ship the plugin-zip layout instead of a
single shaded jar per plugin.

**Verification**: `PluginManager` log lines say "Realm ``" for every plugin, not
"PluginClassLoader fallback".

### Phase 3 — Drop relocations everywhere except driver/connector modules

- Delete the three `` blocks in the root `pom.xml` (lines 3137–3150).
- Remove `shade.phase.prop=package` from `pinot-distribution/pom.xml` and from every plugin
pom under `pinot-plugins/` (~25 files).
- Delete the `build-shaded-jar` profile activations in `pinot-spi/pom.xml`,
`pinot-common/pom.xml`, `pinot-core/pom.xml`, `pinot-jdbc-client/pom.xml`.
- Re-add explicit shade configs (without relocations) in `pinot-cli`, `pinot-perf` if needed
for self-contained uber-jars.
- Keep relocations in: `pinot-clients/pinot-java-client`, `pinot-clients/pinot-jdbc-client`,
`pinot-connectors/pinot-spark-3-connector`.

**Verification**:
- `jar tf plugins/.../pinot-kafka-3.0-*.jar | grep 'org/apache/pinot/shaded'` returns nothing.
- Plugin jar sizes drop noticeably (Scala-using plugins lose ~7 MB; jackson-using plugins
~3 MB; guava-using plugins ~2 MB).

### Phase 4 — Make every discovery mechanism realm-aware

After Phases 1–3, plugin classes are no longer visible to the system classloader. Every code
path that today discovers plugin implementations needs to consult plugin realms instead. There
are four current mechanisms; below is how each one migrates.

#### 4a. `PluginManager.createInstance(FQCN)` — no change

Already realm-aware. Dispatches to `PluginClassLoader` for the legacy registry or to the
`ClassRealm` via `Class.forName(name, true, realm)` (`PluginManager.java` lines 452–468).

Call sites to leave alone:
- `pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/PinotFSFactory.java` line 51
- `pinot-spi/src/main/java/org/apache/pinot/spi/stream/StreamConsumerFactoryProvider.java` line 38
- `pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/RecordReaderFactory.java` line 157

The `INPUT_FORMAT_TO_RECORD_READER_CLASS_NAME_MAP` static map in `RecordReaderFactory` is
ugly but functional; can be cleaned up separately.

#### 4b. `ServiceLoader.load(Iface)` — switch to a realm-aware helper

Add a new public method on `PluginManager`:

```java
public List loadServices(Class iface) {
List results = new ArrayList<>();
// Boot/system classloader (covers core jar)
ServiceLoader.load(iface, getClass().getClassLoader()).forEach(results::add);
// Legacy PluginClassLoader registry
for (PluginClassLoader pcl : _registry.values()) {
ServiceLoader.load(iface, pcl).forEach(results::add);
}
// ClassRealms
for (ClassRealm realm : _classWorld.getRealms()) {
ServiceLoader.load(iface, realm).forEach(results::add);
}
return results;
}
```

Replace each `ServiceLoader.load(X.class)` with `PluginManager.get().loadServices(X.class)`:

| File | Line | Interface |
|---|---|---|
| `pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexService.java` | 135 | `IndexPlugin` |
| `pinot-spi/src/main/java/org/apache/pinot/spi/executor/ExecutorServiceUtils.java` | 69 | `ExecutorServicePlugin` |
| `pinot-spi/src/main/java/org/apache/pinot/spi/cursors/ResponseStoreService.java` | 53 | `ResponseStore` |
| `pinot-spi/src/main/java/org/apache/pinot/spi/recordtransformer/enricher/RecordEnricherRegistry.java` | 39 | `RecordEnricherFactory` |
| `pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/plan/OpChainConverterDispatcher.java` | 102 | `OpChainConverter` |
| `pinot-core/src/main/java/org/apache/pinot/core/routing/timeboundary/TimeBoundaryStrategyService.java` | 36 | `TimeBoundaryStrategy` |
| `pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableConfigSerDeProvider.java` | 57 | `LogicalTableConfigSerDe` |
| `pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/SslContextProviderFactory.java` | 109 | `SslContextProvider` |
| `pinot-plugins/pinot-metrics/pinot-compound-metrics/.../CompoundPinotMetricsFactory.java` | 189 | `PinotMetricsFactory` |

No plugin-side change is required: Pinot already uses Google's `@AutoService` annotation
processor to generate `META-INF/services/` files at compile time. Existing
implementations are annotated `@AutoService(.class)` — see
`pinot-segment-local/.../*IndexPlugin.java` (every index type), `CachedExecutorServicePlugin`
and `FixedExecutorServicePlugin` in `pinot-common`, `MinTimeBoundaryStrategy` in `pinot-core`,
`FsResponseStore` in `pinot-broker`, `OpChainConverter` in `pinot-query-runtime`,
`DefaultLogicalTableConfigSerDe` in `pinot-common`, `CustomFunctionEnricherFactory` in
`pinot-segment-local`, and the three metrics factories in `pinot-plugins/pinot-metrics/`.
Today's discovery works because those generated service files are inside jars that happen to
be on the system classpath. After Phase 1, the same files are inside plugin-realm jars; the
`PluginManager.loadServices` walk picks them up unchanged.

Third-party plugins implementing one of these SPIs almost certainly use `@AutoService` too
(or hand-write `META-INF/services` files) — that's the only way `ServiceLoader.load(Iface)`
finds them today. **They keep working without recompilation as long as Phases 1 and 4b ship
together.**

The `KafkaStarterUtils` quickstart call (`pinot-tools/.../KafkaStarterUtils.java` line 62) is
not in the production hot path and can use the same helper.

#### 4c. `Class.forName(FQCN)` — switch to `PluginManager.createInstance`

One production call site:

`pinot-broker/src/main/java/org/apache/pinot/broker/broker/AccessControlFactory.java` line 66:

```java
// before
(AccessControlFactory) Class.forName(accessControlFactoryClassName)
.getDeclaredConstructor().newInstance();
// after
PluginManager.get().createInstance(accessControlFactoryClassName);
```

This works because `createInstance` already searches all plugin realms and falls back to
`Class.forName` on the system CL.

#### 4d. Reflection scan for `@MetricsFactory` — replace with realm-aware `ServiceLoader`

`pinot-spi/src/main/java/org/apache/pinot/spi/metrics/PinotMetricUtils.java` lines 74–110
scans the system classpath for `@MetricsFactory`-annotated classes. The three in-tree metrics
factories (`DropwizardMetricsFactory`, `YammerMetricsFactory`, `CompoundPinotMetricsFactory`)
are already `@AutoService(PinotMetricsFactory.class)`-annotated, so
`META-INF/services/org.apache.pinot.spi.metrics.PinotMetricsFactory` files exist in their
generated jars — the annotation scan and the service-loader path can find each of them today.
**For these three plugins, the annotation scan is already redundant.**

Migration: drop the annotation scan and replace it with
`PluginManager.get().loadServices(PinotMetricsFactory.class)`. No change is needed for the
in-tree metrics plugins.

**Third-party metrics plugins**: any plugin that registered through `@MetricsFactory` and
relied on the reflection scan to be discovered (i.e. did *not* annotate with
`@AutoService(PinotMetricsFactory.class)` and did *not* hand-write a `META-INF/services` file)
will stop being found after Phase 4d. Such plugins must either (a) add `@AutoService` and
recompile, or (b) hand-write the service file. The `@MetricsFactory` annotation itself can
stay on the source for compatibility — it just becomes documentation rather than a discovery
trigger.

If preserving full no-touch backward-compatibility for purely-annotated metrics plugins
matters more than removing the reflection scan, an alternative is to keep the scan but run it
once per plugin realm classloader (walk `PluginManager`'s realms in `PinotReflectionUtils`).
That preserves old plugins but is slower at startup and keeps the duplicate discovery path.

The line 126 `Class.forName(listenerClassName)` for metrics listeners migrates the same way
as 4c (use `PluginManager.createInstance`).

#### Per-plugin work for Phase 4

For every plugin module that implements one of the SPIs migrated above, add the corresponding
`META-INF/services/` resource. This is the only plugin-side change required —
plugins do not need to know about realms; the discovery side handles that.

**Verification per migrated extension point**:
- The plugin's jar is in `plugins///` only (not on `-classpath`).
- The implementation is discovered when the plugin's realm is loaded.
- Two plugins providing the same SPI both work; ordering matches plugin discovery order.

## Impact on third-party / out-of-tree plugins

In-tree plugins ship with Pinot and migrate atomically with each phase. Third-party plugins
(distributed independently, possibly built against an older Pinot version) can be on a different
schedule. This section enumerates the impact and the upgrade path for each phase and each
discovery mechanism.

### Phases 1 & 2 — packaging and classpath

| Old plugin shape | Behavior on the new runtime | What needs upgrading |
|---|---|---|
| Single shaded uber-jar (everything bundled, classes already shaded to `org.apache.pinot.shaded.*`) | Works — the jar is self-contained, drops into a realm with all its bundled classes including its own copy of shaded jackson/guava/scala. The realm imports `org.apache.pinot.spi.*` from the parent so calls into Pinot's SPI continue to resolve. | Nothing required to keep working. To benefit from lighter packaging and to drop the redundant jackson/guava/scala bundling, rebuild against the new Pinot once Phase 3 lands. |
| Plugin published as separate primary jar + dep jars and dropped into `plugins//` (already the layout this proposal moves toward) | Works directly under realms. No change. | Nothing. |
| Plugin that today relies on being on the **main** JVM `-classpath` (e.g., users put their plugin jar into `lib/`) | **Breaks.** After Phase 1 the script no longer treats arbitrary jars in `plugins/` as classpath entries; deploying to `lib/` is also not a supported workaround for plugins post-migration. | Move the plugin jar (or its plugin-zip) into `plugins///`. Add a `pinot-plugin.properties` next to it (an empty file is fine for limited realms; set `parent.realmId=pinot` for plugins that need to call into pinot-common/pinot-core). |
| Plugin that explicitly references shaded names in pinot-common/pinot-core internals (e.g. uses `org.apache.pinot.shaded.com.google.common.base.Preconditions` from pinot-common) | After Phase 3, pinot-common no longer exposes those shaded classes. References resolve to `NoClassDefFoundError`. | Rebuild against the new Pinot, dropping the inherited relocations. The original `com.google.common.base.Preconditions` is now visible at its real package name. |

**Migration window**: a plugin that works today and is rebuilt against new Pinot keeps working
across the version boundary. A plugin that is **not** rebuilt and was relying on shaded Pinot
internals (rare) breaks at startup with `NoClassDefFoundError`. A plugin that is not rebuilt
and only uses pinot-spi keeps working — pinot-spi has not been shaded.

### Phase 3 — relocations dropped

This is invisible to a plugin that doesn't touch pinot-common / pinot-core internals. The only
plugin-visible change is at the dependency level: jackson, guava, and scala return to their
original package names everywhere on the parent classpath. Plugin code that calls
`com.fasterxml.jackson.databind.ObjectMapper` works unchanged; plugin code that calls
`org.apache.pinot.shaded.com.fasterxml.jackson.databind.ObjectMapper` because it was reaching
into a shaded internal stops resolving and the plugin has to be rebuilt.

### Phase 4 — discovery migration, per mechanism

#### 4a. `PluginManager.createInstance(FQCN)` — no impact on third-party plugins

The plugin contract is unchanged. Plugin ships a public class with a no-arg constructor; user
config points at the FQCN. The realm walk is internal to Pinot. **Old plugins continue to work
without any changes.**

#### 4b. `ServiceLoader.load(Iface)` — no plugin-side upgrade needed

Plugins that today use these extension points already ship a
`META-INF/services/` resource — typically generated by `@AutoService` at compile
time, occasionally hand-written. That's the only way `ServiceLoader.load(Iface)` finds them
today. After Phase 1, those service files stop being found by the system-CL
`ServiceLoader.load(...)` because the plugin jar is no longer on the system classpath. After
Phase 4b, the realm-aware `PluginManager.loadServices` walks plugin realms and finds the
service file inside the plugin jar. **No plugin-side change required.**

There is a window between Phase 1 and Phase 4b where the plugin's service registration is
unreachable — Phases 1 and 4b should land in the same release. If they cannot, a third-party
plugin using one of the migrated extension points (`IndexPlugin`, `ExecutorServicePlugin`,
`ResponseStore`, `RecordEnricherFactory`, `OpChainConverter`, `TimeBoundaryStrategy`,
`LogicalTableConfigSerDe`, `SslContextProvider`) must be redeployed only after Phase 4b ships.

#### 4c. `Class.forName(FQCN)` — no impact on third-party plugins

`AccessControlFactory` plugins are configured by FQCN today. The implementation class FQCN
remains the same; the only change is that broker startup now resolves it via
`PluginManager.createInstance` instead of `Class.forName` against the broker's classloader.
Plugin authors do not have to do anything. **Old plugins continue to work.**

#### 4d. `@MetricsFactory` annotation scan — depends on which option is chosen

This is the only Phase 4 sub-step where third-party plugins may need code changes.

**Preferred option (drop the annotation scan; switch to `ServiceLoader`)**: every plugin that
today provides a `@MetricsFactory`-annotated class must add
`META-INF/services/org.apache.pinot.spi.metrics.PinotMetricsFactory` listing the FQCN of that
class. Plugins that do not add this file are silently not discovered after Phase 4d ships.
The `@MetricsFactory` annotation can remain present (harmless) for source-level
backward-compatibility but is no longer load-bearing.

**Fallback option (keep the annotation scan, run it per realm)**: no plugin-side change
required. Old plugins continue to work.

The proposal recommends the preferred option for code-cleanup reasons. If preserving full
no-touch backward compatibility for third-party metrics plugins is more important than
removing the reflection scan, the fallback is acceptable.

### Summary table — what does a plugin author have to do?

| Plugin extension point | If plugin is not rebuilt | If plugin is rebuilt against new Pinot |
|---|---|---|
| `PinotFS`, `StreamConsumerFactory`, `RecordReader` (uses `PluginManager.createInstance`) | Works — drop existing shaded jar into `plugins///` with a `pinot-plugin.properties` next to it | Works; can drop its own shading and ship a plugin-zip layout |
| `IndexPlugin`, `ExecutorServicePlugin`, `ResponseStore`, `RecordEnricherFactory`, `OpChainConverter`, `TimeBoundaryStrategy`, `LogicalTableConfigSerDe`, `SslContextProvider` (uses `ServiceLoader`) | Works as long as Phase 1 and Phase 4b land together; existing `META-INF/services/` file is still load-bearing | Works; can drop its own shading |
| `AccessControl` (uses `Class.forName`) | Works | Works |
| `PinotMetricsFactory` (`@MetricsFactory` scan) | Works if the plugin uses `@AutoService(PinotMetricsFactory.class)` (or hand-wrote a `META-INF/services` file). **Breaks if the plugin only relied on the `@MetricsFactory` reflection scan** — must add `@AutoService` (or a service file) and rebuild | Add `@AutoService(PinotMetricsFactory.class)` if not already present |
| Plugin that touches pinot-common / pinot-core internals (anything beyond pinot-spi) using shaded class names | Breaks at runtime with `NoClassDefFoundError` for `org.apache.pinot.shaded.*` references that no longer exist | Works after recompilation against the new Pinot; references resolve at original package names |

### Documentation deliverables

- A short "plugin author migration guide" page in the Pinot docs:
- The `pinot-plugin.properties` recipe (when an empty file is enough vs. when
`parent.realmId=pinot` is needed).
- The plugin-zip layout (classes + dep jars side-by-side).
- The list of extension points that require a `META-INF/services/` file.
- A release-notes entry calling out the `@MetricsFactory` change (if the preferred 4d option
is chosen).
- A `BACKWARD-INCOMPAT` label on the PR(s) that move plugins off the system classpath
(Phase 1) so cluster operators see it during upgrade prep.

---

## Out of scope

- Replacing Jersey/HK2 in REST resource graphs.
- Per-query / per-request injection scopes.
- Rewriting the per-row hot path with a different DI/SPI model.
- StarTree's commercial `startree-pinot` distribution — the OSS change requires a coordinated
follow-up there since `startree-distribution` currently consumes 20 OSS plugin shaded
classifiers that will no longer be produced.

## Compatibility

- `pinot-spi` stays unchanged in terms of bytecode level (Java 11) and dependency surface.
- Mixed-version clusters can upgrade roles independently; nothing in this proposal touches
wire protocols or serialization.
- For one release cycle after Phase 4, both the old discovery path (e.g.
`ServiceLoader.load(IndexPlugin.class)` against the system CL) and the new realm-aware path
can coexist by leaving the old `META-INF/services` entries in place. Older plugin jars
continue to work; new plugin jars don't have to ship duplicates.
- Downstream distributions consuming the OSS `-shaded` classifiers (e.g. `startree-pinot`)
need a coordinated update.

## References

- Existing realm machinery: `pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginManager.java`
- Current shade plugin definition: root `pom.xml` lines 3084–3152
- Startup script that defeats realm isolation: `pinot-tools/src/main/resources/appAssemblerScriptTemplate` lines 110–163
- Distribution → plugin transitive path: `pinot-distribution/pom.xml` → `pinot-tools/pom.xml`
- Existing realm-loaded plugin example: `pinot-plugins/pinot-metrics/pinot-dropwizard/src/main/resources/pinot-plugin.properties`

Contributor guide

Open the contributing guide

Research direction

Start with pinot-tools/pom.xml, the root pom.xml shade configuration, and pinot-tools/src/main/resources/appAssemblerScriptTemplate; then trace PluginManager and ClassRealm. Run the Phase 1 jar-content checks and quickstart commands described in the issue before changing later phases. Done requires plugins to be absent from the core jar and JVM classpath, while realm loading and quickstarts continue to work.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, build-system, devtools
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.