Seaven commented on a change in pull request #3267: [Bug] Fix some bugs of
install/uninstall plugins
URL: https://github.com/apache/incubator-doris/pull/3267#discussion_r404537885
##########
File path: fe/src/main/java/org/apache/doris/plugin/PluginMgr.java
##########
@@ -113,30 +140,47 @@ public PluginInfo installPlugin(InstallPluginStmt stmt)
throws IOException, User
* Dynamic uninstall plugin
*/
public PluginInfo uninstallPlugin(String name) throws IOException,
UserException {
- for (int i = 0; i < PluginType.MAX_PLUGIN_SIZE; i++) {
+ if (!checkDynamicPluginNameExist(name)) {
+ throw new DdlException("Plugin " + name + " does not exist");
+ }
+
+ for (int i = 0; i < PluginType.MAX_PLUGIN_TYPE_SIZE; i++) {
if (plugins[i].containsKey(name)) {
PluginLoader loader = plugins[i].get(name);
+ if (loader == null) {
+ // this is not a atomic operation, so even if
containsKey() is true,
+ // we may still get null object by get() method
+ continue;
+ }
- if (null != loader && loader.isDynamicPlugin()) {
- loader.pluginUninstallValid();
- loader.setStatus(PluginStatus.UNINSTALLING);
- // uninstall plugin
- loader.uninstall();
- plugins[i].remove(name);
-
- loader.setStatus(PluginStatus.UNINSTALLED);
- return loader.getPluginInfo();
+ if (!loader.isDynamicPlugin()) {
+ throw new DdlException("Only support uninstall dynamic
plugins");
}
+
+ loader.pluginUninstallValid();
+ loader.setStatus(PluginStatus.UNINSTALLING);
+ // uninstall plugin
+ loader.uninstall();
+ plugins[i].remove(name);
+ loader.setStatus(PluginStatus.UNINSTALLED);
+ removeDynamicPluginName(name);
+
+ // do not get plugin info by calling loader.getPluginInfo().
That method will try to
+ // reload the plugin properties from source if this plugin is
not installed successfully.
+ // Here we only need the plugin's name for persisting.
+ // TODO(cmy): This is a bad design to couple the persist info
with PluginInfo, but for
+ // the compatibility, I till use this method.
Review comment:
It is a good idea not to use PluginInfo for persist
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
