Copilot commented on code in PR #19262:
URL: https://github.com/apache/pinot/pull/19262#discussion_r3788522363
##########
pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginManager.java:
##########
@@ -503,6 +502,93 @@ public synchronized Set<ClassLoader>
getPluginClassLoaders() {
return Collections.unmodifiableSet(result);
}
+ /// Discovers service providers of the given service type via
`ServiceLoader`, first from the thread context
+ /// classloader (the application classpath in a standard deployment), then
from every plugin classloader (see
+ /// [#getPluginClassLoaders()]), in that order.
+ ///
+ /// Providers are de-duplicated by fully-qualified class name across
classloaders: overlapping classpaths (e.g.
+ /// fat-jar + plugin realm) can surface the same provider through multiple
loaders, and the first sighting wins.
+ /// A same-named provider whose `Class` object differs from the first
sighting indicates version skew between
+ /// classloaders; it is logged at WARN and skipped.
+ ///
+ /// Each returned entry pairs the provider instance with a human-readable
description of the classloader it was
+ /// discovered from, for use in call-site log and error messages.
Enumeration failures
+ /// ([ServiceConfigurationError] — malformed descriptors, provider classes
that cannot be found, are not subtypes
+ /// of the service type, or fail to construct) fail fast as
[IllegalStateException] carrying the source
+ /// description, with the original error preserved as the cause.
Per-provider validation and registration policy
+ /// stay with the caller.
+ ///
+ /// Returns an unmodifiable, freshly computed list — never null, empty when
no providers are found. Thread-safe:
+ /// each call performs a fresh enumeration using only method-local state
(the plugin classloader snapshot is taken
+ /// under the existing lock), and the result reflects the calling thread's
context classloader.
+ ///
+ /// Call after all plugins have been loaded; plugin classloaders registered
after this call returns are not
+ /// searched.
+ public <S> List<ServiceProvider<S>> loadServiceProviders(Class<S>
serviceClass) {
+ List<ServiceProvider<S>> providers = new ArrayList<>();
+ Map<String, Class<?>> seenProviderClasses = new HashMap<>();
+ collectServiceProviders(ServiceLoader.load(serviceClass), serviceClass,
"thread context classloader", providers,
+ seenProviderClasses);
Review Comment:
The `source` for thread-context discovery is currently just the generic
string "thread context classloader", which makes error messages less actionable
when multiple classloaders are involved (and the method Javadoc says the source
describes the classloader). Including the actual context ClassLoader instance
in the source string would provide much better diagnostics.
--
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]
