-
Notifications
You must be signed in to change notification settings - Fork 2.2k
[Enhancement] introduce fe extensions #67255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
StarRocks currently features a plugin mechanism that enables users to install and manage plugins via SQL, such as audit log plugins or data integration plugins. However, the existing plugin mechanism primarily focuses on plugin installation/uninstallation and single-function implementation. It lacks the capability for deep extension into internal system behaviors and cannot uniformly manage plugin lifecycles, dependencies, and resources. This makes it difficult to meet more complex business scenarios and expansion requirements. Signed-off-by: stdpain <[email protected]>
24419f4 to
25e3917
Compare
|
|
Better to add docs to describe
|
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]✅ pass : 490 / 535 (91.59%) file detail
|
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
see #67270 |
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package com.starrocks.extension; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to move this extension to a new module like fe/fe-extension ?
|
@cursor review |
| } | ||
| } | ||
| return classes; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URLClassLoader closed before extension classes can load dependencies
The URLClassLoader in scanJar() is closed via try-with-resources at line 75, but the loaded classes are returned and later instantiated in ExtensionManager.loadExtensions(). When the extension's constructor or onLoad() method tries to use other classes from the same JAR that weren't already loaded during scanning, class loading will fail because the classloader is closed. The URLClassLoader needs to remain open for the lifetime of the extension, or all required classes must be loaded before closing.
Additional Locations (1)
| String normalized = | ||
| classFile.toString().replace(File.separatorChar, '/'); | ||
| return BASE_PACKAGES.stream().anyMatch(normalized::contains); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Package filter uses dots but path uses slashes
The isUnderBasePackage() method compares file paths (which use slashes like com/starrocks/extension) against BASE_PACKAGES which use dot notation (like com.starrocks.extension). The normalized.contains() check will never match because "path/com/starrocks/extension/Class.class".contains("com.starrocks.extension") returns false due to the slash vs dot mismatch. This causes the scanner to filter out all class files, preventing any extensions from being discovered.




Why I'm doing:
StarRocks currently features a plugin mechanism that enables users to install and manage plugins via SQL, such as audit log plugins or data integration plugins.
However, the existing plugin mechanism primarily focuses on plugin installation/uninstallation and single-function implementation. It lacks the capability for deep extension into internal system behaviors and cannot uniformly manage plugin lifecycles, dependencies, and resources. This makes it difficult to meet more complex business scenarios and expansion requirements.
What I'm doing:
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check:
Note
Adds a lightweight extension system and decouples Gson configuration.
ExtensionManager,SRModule,StarRocksExtension, and scanners (JarScanner,DirectoryClassPathScanner) to auto-load extensions; FE now callsExtensionManager.loadExtensionsFromDir(Config.ext_dir)at startupConfigkeyext_dir(defaults to$STARROCKS_HOME/lib)DefaultExtensionContextproviding defaults (WarehouseManager,ResourceUsageMonitor,BaseSlotManagerviaSlotManager,IGsonBuilderFactory)GlobalStateMgrnow obtainsWarehouseManager,ResourceUsageMonitor, andBaseSlotManagerfromExtensionManagerinstead of direct instantiationIGsonBuilderFactoryandDefaultGsonBuilderFactory; moves adapters intopersist.gson.internal/*;GsonUtilsdelegates builder creation to the factoryUtFrameUtils) loads extensions from classpath (target/classes)Written by Cursor Bugbot for commit 25e3917. This will update automatically on new commits. Configure here.