From 5fadc4ee2351d5763b8ff00072670aa3227f24e1 Mon Sep 17 00:00:00 2001 From: Varun Thacker Date: Wed, 6 Jul 2016 14:08:05 +0530 Subject: [PATCH] SOLR-9088: Fixed TestManagedSchemaAPI failures which exposed race conditions in the schema API --- solr/CHANGES.txt | 2 + .../org/apache/solr/cloud/ZkController.java | 24 +++++++- .../java/org/apache/solr/core/SolrCore.java | 57 +++++++++++++++---- .../org/apache/solr/schema/SchemaManager.java | 57 +++++++++---------- 4 files changed, 97 insertions(+), 43 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f39580e9625..860230093c4 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -117,6 +117,8 @@ Bug Fixes * SOLR-9235: Fixed NPE when using non-numeric range query in deleteByQuery (hossman) +* SOLR-9088: Fixed TestManagedSchemaAPI failures which exposed race conditions in the schema API ( Varun Thacker, noble) + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 9ebca6f81d6..a6a15082acf 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -53,7 +53,25 @@ import org.apache.solr.cloud.overseer.OverseerAction; import org.apache.solr.cloud.overseer.SliceMutator; import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrException.ErrorCode; -import org.apache.solr.common.cloud.*; +import org.apache.solr.common.cloud.BeforeReconnect; +import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.ClusterStateUtil; +import org.apache.solr.common.cloud.DefaultConnectionStrategy; +import org.apache.solr.common.cloud.DefaultZkACLProvider; +import org.apache.solr.common.cloud.DefaultZkCredentialsProvider; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.OnReconnect; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.Slice; +import org.apache.solr.common.cloud.SolrZkClient; +import org.apache.solr.common.cloud.ZkACLProvider; +import org.apache.solr.common.cloud.ZkCmdExecutor; +import org.apache.solr.common.cloud.ZkConfigManager; +import org.apache.solr.common.cloud.ZkCoreNodeProps; +import org.apache.solr.common.cloud.ZkCredentialsProvider; +import org.apache.solr.common.cloud.ZkNodeProps; +import org.apache.solr.common.cloud.ZkStateReader; +import org.apache.solr.common.cloud.ZooKeeperException; import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.SolrParams; @@ -2242,8 +2260,8 @@ public final class ZkController { String errMsg = "Failed to persist resource at {0} - old {1}"; try { try { - zkClient.setData(resourceLocation, content, znodeVersion, true); - latestVersion = znodeVersion + 1;// if the set succeeded , it should have incremented the version by one always + Stat stat = zkClient.setData(resourceLocation, content, znodeVersion, true); + latestVersion = stat.getVersion();// if the set succeeded , it should have incremented the version by one always log.info("Persisted config data to node {} ", resourceLocation); touchConfDir(zkLoader); } catch (NoNodeException 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 53af3d1b5ce..14a4e0ff1e8 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrCore.java +++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java @@ -28,7 +28,19 @@ import java.lang.reflect.Constructor; import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.NoSuchFileException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -77,7 +89,22 @@ import org.apache.solr.handler.component.SearchComponent; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.request.SolrRequestHandler; -import org.apache.solr.response.*; +import org.apache.solr.response.BinaryResponseWriter; +import org.apache.solr.response.CSVResponseWriter; +import org.apache.solr.response.GeoJSONResponseWriter; +import org.apache.solr.response.GraphMLResponseWriter; +import org.apache.solr.response.JSONResponseWriter; +import org.apache.solr.response.PHPResponseWriter; +import org.apache.solr.response.PHPSerializedResponseWriter; +import org.apache.solr.response.PythonResponseWriter; +import org.apache.solr.response.QueryResponseWriter; +import org.apache.solr.response.RawResponseWriter; +import org.apache.solr.response.RubyResponseWriter; +import org.apache.solr.response.SchemaXmlResponseWriter; +import org.apache.solr.response.SmileResponseWriter; +import org.apache.solr.response.SolrQueryResponse; +import org.apache.solr.response.SortingResponseWriter; +import org.apache.solr.response.XMLResponseWriter; import org.apache.solr.response.transform.TransformerFactory; import org.apache.solr.rest.ManagedResourceStorage; import org.apache.solr.rest.ManagedResourceStorage.StorageIO; @@ -86,6 +113,7 @@ import org.apache.solr.schema.FieldType; import org.apache.solr.schema.IndexSchema; import org.apache.solr.schema.IndexSchemaFactory; import org.apache.solr.schema.ManagedIndexSchema; +import org.apache.solr.schema.SchemaManager; import org.apache.solr.schema.SimilarityFactory; import org.apache.solr.search.QParserPlugin; import org.apache.solr.search.SolrFieldCacheMBean; @@ -2488,13 +2516,13 @@ public final class SolrCore implements SolrInfoMBean, Closeable { SolrZkClient zkClient = cc.getZkController().getZkClient(); int solrConfigversion, overlayVersion, managedSchemaVersion = 0; SolrConfig cfg = null; - try (SolrCore core1 = cc.solrCores.getCoreFromAnyList(coreName, true)) { - if (core1 == null || core1.isClosed()) return; - cfg = core1.getSolrConfig(); - solrConfigversion = core1.getSolrConfig().getOverlay().getZnodeVersion(); - overlayVersion = core1.getSolrConfig().getZnodeVersion(); + try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true)) { + if (solrCore == null || solrCore.isClosed()) return; + cfg = solrCore.getSolrConfig(); + solrConfigversion = solrCore.getSolrConfig().getOverlay().getZnodeVersion(); + overlayVersion = solrCore.getSolrConfig().getZnodeVersion(); if (managedSchmaResourcePath != null) { - managedSchemaVersion = ((ManagedIndexSchema) core1.getLatestSchema()).getSchemaZkVersion(); + managedSchemaVersion = ((ManagedIndexSchema) solrCore.getLatestSchema()).getSchemaZkVersion(); } } @@ -2504,6 +2532,13 @@ public final class SolrCore implements SolrInfoMBean, Closeable { if (checkStale(zkClient, overlayPath, solrConfigversion) || checkStale(zkClient, solrConfigPath, overlayVersion) || checkStale(zkClient, managedSchmaResourcePath, managedSchemaVersion)) { + + try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true)) { + solrCore.setLatestSchema(SchemaManager.getFreshManagedSchema(solrCore)); + } catch (Exception e) { + log.warn("", SolrZkClient.checkInterrupted(e)); + } + log.info("core reload {}", coreName); try { cc.reload(coreName); @@ -2513,9 +2548,9 @@ public final class SolrCore implements SolrInfoMBean, Closeable { return; } //some files in conf directory may have other than managedschema, overlay, params - try (SolrCore core1 = cc.solrCores.getCoreFromAnyList(coreName, true)) { - if (core1 == null || core1.isClosed()) return; - for (Runnable listener : core1.confListeners) { + try (SolrCore solrCore = cc.solrCores.getCoreFromAnyList(coreName, true)) { + if (solrCore == null || solrCore.isClosed()) return; + for (Runnable listener : solrCore.confListeners) { try { listener.run(); } catch (Exception e) { 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 3b492a75dbe..ca3d756edff 100644 --- a/solr/core/src/java/org/apache/solr/schema/SchemaManager.java +++ b/solr/core/src/java/org/apache/solr/schema/SchemaManager.java @@ -16,6 +16,18 @@ */ package org.apache.solr.schema; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.io.StringWriter; +import java.lang.invoke.MethodHandles; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + import org.apache.solr.cloud.ZkController; import org.apache.solr.cloud.ZkSolrResourceLoader; import org.apache.solr.common.SolrException; @@ -31,18 +43,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.InputSource; -import java.io.IOException; -import java.io.InputStream; -import java.io.Reader; -import java.io.StringWriter; -import java.lang.invoke.MethodHandles; -import java.nio.charset.StandardCharsets; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.TimeUnit; - import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; @@ -108,7 +108,7 @@ public class SchemaManager { SolrCore core = req.getCore(); String errorMsg = "Unable to persist managed schema. "; while (!timeOut.hasTimedOut()) { - managedIndexSchema = getFreshManagedSchema(); + managedIndexSchema = getFreshManagedSchema(req.getCore()); for (CommandOperation op : operations) { OpType opType = OpType.get(op.name); if (opType != null) { @@ -131,9 +131,9 @@ public class SchemaManager { } try { - ZkController.persistConfigResourceToZooKeeper(zkLoader, managedIndexSchema.getSchemaZkVersion(), + int latestVersion = ZkController.persistConfigResourceToZooKeeper(zkLoader, managedIndexSchema.getSchemaZkVersion(), managedIndexSchema.getResourceName(), sw.toString().getBytes(StandardCharsets.UTF_8), true); - waitForOtherReplicasToUpdate(timeOut); + waitForOtherReplicasToUpdate(timeOut, latestVersion); core.setLatestSchema(managedIndexSchema); return Collections.emptyList(); } catch (ZkController.ResourceModifiedInZkException e) { @@ -155,7 +155,7 @@ public class SchemaManager { return singletonList(errorMsg + "Timed out."); } - private void waitForOtherReplicasToUpdate(TimeOut timeOut) { + private void waitForOtherReplicasToUpdate(TimeOut timeOut, int latestVersion) { CoreDescriptor cd = req.getCore().getCoreDescriptor(); String collection = cd.getCollectionName(); if (collection != null) { @@ -164,11 +164,8 @@ public class SchemaManager { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Not enough time left to update replicas. However, the schema is updated already."); } - ManagedIndexSchema.waitForSchemaZkVersionAgreement(collection, - cd.getCloudDescriptor().getCoreNodeName(), - (managedIndexSchema).getSchemaZkVersion(), - zkLoader.getZkController(), - (int) timeOut.timeLeft(TimeUnit.SECONDS)); + ManagedIndexSchema.waitForSchemaZkVersionAgreement(collection, cd.getCloudDescriptor().getCoreNodeName(), + latestVersion, zkLoader.getZkController(), (int) timeOut.timeLeft(TimeUnit.SECONDS)); } } @@ -423,21 +420,23 @@ public class SchemaManager { return sb.toString(); } - public ManagedIndexSchema getFreshManagedSchema() throws IOException, KeeperException, InterruptedException { - SolrResourceLoader resourceLoader = req.getCore().getResourceLoader(); + public static ManagedIndexSchema getFreshManagedSchema(SolrCore core) throws IOException, + KeeperException, InterruptedException { + + SolrResourceLoader resourceLoader = core.getResourceLoader(); + String name = core.getLatestSchema().getResourceName(); if (resourceLoader instanceof ZkSolrResourceLoader) { - InputStream in = resourceLoader.openResource(req.getSchema().getResourceName()); + InputStream in = resourceLoader.openResource(name); if (in instanceof ZkSolrResourceLoader.ZkByteArrayInputStream) { int version = ((ZkSolrResourceLoader.ZkByteArrayInputStream) in).getStat().getVersion(); log.info("managed schema loaded . version : {} ", version); - return new ManagedIndexSchema - (req.getCore().getSolrConfig(), req.getSchema().getResourceName(), new InputSource(in), - true, req.getSchema().getResourceName(), version, req.getSchema().getSchemaUpdateLock()); + return new ManagedIndexSchema(core.getSolrConfig(), name, new InputSource(in), true, name, version, + core.getLatestSchema().getSchemaUpdateLock()); } else { - return (ManagedIndexSchema) req.getCore().getLatestSchema(); + return (ManagedIndexSchema) core.getLatestSchema(); } } else { - return (ManagedIndexSchema) req.getCore().getLatestSchema(); + return (ManagedIndexSchema) core.getLatestSchema(); } } }