You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2020/09/19 05:22:32 UTC

[lucene-solr] 04/10: Revert "SOLR-14151: Bug fixes"

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

ishan pushed a commit to branch jira/solr-14151-revert-8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 96720ff99db9baf2db4b55dfef6e918695a1ff9b
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sat Sep 19 10:49:29 2020 +0530

    Revert "SOLR-14151: Bug fixes"
    
    This reverts commit b46285c686faa672ccf4c24a7812c49b7ad8b939.
---
 .../apache/solr/cloud/ZkSolrResourceLoader.java    |  6 ---
 .../org/apache/solr/core/ConfigSetService.java     |  3 +-
 .../java/org/apache/solr/core/CoreContainer.java   | 21 ++++------
 .../src/java/org/apache/solr/core/SolrCore.java    | 21 +++++-----
 .../src/java/org/apache/solr/core/SolrCores.java   |  7 +---
 .../org/apache/solr/core/SolrResourceLoader.java   | 47 +---------------------
 .../java/org/apache/solr/handler/IndexFetcher.java |  2 +-
 .../org/apache/solr/handler/SchemaHandler.java     |  6 +--
 .../org/apache/solr/handler/SolrConfigHandler.java |  2 +-
 .../java/org/apache/solr/schema/IndexSchema.java   |  2 +-
 .../java/org/apache/solr/schema/SchemaManager.java |  4 +-
 .../src/test/org/apache/solr/pkg/TestPackages.java |  2 +
 12 files changed, 31 insertions(+), 92 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
index cde48f1..4eec58f 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
@@ -27,7 +27,6 @@ import java.util.Properties;
 import org.apache.solr.common.SolrException.ErrorCode;
 import org.apache.solr.common.cloud.ZkConfigManager;
 import org.apache.solr.common.cloud.ZooKeeperException;
-import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrResourceLoader;
 import org.apache.solr.core.SolrResourceNotFoundException;
 import org.apache.solr.schema.ZkIndexSchemaReader;
@@ -159,9 +158,4 @@ public class ZkSolrResourceLoader extends SolrResourceLoader {
   }
 
   public ZkIndexSchemaReader getZkIndexSchemaReader() { return zkIndexSchemaReader; }
-
-  @Override
-  public CoreContainer getCoreContainer() {
-    return zkController.getCoreContainer();
-  }
 }
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
index 8a5f9c7..76bc0c1 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
@@ -81,7 +81,8 @@ public abstract class ConfigSetService {
               ) ? false: true;
 
       SolrConfig solrConfig = createSolrConfig(dcore, coreLoader, trusted);
-      return new ConfigSet(configSetName(dcore), solrConfig, force -> createIndexSchema(dcore, solrConfig, force), properties, trusted);
+      IndexSchema indexSchema = createIndexSchema(dcore, solrConfig, false);
+      return new ConfigSet(configSetName(dcore), solrConfig, force -> indexSchema, properties, trusted);
     } catch (Exception e) {
       throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
           "Could not load conf for core " + dcore.getName() +
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index df9eef9..3e9b424 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1611,13 +1611,6 @@ public class CoreContainer {
   public void reload(String name) {
     reload(name, null);
   }
-  public void reload(String name, UUID coreId, boolean async) {
-    if(async) {
-      runAsync(() -> reload(name, coreId));
-    } else {
-      reload(name, coreId);
-    }
-  }
   /**
    * Recreates a SolrCore.
    * While the new core is loading, requests will continue to be dispatched to
@@ -1632,8 +1625,13 @@ public class CoreContainer {
       throw new AlreadyClosedException();
     }
     SolrCore newCore = null;
-    SolrCore core = solrCores.getCoreFromAnyList(name, false, coreId);
+    SolrCore core = solrCores.getCoreFromAnyList(name, false);
     if (core != null) {
+      if(coreId != null && core.uniqueId != coreId) {
+        //trying to reload an already unloaded core
+        return;
+      }
+
       // The underlying core properties files may have changed, we don't really know. So we have a (perhaps) stale
       // CoreDescriptor and we need to reload it from the disk files
       CoreDescriptor cd = reloadCoreDescriptor(core.getCoreDescriptor());
@@ -1863,9 +1861,6 @@ public class CoreContainer {
     return cfg.getCoreRootDirectory();
   }
 
-  public SolrCore getCore(String name) {
-    return getCore(name, null);
-  }
   /**
    * Gets a core by name and increase its refcount.
    *
@@ -1874,10 +1869,10 @@ public class CoreContainer {
    * @throws SolrCoreInitializationException if a SolrCore with this name failed to be initialized
    * @see SolrCore#close()
    */
-  public SolrCore getCore(String name, UUID id) {
+  public SolrCore getCore(String name) {
 
     // Do this in two phases since we don't want to lock access to the cores over a load.
-    SolrCore core = solrCores.getCoreFromAnyList(name, true, id);
+    SolrCore core = solrCores.getCoreFromAnyList(name, true);
 
     // If a core is loaded, we're done just return it.
     if (core != null) {
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 0c5bec2..f7533cb 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -225,9 +225,9 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
   private final RecoveryStrategy.Builder recoveryStrategyBuilder;
   private IndexReaderFactory indexReaderFactory;
   private final Codec codec;
-  private final ConfigSet configSet;
   private final MemClassLoader memClassLoader;
   //singleton listener for all packages used in schema
+  private final PackageListeningClassLoader schemaPluginsLoader;
 
   private final List<Runnable> confListeners = new CopyOnWriteArrayList<>();
 
@@ -272,6 +272,9 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
   public PackageListeners getPackageListeners() {
     return packageListeners;
   }
+  public PackageListeningClassLoader getSchemaPluginsLoader() {
+    return schemaPluginsLoader;
+  }
 
   static int boolean_query_max_clause_count = Integer.MIN_VALUE;
 
@@ -935,12 +938,15 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
     final CountDownLatch latch = new CountDownLatch(1);
     try {
       this.coreContainer = coreContainer;
-      this.configSet = configSet;
       this.coreDescriptor = Objects.requireNonNull(coreDescriptor, "coreDescriptor cannot be null");
       setName(coreDescriptor.getName());
 
       this.solrConfig = configSet.getSolrConfig();
       this.resourceLoader = configSet.getSolrConfig().getResourceLoader();
+      schemaPluginsLoader = new PackageListeningClassLoader(coreContainer, resourceLoader,
+              solrConfig::maxPackageVersion,
+              () -> setLatestSchema(configSet.getIndexSchema()));
+      this.packageListeners.addListener(schemaPluginsLoader);
       IndexSchema schema = configSet.getIndexSchema();
 
       this.configSetProperties = configSet.getProperties();
@@ -2781,11 +2787,6 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
     return memClassLoader;
   }
 
-  public void fetchLatestSchema() {
-    IndexSchema schema =  configSet.getIndexSchema(true);
-    setLatestSchema(schema);
-  }
-
   public interface RawWriter {
     default String getContentType() {
       return BinaryResponseParser.BINARY_CONTENT_TYPE;
@@ -3098,7 +3099,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
       SolrZkClient zkClient = cc.getZkController().getZkClient();
       int solrConfigversion, overlayVersion, managedSchemaVersion = 0;
       SolrConfig cfg = null;
-      try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true, coreId)) {
+      try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true)) {
         if (solrCore == null || solrCore.isClosed() || solrCore.getCoreContainer().isShutDown()) return;
         cfg = solrCore.getSolrConfig();
         solrConfigversion = solrCore.getSolrConfig().getOverlay().getZnodeVersion();
@@ -3119,7 +3120,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
         if (configHandler.getReloadLock().tryLock()) {
 
           try {
-            cc.reload(coreName, coreId, false);
+            cc.reload(coreName, coreId);
           } catch (SolrCoreState.CoreIsClosedException e) {
             /*no problem this core is already closed*/
           } finally {
@@ -3132,7 +3133,7 @@ public final class SolrCore implements SolrInfoBean, SolrMetricProducer, Closeab
         return;
       }
       //some files in conf directory may have  other than managedschema, overlay, params
-      try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true, coreId)) {
+      try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true)) {
         if (solrCore == null || solrCore.isClosed() || cc.isShutDown()) return;
         for (Runnable listener : solrCore.confListeners) {
           try {
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java
index d9c1539..d18e3a1 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCores.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java
@@ -34,7 +34,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -283,19 +282,15 @@ class SolrCores {
       return ret;
     }
   }
-  SolrCore  getCoreFromAnyList(String name, boolean incRefCount) {
-    return getCoreFromAnyList(name, incRefCount, null);
-  }
 
   /* If you don't increment the reference count, someone could close the core before you use it. */
-  SolrCore  getCoreFromAnyList(String name, boolean incRefCount, UUID coreId) {
+  SolrCore  getCoreFromAnyList(String name, boolean incRefCount) {
     synchronized (modifyLock) {
       SolrCore core = cores.get(name);
 
       if (core == null && getTransientCacheHandler() != null) {
         core = getTransientCacheHandler().getCore(name);
       }
-      if(core != null && coreId != null && coreId != core.uniqueId) return null;
 
       if (core != null && incRefCount) {
         core.open();
diff --git a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
index ed97024..fa963b3 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -44,7 +44,6 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.SolrClassLoader;
 import org.apache.solr.handler.component.SearchComponent;
 import org.apache.solr.handler.component.ShardHandlerFactory;
-import org.apache.solr.pkg.PackageListeningClassLoader;
 import org.apache.solr.request.SolrRequestHandler;
 import org.apache.solr.response.QueryResponseWriter;
 import org.apache.solr.rest.RestManager;
@@ -60,7 +59,7 @@ import org.slf4j.LoggerFactory;
 /**
  * @since solr 1.3
  */
-public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassLoader, SolrCoreAware  {
+public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassLoader {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private static final String base = "org.apache.solr";
@@ -77,30 +76,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
   protected URLClassLoader classLoader;
   private final Path instanceDir;
   private String dataDir; // gone in 9.0
-  private String coreName;
-  private UUID coreId;
-  private SolrConfig config;
-  private CoreContainer coreContainer;
-  private PackageListeningClassLoader schemaLoader ;
-
-  private PackageListeningClassLoader createSchemaLoader() {
-    CoreContainer cc = getCoreContainer();
-    if (cc == null) {
-      //corecontainer not available . can't load from packages
-      return null;
-    }
-    return new PackageListeningClassLoader(cc, this, pkg -> {
-      if (getSolrConfig() == null) return null;
-      return getSolrConfig().maxPackageVersion(pkg);
-    }, () -> {
-      if(getCoreContainer() == null || config == null || coreName == null || coreId==null) return;
-      try (SolrCore c = getCoreContainer().getCore(coreName, coreId)) {
-        if (c != null) {
-          c.fetchLatestSchema();
-        }
-      }
-    });
-  }
 
 
 
@@ -126,13 +101,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
     return managedResourceRegistry;
   }
 
-  public SolrClassLoader getSchemaLoader() {
-    if (schemaLoader == null) {
-      schemaLoader = createSchemaLoader();
-    }
-    return schemaLoader;
-  }
-
   public SolrResourceLoader() {
     this(SolrPaths.locateSolrHome(), null);
   }
@@ -676,11 +644,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
   public void inform(SolrCore core)
   {
     this.dataDir = core.getDataDir(); // removed in 9.0
-    this.coreName = core.getName();
-    this.config = core.getSolrConfig();
-    this.coreId = core.uniqueId;
-    this.coreContainer = core.getCoreContainer();
-    if(getSchemaLoader() != null) core.getPackageListeners().addListener(schemaLoader);
 
     // make a copy to avoid potential deadlock of a callback calling newInstance and trying to
     // add something to waitingForCore.
@@ -827,14 +790,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
     throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, builder.toString() );
   }
 
-  public CoreContainer getCoreContainer(){
-    return coreContainer;
-  }
-
-  public SolrConfig getSolrConfig() {
-    return config;
-
-  }
   @Override
   public void close() throws IOException {
     IOUtils.close(classLoader);
diff --git a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
index 0f5bb7b..57f8ba0 100644
--- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
+++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
@@ -948,7 +948,7 @@ public class IndexFetcher {
     final CountDownLatch latch = new CountDownLatch(1);
     new Thread(() -> {
       try {
-        solrCore.getCoreContainer().reload(solrCore.getName(), solrCore.uniqueId);
+        solrCore.getCoreContainer().reload(solrCore.getName());
       } catch (Exception e) {
         log.error("Could not reload core ", e);
       } finally {
diff --git a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
index 12733ec..fef5773 100644
--- a/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SchemaHandler.java
@@ -26,7 +26,6 @@ import org.apache.solr.api.ApiBag;
 import org.apache.solr.cloud.ZkSolrResourceLoader;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.cloud.SolrClassLoader;
 import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -35,7 +34,6 @@ import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrCore;
-import org.apache.solr.pkg.PackageListeningClassLoader;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.request.SolrRequestHandler;
 import org.apache.solr.response.SolrQueryResponse;
@@ -231,9 +229,7 @@ public class SchemaHandler extends RequestHandlerBase implements SolrCoreAware,
         String klas = (String) v;
         PluginInfo.ClassName parsedClassName = new PluginInfo.ClassName(klas);
         if (parsedClassName.pkg != null) {
-          SolrClassLoader solrClassLoader = req.getCore().getLatestSchema().getSolrClassLoader();
-          MapWriter mw = solrClassLoader instanceof PackageListeningClassLoader ?
-              ((PackageListeningClassLoader) solrClassLoader).getPackageVersion(parsedClassName) : null;
+          MapWriter mw = req.getCore().getSchemaPluginsLoader().getPackageVersion(parsedClassName);
           if (mw != null) nl.add("_packageinfo_", mw);
         }
       }
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 9ac6207..7f48b63 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -548,7 +548,7 @@ public class SolrConfigHandler extends RequestHandlerBase implements SolrCoreAwa
             latestVersion, 30);
       } else {
         SolrResourceLoader.persistConfLocally(loader, ConfigOverlay.RESOURCE_NAME, overlay.toByteArray());
-        req.getCore().getCoreContainer().reload(req.getCore().getName(), req.getCore().uniqueId, false);
+        req.getCore().getCoreContainer().reload(req.getCore().getName(), req.getCore().uniqueId);
         log.info("Executed config commands successfully and persisted to File System {}", ops);
       }
 
diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java
index bd4ae7e..3e2237f 100644
--- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java
+++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java
@@ -190,7 +190,7 @@ public class IndexSchema {
   protected IndexSchema(Version luceneVersion, SolrResourceLoader loader, Properties substitutableProperties) {
     this.luceneVersion = Objects.requireNonNull(luceneVersion);
     this.loader = loader;
-    this.solrClassLoader = loader.getSchemaLoader() == null ? loader : loader.getSchemaLoader();
+    this.solrClassLoader = loader;//loader.getCore() == null? loader: loader.getCore().getSchemaPluginsLoader();
     this.substitutableProperties = substitutableProperties;
   }
 
diff --git a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java
index 3731a17..d2f5b67 100644
--- a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java
+++ b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java
@@ -132,7 +132,7 @@ public class SchemaManager {
             latestVersion = ZkController.persistConfigResourceToZooKeeper
                 (zkLoader, managedIndexSchema.getSchemaZkVersion(), managedIndexSchema.getResourceName(),
                  sw.toString().getBytes(StandardCharsets.UTF_8), true);
-            req.getCore().getCoreContainer().reload(req.getCore().getName(), req.getCore().uniqueId, false);
+            req.getCore().getCoreContainer().reload(req.getCore().getName(), req.getCore().uniqueId);
             break;
           } catch (ZkController.ResourceModifiedInZkException e) {
             log.info("Schema was modified by another node. Retrying..");
@@ -142,7 +142,7 @@ public class SchemaManager {
             //only for non cloud stuff
             managedIndexSchema.persistManagedSchema(false);
             core.setLatestSchema(managedIndexSchema);
-            core.getCoreContainer().reload(core.getName(), core.uniqueId, false);
+            core.getCoreContainer().reload(core.getName(), core.uniqueId);
           } catch (SolrException e) {
             log.warn(errorMsg);
             errors = singletonList(errorMsg + e.getMessage());
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 f967cbd..46b4071 100644
--- a/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
+++ b/solr/core/src/test/org/apache/solr/pkg/TestPackages.java
@@ -63,6 +63,7 @@ import org.apache.solr.util.plugin.SolrCoreAware;
 import org.apache.zookeeper.data.Stat;
 import org.junit.After;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.apache.solr.common.cloud.ZkStateReader.SOLR_PKGS_PATH;
@@ -636,6 +637,7 @@ public class TestPackages extends SolrCloudTestCase {
   }
 
   @SuppressWarnings("rawtypes")
+  @Ignore("SOLR-14750")
   public void testSchemaPlugins() throws Exception {
     String COLLECTION_NAME = "testSchemaLoadingColl";
     System.setProperty("managed.schema.mutable", "true");