You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2019/10/16 06:45:51 UTC

[lucene-solr] branch jira/SOLR-13822 updated: More tests and made more robust

This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch jira/SOLR-13822
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/jira/SOLR-13822 by this push:
     new 7f9c3d5  More tests and made more robust
7f9c3d5 is described below

commit 7f9c3d59996fe038606b08d37036dc28526c5050
Author: noble <no...@apache.org>
AuthorDate: Wed Oct 16 17:43:33 2019 +1100

    More tests and made more robust
---
 .../src/java/org/apache/solr/api/AnnotatedApi.java |  3 +
 .../src/java/org/apache/solr/core/SolrCore.java    |  3 +-
 .../org/apache/solr/handler/SolrConfigHandler.java | 27 ++++-----
 .../src/java/org/apache/solr/pkg/PackageAPI.java   |  4 ++
 .../java/org/apache/solr/pkg/PackageListeners.java | 21 ++++---
 .../java/org/apache/solr/pkg/PackageLoader.java    | 24 ++++----
 .../org/apache/solr/pkg/PackagePluginHolder.java   | 14 ++++-
 .../src/test/org/apache/solr/pkg/TestPackages.java | 69 ++++++++++++++++++----
 8 files changed, 115 insertions(+), 50 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java b/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
index e9073ae..eca2283 100644
--- a/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
+++ b/solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
@@ -239,10 +239,13 @@ public class AnnotatedApi extends Api implements PermissionNameProvider {
 
 
       } catch (SolrException se) {
+        log.error("Error executing command  ", se);
         throw se;
       } catch (InvocationTargetException ite) {
+        log.error("Error executing command ", ite);
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, ite.getCause());
       } catch (Exception e) {
+        log.error("Error executing command : ", e);
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
       }
 
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index 59c9a7a..7b6d55e 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -287,7 +287,8 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
       return resourceLoader;
     }
     PackageLoader.Package aPackage = coreContainer.getPackageLoader().getPackage(pkg);
-    return aPackage.getLatest().getLoader();
+    PackageLoader.Package.Version latest = aPackage.getLatest();
+    return latest.getLoader();
   }
 
   /**
diff --git a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
index 2085221..03b9600 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -151,7 +151,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
 
   public static boolean getImmutable(SolrCore core) {
     NamedList configSetProperties = core.getConfigSetProperties();
-    if(configSetProperties == null) return false;
+    if (configSetProperties == null) return false;
     Object immutable = configSetProperties.get(IMMUTABLE_CONFIGSET_ARG);
     return immutable != null ? Boolean.parseBoolean(immutable.toString()) : false;
   }
@@ -248,11 +248,12 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
               if (map != null) {
                 Object o = map.get(componentName);
                 val.put(parts.get(1), makeMap(componentName, o));
-                if(req.getParams().getBool("meta", false)){
+                if (req.getParams().getBool("meta", false)) {
+                  List<PackageListeners.Listener> listeners = req.getCore().getPackageListeners().getListeners();
                   for (PackageListeners.Listener listener :
-                      req.getCore().getPackageListeners().getListeners()) {
+                      listeners) {
                     PluginInfo info = listener.pluginInfo();
-                    if(info.type.equals(parts.get(1)) && info.name.equals(componentName)){
+                    if (info.type.equals(parts.get(1)) && info.name.equals(componentName)) {
                       if (o instanceof Map) {
                         Map m1 = (Map) o;
                         m1.put("_packageinfo_", listener.getPackageVersion());
@@ -261,9 +262,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
                   }
                 }
               }
-
             }
-
             resp.add("config", val);
           }
         }
@@ -440,7 +439,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
 
       List errs = CommandOperation.captureErrors(ops);
       if (!errs.isEmpty()) {
-        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing params", errs);
+        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST, "error processing params", errs);
       }
 
       SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -503,8 +502,8 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
       }
       List errs = CommandOperation.captureErrors(ops);
       if (!errs.isEmpty()) {
-        log.error("ERRROR:" +Utils.toJSONString(errs));
-        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST,"error processing commands", errs);
+        log.error("ERROR:" + Utils.toJSONString(errs));
+        throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST, "error processing commands", errs);
       }
 
       SolrResourceLoader loader = req.getCore().getResourceLoader();
@@ -542,8 +541,8 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
       op.getMap(PluginInfo.INVARIANTS, null);
       op.getMap(PluginInfo.APPENDS, null);
       if (op.hasError()) return overlay;
-      if(info.clazz == PluginBag.RuntimeLib.class) {
-        if(!PluginBag.RuntimeLib.isEnabled()){
+      if (info.clazz == PluginBag.RuntimeLib.class) {
+        if (!PluginBag.RuntimeLib.isEnabled()) {
           op.addError("Solr not started with -Denable.runtime.lib=true");
           return overlay;
         }
@@ -575,7 +574,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
 
     private boolean pluginExists(SolrConfig.SolrPluginInfo info, ConfigOverlay overlay, String name) {
       List<PluginInfo> l = req.getCore().getSolrConfig().getPluginInfos(info.clazz.getName());
-      for (PluginInfo pluginInfo : l) if(name.equals( pluginInfo.name)) return true;
+      for (PluginInfo pluginInfo : l) if (name.equals(pluginInfo.name)) return true;
       return overlay.getNamedPlugins(info.getCleanTag()).containsKey(name);
     }
 
@@ -586,7 +585,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
         try {
           req.getCore().createInitInstance(new PluginInfo(SolrRequestHandler.TYPE, op.getDataMap()), expected, clz, "");
         } catch (Exception e) {
-          log.error("Error checking plugin : ",e);
+          log.error("Error checking plugin : ", e);
           op.addError(e.getMessage());
           return false;
         }
@@ -692,7 +691,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
           c == '_' ||
           c == '-' ||
           c == '.'
-          ) continue;
+      ) continue;
       else {
         return formatString("''{0}'' name should only have chars [a-zA-Z_-.0-9] ", s);
       }
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java b/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
index 0267f37..afa582d 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageAPI.java
@@ -310,9 +310,13 @@ public class PackageAPI {
     }
 
     private void syncToVersion(int expectedVersion) {
+      int origVersion = pkgs.znodeVersion;
       for (int i = 0; i < 10; i++) {
         log.debug("my version is {} , and expected version {}", pkgs.znodeVersion, expectedVersion);
         if (pkgs.znodeVersion >= expectedVersion) {
+          if(origVersion < pkgs.znodeVersion){
+            packageLoader.refreshPackageConf();
+          }
           return;
         }
         try {
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageListeners.java b/solr/core/src/java/org/apache/solr/pkg/PackageListeners.java
index c6ebae5..3ac12ef 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageListeners.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageListeners.java
@@ -17,27 +17,33 @@
 
 package org.apache.solr.pkg;
 
-import java.lang.ref.WeakReference;
+import java.lang.invoke.MethodHandles;
+import java.lang.ref.Reference;
+import java.lang.ref.SoftReference;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
 
 import org.apache.solr.core.PluginInfo;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class PackageListeners {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   // this registry only keeps a weak reference because it does not want to
   // cause a memory leak if the listener forgets to unregister itself
-  private List<WeakReference<Listener>> listeners = new ArrayList<>();
+  private List<Reference<Listener>> listeners = new ArrayList<>();
 
   public synchronized void addListener(Listener listener) {
-    listeners.add(new WeakReference<>(listener));
+    listeners.add(new SoftReference<>(listener));
 
   }
 
   public synchronized void removeListener(Listener listener) {
-    Iterator<WeakReference<Listener>> it = listeners.iterator();
+    Iterator<Reference<Listener>> it = listeners.iterator();
     while (it.hasNext()) {
-      WeakReference<Listener> ref = it.next();
+      Reference<Listener> ref = it.next();
       Listener pkgListener = ref.get();
       if(pkgListener == null || pkgListener == listener){
         it.remove();
@@ -54,7 +60,7 @@ public class PackageListeners {
   }
 
   private synchronized void invokeListeners(PackageLoader.Package pkg) {
-    for (WeakReference<Listener> ref : listeners) {
+    for (Reference<Listener> ref : listeners) {
       Listener listener = ref.get();
       if (listener != null && listener.packageName().equals(pkg.name())) {
         listener.changed(pkg);
@@ -64,12 +70,11 @@ public class PackageListeners {
 
   public List<Listener> getListeners(){
     List<Listener> result = new ArrayList<>();
-    for (WeakReference<Listener> ref : listeners) {
+    for (Reference<Listener> ref : listeners) {
       Listener l = ref.get();
       if(l != null){
         result.add(l);
       }
-
     }
     return result;
   }
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java b/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
index 7efcc88..d131647 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackageLoader.java
@@ -76,6 +76,7 @@ public class PackageLoader {
 
     List<Package> updated = new ArrayList<>();
     Map<String, List<PackageAPI.PkgVersion>> modified = getModified(myCopy, packageAPI.pkgs);
+
     for (Map.Entry<String, List<PackageAPI.PkgVersion>> e : modified.entrySet()) {
       if (e.getValue() != null) {
         Package p = packageClassLoaders.get(e.getKey());
@@ -95,7 +96,6 @@ public class PackageLoader {
     }
     for (SolrCore core : coreContainer.getCores()) {
       core.getPackageListeners().packagesUpdated(updated);
-
     }
   }
 
@@ -105,15 +105,18 @@ public class PackageLoader {
       List<PackageAPI.PkgVersion> versions = old.packages.get(e.getKey());
       if (versions != null) {
         if (!Objects.equals(e.getValue(), versions)) {
+          log.info("Package {} is modified ",e.getKey());
           changed.put(e.getKey(), e.getValue());
         }
       } else {
+        log.info("A new package: {} introduced", e.getKey());
         changed.put(e.getKey(), e.getValue());
       }
     }
     //some packages are deleted altogether
     for (String s : old.packages.keySet()) {
       if (!newPkgs.packages.keySet().contains(s)) {
+        log.info("Package: {} is removed althogether", s);
         changed.put(s, null);
       }
     }
@@ -122,10 +125,6 @@ public class PackageLoader {
 
   }
 
-  public SolrResourceLoader getResourceLoader(String pkg, String version) {
-    return null;
-  }
-
 
   public class Package {
     final String name;
@@ -148,6 +147,7 @@ public class PackageLoader {
       for (PackageAPI.PkgVersion v : modified) {
         Version version = myVersions.get(v.version);
         if (version == null) {
+          log.info("A new version: {} added for package: {} with artifacts {}", v.version,  this.name, v.files);
           myVersions.put(v.version, new Version(this, v));
           sortedVersions.add(v.version);
         }
@@ -159,6 +159,7 @@ public class PackageLoader {
       }
       for (String s : new HashSet<>(myVersions.keySet())) {
         if (!newVersions.contains(s)) {
+          log.info("version: {} is removed from package: {}", s,  this.name);
           sortedVersions.remove(s);
           myVersions.remove(s);
         }
@@ -166,8 +167,13 @@ public class PackageLoader {
 
       sortedVersions.sort(String::compareTo);
       if (sortedVersions.size() > 0) {
-        latest = sortedVersions.get(sortedVersions.size() - 1);
+        String latest = sortedVersions.get(sortedVersions.size() - 1);
+        if(!latest.equals(this.latest)){
+          log.info("version: {} is the new latest in package: {}", latest,  this.name);
+        }
+        this.latest = latest;
       } else {
+        log.error("latest version:  null");
         latest = null;
       }
 
@@ -225,7 +231,7 @@ public class PackageLoader {
 
         try {
           loader = new SolrResourceLoader(
-              "PACKAGE_LOADER:"+ parent.name()+ ":"+ version,
+              "PACKAGE_LOADER: "+ parent.name()+ ":"+ version,
               paths,
               coreContainer.getResourceLoader().getInstancePath(),
               coreContainer.getResourceLoader().getClassLoader());
@@ -244,11 +250,7 @@ public class PackageLoader {
 
       public SolrResourceLoader getLoader() {
         return loader;
-
       }
-
     }
   }
-
-
 }
diff --git a/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java b/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
index f0364c4..73e7f90 100644
--- a/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
+++ b/solr/core/src/java/org/apache/solr/pkg/PackagePluginHolder.java
@@ -68,11 +68,19 @@ public class PackagePluginHolder<T> extends PluginBag.PluginHolder<T> {
 
 
   private synchronized void reload(PackageLoader.Package pkg) {
-    if(pkgVersion != null && aPackage.getLatest() == pkgVersion ) return;
+    if (pkgVersion != null && pkg.getLatest() == pkgVersion) {
+      //I'm already using the latest classloder in the package. nothing to do
+      return;
+    }
 
-    if (inst != null) log.info("reloading plugin {} ", pluginInfo.name);
     PackageLoader.Package.Version newest = pkg.getLatest();
-    if(newest == null) return;
+    if (newest == null){
+      log.error("No latest version available for package : {}", pkg.name());
+      return;
+    }
+    log.info("loading plugin: {} -> {} using  package {}:{}",
+        pluginInfo.type, pluginInfo.name, pkg.name(), newest.getVersion());
+
     Object instance = SolrCore.createInstance(pluginInfo.className,
         pluginMeta.clazz, pluginMeta.getCleanTag(), core, newest.getLoader());
     PluginBag.initInstance(instance, pluginInfo);
diff --git a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
index 5de69cf..6c0c036 100644
--- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
+++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
@@ -68,6 +68,7 @@ public class TestPackages extends SolrCloudTestCase {
     try {
       String FILE1 = "/mypkg/runtimelibs.jar";
       String FILE2 = "/mypkg/runtimelibs_v2.jar";
+      String FILE3 = "/mypkg/runtimelibs_v3.jar";
       String COLLECTION_NAME = "testPluginLoadingColl";
       byte[] derFile = readFile("cryptokeys/pub_key512.der");
       cluster.getZkClient().makePath("/keys/exe", true);
@@ -149,7 +150,6 @@ public class TestPackages extends SolrCloudTestCase {
 
       //add the version using package API
       add.version = "1.1";
-      add.pkg = "mypkg";
       add.files = Arrays.asList(new String[]{FILE2});
       req.process(cluster.getSolrClient());
 
@@ -165,34 +165,77 @@ public class TestPackages extends SolrCloudTestCase {
           COLLECTION_NAME, "requestHandler", "/runtime",
           "mypkg", "1.1" );
 
-      /*executeReq( "/" + COLLECTION_NAME + "/get?wt=json", cluster.getRandomJetty(random()),
+      executeReq( "/" + COLLECTION_NAME + "/get?wt=json", cluster.getRandomJetty(random()),
           Utils.JSONCONSUMER,
-          Utils.makeMap("class", "org.apache.solr.core.RuntimeLibSearchComponent",
-              "Version","2"));
+          Utils.makeMap(  "Version","2"));
+
+
+      //now upload the third jar
+      postFileAndWait(cluster, "runtimecode/runtimelibs_v3.jar.bin", FILE3,
+          "a400n4T7FT+2gM0SC6+MfSOExjud8MkhTSFylhvwNjtWwUgKdPFn434Wv7Qc4QEqDVLhQoL3WqYtQmLPti0G4Q==");
+
+      add.version = "2.1";
+      add.files = Arrays.asList(new String[]{FILE3});
+      req.process(cluster.getSolrClient());
+
+      //now let's verify that the classes are updated
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "queryResponseWriter", "json1",
+          "mypkg", "2.1" );
+
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "searchComponent", "get",
+          "mypkg", "2.1" );
+
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "requestHandler", "/runtime",
+          "mypkg", "2.1" );
+
+      executeReq( "/" + COLLECTION_NAME + "/runtime?wt=json", cluster.getRandomJetty(random()),
+          Utils.JSONCONSUMER,
+          Utils.makeMap("Version","2"));
+
 
       PackageAPI.DelVersion delVersion = new PackageAPI.DelVersion();
       delVersion.pkg = "mypkg";
-      delVersion.version = "1.1";
-      new V2Request.Builder("/cluster/package")
+      delVersion.version = "1.0";
+      V2Request delete = new V2Request.Builder("/cluster/package")
           .withMethod(SolrRequest.METHOD.POST)
           .forceV2(true)
-          .withPayload(delVersion)
-          .build()
-          .process(cluster.getSolrClient());
+          .withPayload(Collections.singletonMap("delete", delVersion))
+          .build();
+      delete.process(cluster.getSolrClient());
 
       verifyCmponent(cluster.getSolrClient(),
           COLLECTION_NAME, "queryResponseWriter", "json1",
-          "mypkg", "1.0" );
+          "mypkg", "2.1" );
 
       verifyCmponent(cluster.getSolrClient(),
           COLLECTION_NAME, "searchComponent", "get",
-          "mypkg", "1.0" );
+          "mypkg", "2.1" );
 
       verifyCmponent(cluster.getSolrClient(),
           COLLECTION_NAME, "requestHandler", "/runtime",
-          "mypkg", "1.0" );
+          "mypkg", "2.1" );
+
+      delVersion.version = "2.1";
+      delete.process(cluster.getSolrClient());
+
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "queryResponseWriter", "json1",
+          "mypkg", "1.1" );
+
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "searchComponent", "get",
+          "mypkg", "1.1" );
+
+      verifyCmponent(cluster.getSolrClient(),
+          COLLECTION_NAME, "requestHandler", "/runtime",
+          "mypkg", "1.1" );
+
+
+
 
-*/
     } finally {
       cluster.shutdown();
     }