[feature](plugin) Add information_schema.plugins for kernel plugin metadata#65644
[feature](plugin) Add information_schema.plugins for kernel plugin metadata#65644CalvinKirs wants to merge 4 commits into
Conversation
…tadata Add a unified information_schema.plugins system table that lists all loaded kernel plugins (filesystem, connector, authentication, lineage) of the currently connected FE. Columns: PLUGIN_NAME, PLUGIN_TYPE, PLUGIN_VERSION, SOURCE, DESCRIPTION. Design points: - (plugin_type, plugin_name) is the primary key, enforced at load time: the first registration wins and is never silently overridden, so directory jars cannot displace built-in plugins. - name()/description() are snapshotted once at load time; querying the table never executes plugin code. A broken self-report fails the load. - Plugin names are validated at load (trimmed, non-blank, <=64 chars, [A-Za-z0-9._-]); invalid names fail the load for external plugins. - PLUGIN_VERSION comes from the jar MANIFEST Implementation-Version (null when unavailable), never from plugin interface methods. - Only successfully loaded plugins appear; failures go to logs. - Querying requires global ADMIN privilege; non-admin users see no rows. - View semantics are per-FE: the BE scanner RPCs the currently connected FE (same pattern as catalog_meta_cache_statistics). Implementation: - fe-extension-spi: add default description() to PluginFactory. - fe-extension-loader: new PluginRegistry (process-wide, load-time snapshots) and PluginNames validation; DirectoryPluginRuntimeManager snapshots name/description and reads Implementation-Version. - Four family managers register BUILTIN/EXTERNAL rows; filesystem and connector managers now also reject external providers whose name conflicts with an already-registered one. - FE: SchemaTable definition + MetadataGenerator data source; thrift TSchemaTableName.PLUGINS. Reuses existing TSchemaTableType.SCH_PLUGINS. - BE: new schema_plugins_scanner wired into schema_scanner. - Tests: loader unit tests and regression suite test_plugins_schema.
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
One open design question worth discussing: should querying Today only authentication is lazy (external auth plugins load on first use via |
|
/review |
|
run buildall |
|
Codex automated review failed and did not complete. Error: Review context preparation failed before Codex ran; inspect the 'Prepare authoritative PR context and required AGENTS guides' step. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
FE UT Coverage ReportIncrement line coverage |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes for eight correctness, compatibility, lifecycle, and test-contract issues documented inline.
Review checkpoints:
- Goal and tests: the intended result is an ADMIN-only, five-column inventory of successfully loaded plugins on the FE to which the session is connected. The happy-path unit/regression coverage does not exercise the advertised SQL NULL, family-to-registry identity, conflict cleanup, or split root/lib version paths. The regression also bypasses Doris's required generated-result convention and requires a BUILTIN row that the standard packaged cluster need not contain because filesystem/connector implementations are deployed as EXTERNAL directory plugins.
- Scope and compatibility: the default
description()method is binary-compatible, but tightening every external plugin name to a new ASCII/64-character grammar is not additive and can reject previously loadable deployments. - Runtime consistency and lifecycle: the concurrent containers and sorted registry snapshot are individually safe, with no new lock or hot-path concern. Publication across family runtime state and the singleton registry is not one decision, however; duplicate/invalid names can make inventory disagree with execution, built-in metadata failures can leave partial state, and newly rejected connector/filesystem handles remain retained with open classloaders.
- FE/BE contract: schema order, requested-column handling, current-user propagation, ADMIN filtering, connected-FE routing, scanner batching, and the supported BE-before-FE rolling-upgrade direction were traced and found consistent. Missing versions are not encoded/materialized as SQL NULL.
- Persistence, configuration, and data writes: this patch adds no persistence/replay, transaction, data-write, or dynamic-configuration path.
- Security model: ADMIN filtering is consistent with the stated contract. Internal FE/BE RPC and plugin deployment are trusted boundaries under the repository threat model; I found no separate in-scope security vulnerability.
- Tooling and observability: a literal NUL makes the new production registry source binary to Git and normal search/review tooling even though compilation succeeds.
Validation was static as required by the review prompt; I did not run local builds or tests. At the reviewed head, COMPILE, FE UT, CheckStyle, Cloud UT, External Regression, NonConcurrent Regression, and vault_p0 are green. Several external regression/coverage/performance/BE jobs are red with unavailable TeamCity details; the macOS BE UT failure is an environment JDK 25-versus-17 mismatch rather than evidence for these findings.
Automation convergence status: the three-round limit was reached when the final packaging/test candidate was found. That candidate was independently verified and merged into the test comment, and every suspicious point now has a concrete disposition, but the exact final comment set could not receive another all-agent unchanged reconfirmation within the allowed rounds; this review is therefore marked incomplete under the review-run contract.
| e); | ||
| } | ||
| if (pluginName == null || pluginName.trim().isEmpty()) { | ||
| String nameValidationError = PluginNames.validate(pluginName); |
There was a problem hiding this comment.
[P1] Preserve the existing external-plugin name contract. Before this change the loader accepted every nonblank name and stored its trimmed value, while the public SPI/guide only require a stable nonempty identity. This validator now rejects deployed names such as acme/auth:v2 (and all non-ASCII names or names over 64 chars) during FE startup, even though this PR is adding inventory metadata. Please keep the existing acceptance contract, or introduce this restriction with an explicit compatibility/migration plan.
| ServiceLoader.load(AuthenticationPluginFactory.class) | ||
| .forEach(factory -> factories.put(factory.name(), factory)); | ||
| .forEach(factory -> { | ||
| factories.put(factory.name(), factory); |
There was a problem hiding this comment.
[P1] Make runtime selection and inventory publication use one canonical decision. This map uses the raw name and last-wins put, while PluginRegistry trims/validates the name and keeps the first value; its boolean result is ignored. With two ServiceLoader factories named ldap, getFactory() executes the second but information_schema.plugins describes the first. A name rejected by the registry can likewise remain executable yet absent from the table. Please normalize/validate once and apply the same precedence atomically to both stores (and the parallel family managers).
| ServiceLoader.load(ConnectorProvider.class) | ||
| .forEach(p -> { | ||
| providers.add(p); | ||
| PluginRegistry.getInstance().registerBuiltin(PLUGIN_FAMILY, p); |
There was a problem hiding this comment.
[P2] Isolate the new metadata callback before publishing the provider. registerBuiltin() invokes the overridable description() here without a per-provider catch, after providers.add(p). One throwing implementation can therefore abort FE startup/discovery or leave an active provider with no inventory row; the external loader already treats this callback as a per-plugin failure and cleans up. Please snapshot metadata under the same failure boundary before mutating runtime state, then either reject that provider cleanly or deliberately fall back to empty metadata.
| for (PluginHandle<ConnectorProvider> handle : report.getSuccesses()) { | ||
| // Built-ins (and earlier-loaded plugins) must never be displaced by a | ||
| // same-name directory jar. | ||
| if (hasProviderNamed(handle.getPluginName())) { |
There was a problem hiding this comment.
[P2] Dispose the handle rejected by this family-level conflict. loadAll() has already inserted this success into runtimeManager.handlesByName, so continue leaves the factory and its URLClassLoader strongly retained and the jar resources open for the FE lifetime; FileSystemPluginManager has the same path. Please reject before publication or add an atomic remove-and-close operation. Closing alone is insufficient while the handle remains in the runtime map.
| e); | ||
| } | ||
|
|
||
| String implementationVersion = readImplementationVersion( |
There was a problem hiding this comment.
[P2] Look up the version in the jar that actually defined the factory. Discovery only parses the class name from root-jar service descriptors, then the runtime classloader can load that class from root plus lib jars. With a root descriptor jar and the implementation in lib/plugin-impl.jar, loading succeeds but this root-only search returns NULL. Please use the loaded class's code source or scan all runtime jars in classloader order, and cover the split layout.
| row.addToColumnValue(new TCell().setStringVal(record.getName())); | ||
| row.addToColumnValue(new TCell().setStringVal(record.getType())); | ||
| if (record.getVersion() == null) { | ||
| row.addToColumnValue(new TCell()); |
There was a problem hiding this comment.
[P1] Preserve SQL NULL end to end for unavailable versions. An unset TCell reaches SchemaScanner::insert_block_column(), whose VARCHAR branch inserts the default empty string and always appends false to the null map, so PLUGIN_VERSION IS NULL is false and the advertised NULL becomes ''. Please send the explicit null marker and make this scanner/helper insert a nullable default when it is set, with a result-level test for a versionless built-in.
|
|
||
| @BeforeEach | ||
| void setUp() { | ||
| PluginRegistry.getInstance().clearForTest(); |
There was a problem hiding this comment.
[P2] Anchoring here because the production registry has no textual diff: keep PluginRegistry.java textual. Its key separator is a literal NUL byte, so Git classifies the entire new 197-line source as binary, normal text diffs hide it, and ordinary text search cannot inspect it. The current compile checks pass, but the repository/tooling regression remains. Please express the runtime separator with a Java escape (or use a structured key) so the corrected file appears as a normal text diff.
| Assert.assertTrue(row[2] == "BUILTIN" || row[2] == "EXTERNAL") | ||
| } | ||
| def builtinRows = rows.findAll { it[2] == "BUILTIN" } | ||
| Assert.assertTrue("expect built-in plugins registered", builtinRows.size() > 0) |
There was a problem hiding this comment.
[P1] Do not require a BUILTIN row in the standard packaged cluster. FE core removes filesystem implementations from its runtime classpath (and authentication's password provider is test-scope); build.sh deploys filesystem/connector implementations under output/fe/plugins, and these managers register those rows as EXTERNAL. A correctly populated default inventory can therefore have zero BUILTIN rows and fail here. Please assert the packaged external inventory (or inject a controlled built-in in a unit test). Also convert deterministic results to qt_/order_qt_, generate the .out, and include PLUGIN_VERSION so the advertised NULL is covered.
- PluginRegistry: replace NUL-byte key separator with a space (file was treated as binary by git); safe because plugin names are charset-restricted - DirectoryPluginRuntimeManager: add discard() so rejected duplicate handles release their classloaders; read Implementation-Version from the factory class's own code-source jar first (covers root-descriptor + lib-impl layouts) - Managers (fs/connector/auth/lineage): snapshot self-reported metadata before publishing each built-in so one throwing plugin is skipped instead of aborting startup; discard rejected duplicate handles - AuthenticationPluginManager: built-in registration is now first-wins (putIfAbsent), matching the registry's precedence - BE scanner: surface unset string cells as SQL NULL instead of empty string - Regression test: don't require BUILTIN rows (packaged clusters deploy fs/connector providers as directory plugins); assert PLUGIN_VERSION is NULL or non-empty
|
run buildall |
TPC-H: Total hot run time: 29600 ms |
TPC-DS: Total hot run time: 177956 ms |
ClickBench: Total hot run time: 25.16 s |
FE Regression Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Requesting changes for three additional plugin-inventory issues documented inline. I did not duplicate the eight existing threads. The current head fixes the textual registry source, SQL NULL transport, defining-jar lookup, rejected-handle cleanup, and the packaged BUILTIN assumption; the existing name-contract, canonical-publication, callback, version-layout, and result-test threads remain applicable.
Review checkpoints:
- Goal and behavior: this adds an ADMIN-only, five-column, per-connected-FE factory inventory. FE/BE schema order, projection, nullable transport, batching, privilege gating, and connected-FE routing are consistent, but one public-SPI compatibility path and two metadata/publication paths remain incorrect.
- Scope and compatibility: the schema table is additive and the patch is focused. Adding the public
description()default is not migration-safe for every precompiled factory. During a mixed new-FE/old-BE rollout, an old BE can still select its pre-feature dummy scanner and return an empty result until a supporting BE executes the table. - Lifecycle and concurrency: the concurrent maps, copy-on-write snapshots, lifecycle lock, and accepted-handle discard path show no new race or deadlock. Filesystem sensitive-key discovery still mutates runtime and inventory state in inconsistent orders on callback failure.
- FE/BE and persistence: column identity/order, SQL NULL insertion, current-FE RPC behavior, and ADMIN-empty behavior are correct. The change does not add persisted state, configuration migration, or data writes.
- Performance and observability: metadata is snapshotted at load time and the registry sorts a small inventory; no material hot-path concern found. Failure paths are logged, but the uncaught linkage error bypasses that isolation.
- Tests and validation: changed tests do not cover an old-SPI default-method collision, a package-section manifest version, or filesystem sensitive-key callback failure. Review validation was static only as required by the review contract; no local build or test command was run. Hosted compile/FE UT/checkstyle checks are green; macOS BE UT failed before tests because the runner used JDK 25 while Doris requires JDK 17, and several TeamCity failures could not be inspected because their details endpoint requires authorization.
| // Snapshot self-reported metadata before publishing the provider | ||
| // so one throwing implementation is rejected cleanly instead of | ||
| // aborting startup or being active without an inventory row. | ||
| PluginRegistry.getInstance().registerBuiltin(PLUGIN_FAMILY, p); |
There was a problem hiding this comment.
[P2] Keep registry publication after sensitive-key discovery. sensitivePropertyKeys() is overridable provider code and can throw; because this row is inserted first, the catch below skips the provider but cannot remove the record, so information_schema.plugins permanently advertises a BUILTIN provider that the runtime manager rejected. Snapshot the sensitive keys before mutating either store (and honor one registration decision atomically). The external loop needs the same failure ordering/cleanup: it currently adds the provider before this callback and before the registry row.
… grant - FileSystemPluginManager: snapshot self-reported sensitive keys before mutating any store on both builtin and external paths, so a throwing provider is rejected atomically (no phantom inventory row, no active provider without a row) - DirectoryPluginRuntimeManager: catch LinkageError alongside RuntimeException when reading factory name/description, keeping a broken-classpath jar's failure isolated to that plugin; honor jar-spec package manifest sections when resolving Implementation-Version - test_plugins_schema: grant SELECT_PRIV on regression_test (and cloud cluster usage) to the fresh non-admin user, since the JDBC URL carries a default database and the connection is refused without it
|
run buildall |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
FE UT Coverage ReportIncrement line coverage |
|
/review |
There was a problem hiding this comment.
Requesting changes for one additional plugin-version correctness issue documented inline. I did not duplicate the eleven existing inline threads or the new-FE/old-BE dummy-scanner behavior already recorded in review 4718985065.
Review checkpoints:
- Goal and proof: the intended result is an ADMIN-only, five-column inventory of successfully loaded plugin factories on the FE to which the session is connected. The current FE/BE schema, projection, connected-FE routing, batching, privilege filter, and nullable version materialization are consistent, but built-in version provenance is still wrong for same-package factories in different classpath jars.
- Scope: the schema-table addition is focused and load-time metadata keeps plugin code off the query path. The accepted issue can reuse the defining-jar manifest logic already added for external plugins.
- Concurrency: the concurrent maps, copy-on-write provider lists, immutable registry records, and loader lifecycle lock introduce no new race, deadlock, or lock-heavy path. Cross-store publication concerns that remain are already covered by the canonical-decision review thread.
- Lifecycle: family-conflict handles are now removed and their classloaders closed through
discard; no reload or unload path is added. I found no additional retained-handle or shutdown issue. - Configuration: no configuration item or dynamic-update behavior is added.
- Compatibility: the current-version Thrift enum, column order, and types align. The supported BE-before-FE upgrade direction avoids the already-known old-BE dummy path. Existing threads still cover the external-name migration and old-SPI default-method compatibility problems; this review does not duplicate them.
- Parallel paths and special conditions: built-in and external registration was traced across authentication, connector, filesystem, and lineage. Remaining invalid/duplicate/publication cases map to existing threads; RPC and callback failures otherwise propagate or are isolated according to the surrounding loader contracts.
- Test coverage and results: the new unit and regression tests exercise basic registration, sorting, main-manifest versions, missing versions, schema shape, source labels, uniqueness, filesystem filtering, and non-ADMIN behavior. Existing threads already request deterministic generated results, a guaranteed SQL-NULL case, old-SPI and external-layout fixtures, and family failure tests. No test covers the distinct built-in split-package version case reported here.
- Observability: registry snapshots and startup logs are lightweight, and query-time listing does not execute plugin code. Reporting another jar's version is itself an observability correctness failure.
- Persistence, transactions, and data writes: none are introduced; there is no EditLog, failover, transactionality, or storage-version impact.
- FE/BE variable flow: the new request enum and five cells are handled at all current-version send/receive points; no other FE-to-BE variable is added.
- Performance: registry size is small, sorting occurs on a snapshot, and the scanner batches locally; no material CPU, memory, or I/O regression was found.
- Other concerns: no additional security issue or user-specified focus point was present. ADMIN gating matches the stated visibility contract.
Validation was static as required by the review prompt; I did not run local builds or tests.
| * @return the version, or null when unavailable (e.g. classes directory) | ||
| */ | ||
| public static String implementationVersionOf(Class<?> clazz) { | ||
| Package pkg = clazz.getPackage(); |
There was a problem hiding this comment.
[P2] Read each built-in version from its defining jar. Class.getPackage() returns the one Package object for a package name/classloader; with two ServiceLoader factories in the same package but different jars, the first class to define that package fixes its manifest metadata, so the later plugin row reports the first jar's Implementation-Version. That violates this method's defining-jar contract and differs from the external path, which already resolves CodeSource. Reuse the defining-jar manifest helper here and cover two same-package factory jars with different versions.
TPC-H: Total hot run time: 29687 ms |
TPC-DS: Total hot run time: 177550 ms |
ClickBench: Total hot run time: 25.1 s |
2 participants
What problem does this PR solve?
Kernel plugin families (filesystem, connector, authentication, lineage) currently have no unified, queryable inventory: to know which plugins are loaded on an FE, one has to read logs. This PR adds a unified
information_schema.pluginssystem table listing all successfully loaded kernel plugins of the currently connected FE.Columns:
PLUGIN_NAME,PLUGIN_TYPE,PLUGIN_VERSION,SOURCE,DESCRIPTION.Design points:
(plugin_type, plugin_name)is the primary key, enforced at load time: the first registration wins and is never silently overridden, so a directory jar can never displace a built-in plugin.name()/description()are snapshotted once at load time; querying the table never executes plugin code. A broken self-report fails the load.[A-Za-z0-9._-]); an invalid name fails the load for external plugins.PLUGIN_VERSIONcomes from the jar MANIFESTImplementation-Version(NULL when unavailable), never from plugin interface methods.catalog_meta_cache_statistics), so per-FE jar deployment can be audited connection by connection.Implementation:
fe-extension-spi: adddefault String description()toPluginFactory(empty by default, zero migration cost).fe-extension-loader: new process-widePluginRegistry(load-time snapshots only) andPluginNamesvalidation;DirectoryPluginRuntimeManagersnapshots name/description and readsImplementation-Versionfrom the jar containing the factory class.SchemaTabledefinition +MetadataGeneratordata source; thriftTSchemaTableName.PLUGINS. Reuses the existing MySQL-compatTSchemaTableType.SCH_PLUGINS.schema_plugins_scannerwired intoschema_scanner.Example:
Release note
Add
information_schema.pluginssystem table listing loaded kernel plugins (filesystem/connector/authentication/lineage) of the currently connected FE; requires ADMIN privilege.Check List (For Author)
Test
PluginRegistryTest,DirectoryPluginRuntimeManagerMetadataTestregression-test/suites/query_p0/schema_table/test_plugins_schema.groovyBehavior changed:
information_schema.pluginstable (additive only).Does this need documentation?
information_schema.pluginsshould be documented as a new system table.