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 04:07:00 UTC

[lucene-solr] 06/07: Revert "SOLR-14151: Bug fixes (#1815)"

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

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

commit bd67a9185761c2d1ddf4738b99f9fcda14bdcb86
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sat Sep 19 09:26:07 2020 +0530

    Revert "SOLR-14151: Bug fixes (#1815)"
    
    This reverts commit 95ab98c920833f286608846188d69302b478f80a.
---
 .../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   | 48 +---------------------
 .../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, 32 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 0e17b5c..4d9d910 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkSolrResourceLoader.java
@@ -26,7 +26,6 @@ import java.nio.file.Path;
 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;
@@ -158,9 +157,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 1d45bc5..c77d0b4 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 9e0a890..e8fed0c 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1586,13 +1586,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
@@ -1607,8 +1600,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());
@@ -1838,9 +1836,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.
    *
@@ -1849,10 +1844,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 f3e0408..b44866e 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -225,8 +225,8 @@ public final class SolrCore implements SolrInfoBean, Closeable {
   private final RecoveryStrategy.Builder recoveryStrategyBuilder;
   private IndexReaderFactory indexReaderFactory;
   private final Codec codec;
-  private final ConfigSet configSet;
   //singleton listener for all packages used in schema
+  private final PackageListeningClassLoader schemaPluginsLoader;
 
   private final CircuitBreakerManager circuitBreakerManager;
 
@@ -273,6 +273,9 @@ public final class SolrCore implements SolrInfoBean, Closeable {
   public PackageListeners getPackageListeners() {
     return packageListeners;
   }
+  public PackageListeningClassLoader getSchemaPluginsLoader() {
+    return schemaPluginsLoader;
+  }
 
   static int boolean_query_max_clause_count = Integer.MIN_VALUE;
 
@@ -936,12 +939,15 @@ public final class SolrCore implements SolrInfoBean, Closeable {
     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();
@@ -2762,10 +2768,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
     };
   }
 
-  public void fetchLatestSchema() {
-    IndexSchema schema =  configSet.getIndexSchema(true);
-    setLatestSchema(schema);
-  }
+
   public interface RawWriter {
     default String getContentType() {
       return BinaryResponseParser.BINARY_CONTENT_TYPE;
@@ -3073,7 +3076,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
       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();
@@ -3094,7 +3097,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
         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 {
@@ -3107,7 +3110,7 @@ public final class SolrCore implements SolrInfoBean, Closeable {
         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 574bba0..205b70f 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 7a10f89..c5ad5fc 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrResourceLoader.java
@@ -46,7 +46,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;
@@ -62,7 +61,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
   private String name = "";
   protected URLClassLoader classLoader;
   private final Path instanceDir;
-  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);
   }
@@ -639,12 +607,6 @@ public class SolrResourceLoader implements ResourceLoader, Closeable, SolrClassL
    * Tell all {@link SolrCoreAware} instances about the SolrCore
    */
   public void inform(SolrCore core) {
-    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.
     SolrCoreAware[] arr;
@@ -783,14 +745,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 c8a0512..2bd2045 100644
--- a/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
+++ b/solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
@@ -909,7 +909,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 606bab5..0ab79e5 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;
@@ -227,9 +225,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 4a1aadb..fc7ca3b 100644
--- a/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/SolrConfigHandler.java
@@ -547,7 +547,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 8d8583c..c4bf29e 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;
@@ -639,6 +640,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");