morningman commented on code in PR #70338:
URL: https://github.com/apache/doris/pull/70338#discussion_r4496067570


##########
fe/fe-core/src/main/java/org/apache/doris/connector/ConnectorPluginManager.java:
##########
@@ -314,9 +332,21 @@ public void loadPlugins(List<Path> pluginRoots) {
                 runtimeManager.discard(handle.getPluginName());
                 continue;
             }
+            ConnectorProvider provider = handle.getFactory();
+            // Snapshot the security surface before publishing anything: it is 
plugin code, a connector may
+            // build it through its own ServiceLoader, and one bad plugin dir 
must not stop FE.
+            ConnectorSecuritySurface securitySurface;
+            try {
+                securitySurface = provider.securitySurface();
+            } catch (RuntimeException | LinkageError e) {

Review Comment:
   Fixed in ae2cebe750f. Confirmed against the code: the Hive, Iceberg and 
Paimon surfaces call `MetaStoreProviders.sensitivePropertyKeys()`, whose static 
initializer iterates a `ServiceLoader` over the plugin's jars. A descriptor 
naming a backend that cannot be loaded throws `ServiceConfigurationError`, an 
`Error` a static initializer rethrows unwrapped, so it escaped this 
`RuntimeException | LinkageError` catch and the builtin path's 
`RuntimeException` catch, and `Env.initConnectorPluginManager()` catches 
nothing: a stale jar in a connector's `lib/` stopped FE at startup, where 
before this PR only that connector's catalogs failed. Both catches now include 
`ServiceConfigurationError` (the builtin one matches the construction catch 
above it), so the provider is skipped and logged like any other surface failure.
   
   
`ConnectorPluginManagerTest#pluginWhoseSecuritySurfaceFailsInItsOwnServiceLoaderIsSkippedNotFatal`
 (a plugin directory whose `lib/` jar names a missing backend; the plugin next 
to it still loads) and 
`#classpathProviderWhoseSecuritySurfaceFailsInItsOwnServiceLoaderIsSkippedNotFatal`
 fail on b0bfd3c1b10 with that error and pass now; dropping it from either 
catch fails only that path's case.
   



-- 
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]

Reply via email to