gortiz commented on code in PR #13930:
URL: https://github.com/apache/pinot/pull/13930#discussion_r1742062595


##########
pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginManager.java:
##########
@@ -229,22 +257,86 @@ private void initRecordReaderClassMap() {
   /**
    * Loads jars recursively
    * @param pluginName
-   * @param directory
+   * @param directory the directory of one plugin
    */
   public void load(String pluginName, File directory) {
-    LOGGER.info("Trying to load plugin [{}] from location [{}]", pluginName, 
directory);
-    Collection<File> jarFiles = FileUtils.listFiles(directory, new 
String[]{"jar"}, true);
-    Collection<URL> urlList = new ArrayList<>();
-    for (File jarFile : jarFiles) {
+    Path pluginPropertiesPath = 
directory.toPath().resolve(PINOUT_PLUGIN_PROPERTIES_FILE_NAME);
+    if (Files.isRegularFile(pluginPropertiesPath)) {
+      Properties pluginProperties = new Properties();
+      PinotPluginConfiguration config = null;
+      try (Reader reader = Files.newBufferedReader(pluginPropertiesPath)) {
+        pluginProperties.load(reader);
+        config = new PinotPluginConfiguration(pluginProperties);
+      } catch (IOException e) {
+        LOGGER.warn("Failed to load plugin properties from {}", 
pluginPropertiesPath, e);
+      }
+
+      final ClassLoader baseClassLoader = ClassLoader.getPlatformClassLoader();
+
+      Collection<URL> urlList;
+      try (Stream<Path> pluginClasspathEntries = 
Files.list(directory.toPath())) {
+        urlList = pluginClasspathEntries.map(p -> {
+          try {
+            return p.toUri().toURL();
+          } catch (MalformedURLException e) {
+            throw new RuntimeException(e);
+          }
+        }).collect(Collectors.toList());
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+
       try {
-        urlList.add(jarFile.toURI().toURL());
-      } catch (MalformedURLException e) {
-        LOGGER.error("Unable to load plugin [{}] jar file [{}]", pluginName, 
jarFile, e);
+        ClassRealm pluginRealm = _classWorld.newRealm(
+            pluginName,
+            baseClassLoader);
+        urlList.forEach(pluginRealm::addURL);
+
+        ClassRealm pinotRealm = _classWorld.getClassRealm(PINOT_REALMID);
+
+        // All packages to look up in pinot realm BEFORE itself
+        Stream<String> importedPinotPackages =
+            Stream.of("org.apache.pinot.spi"); // this works like a prefix, so 
ALL spi classes will be accessible
+        importedPinotPackages.forEach(p -> pluginRealm.importFrom(pinotRealm, 
p));
+
+        // Additional importForm as specified by the plugin configuration
+        if (config != null) {
+          config.getImportsFromPerRealm().forEach((r, ifs) -> {
+                try {
+                  ClassRealm cr = _classWorld.getRealm(r);
+                  ifs.forEach(i -> pluginRealm.importFrom(cr, i));
+                } catch (NoSuchRealmException e) {
+                  LOGGER.warn("{} realm does not exist", r);
+                }
+              }
+          );
+
+          // Important: parent is not the same as baseclassloader (see 
pluginRealm above)
+          // baseClassLoader is BEFORE self classloader (should be Platform 
class loader)
+          // parentClassLoader is AFTER self classloader
+          
config.getParentRealmId().map(_classWorld::getClassRealm).ifPresent(pluginRealm::setParentRealm);

Review Comment:
   My bad, it is registered in the _classWorld



-- 
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: commits-unsubscr...@pinot.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to