You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by cp...@apache.org on 2017/04/25 13:59:38 UTC

[11/17] lucene-solr:jira/solr-8668: SOLR-10493: Investigate SolrCloudExampleTest failures.

SOLR-10493: Investigate SolrCloudExampleTest failures.

(cherry picked from commit 0247acd)


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/e17b9877
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/e17b9877
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/e17b9877

Branch: refs/heads/jira/solr-8668
Commit: e17b987734cae654e01d58876e3fc05eea1bb605
Parents: 56e1ad4
Author: Erick Erickson <er...@apache.org>
Authored: Mon Apr 24 12:17:46 2017 -0700
Committer: Erick Erickson <er...@apache.org>
Committed: Mon Apr 24 12:22:11 2017 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../org/apache/solr/cloud/CloudDescriptor.java  | 20 +++++
 .../org/apache/solr/core/CoreContainer.java     | 40 ++++++++--
 .../apache/solr/core/CorePropertiesLocator.java | 18 ++---
 .../java/org/apache/solr/core/CoresLocator.java | 10 ---
 .../apache/solr/cloud/SolrCloudExampleTest.java | 81 +++++++++++++++++++-
 .../org/apache/solr/core/TestCoreContainer.java |  5 --
 .../apache/solr/util/ReadOnlyCoresLocator.java  |  6 --
 .../java/org/apache/solr/util/TestHarness.java  |  4 -
 9 files changed, 142 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5ba38d9..35fd327 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -226,6 +226,8 @@ Bug Fixes
 * SOLR-5127: Multiple highlight fields and wildcards are now supported e.g. hl.fl=title,text_*
   (Sven-S. Porst, Daniel Debray, Simon Endele, Christine Poerschke)
 
+* SOLR-10493: Investigate SolrCloudExampleTest failures. (Erick Erickson)
+
 Other Changes
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java b/solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
index fdc7b02..719b1d1 100644
--- a/solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
+++ b/solr/core/src/java/org/apache/solr/cloud/CloudDescriptor.java
@@ -21,6 +21,7 @@ import java.util.Map;
 import java.util.Properties;
 
 import com.google.common.base.Strings;
+import org.apache.solr.common.StringUtils;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.core.CoreDescriptor;
 import org.apache.solr.util.PropertiesUtil;
@@ -135,4 +136,23 @@ public class CloudDescriptor {
     if(nodeName==null) cd.getPersistableStandardProperties().remove(CoreDescriptor.CORE_NODE_NAME);
     else cd.getPersistableStandardProperties().setProperty(CoreDescriptor.CORE_NODE_NAME, nodeName);
   }
+
+  public void reload(CloudDescriptor reloadFrom) {
+    if (reloadFrom == null) return;
+
+    setShardId(StringUtils.isEmpty(reloadFrom.getShardId()) ? getShardId() : reloadFrom.getShardId());
+    setCollectionName(StringUtils.isEmpty(reloadFrom.getCollectionName()) ? getCollectionName() : reloadFrom.getCollectionName());
+    setRoles(StringUtils.isEmpty(reloadFrom.getRoles()) ? getRoles() : reloadFrom.getRoles());
+    if (reloadFrom.getNumShards() != null) {
+      setNumShards(reloadFrom.getNumShards());
+    }
+    setCoreNodeName(StringUtils.isEmpty(reloadFrom.getCoreNodeName()) ? getCoreNodeName() : reloadFrom.getCoreNodeName());
+    setLeader(reloadFrom.isLeader);
+    setHasRegistered(reloadFrom.hasRegistered);
+    setLastPublished(reloadFrom.getLastPublished());
+
+    for (Map.Entry<String, String> ent : reloadFrom.getParams().entrySet()) {
+      collectionParams.put(ent.getKey(), ent.getValue());
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/java/org/apache/solr/core/CoreContainer.java
----------------------------------------------------------------------
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 3c3aaa5..28c1eaf 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -104,6 +104,7 @@ import static org.apache.solr.common.params.CommonParams.CORES_HANDLER_PATH;
 import static org.apache.solr.common.params.CommonParams.INFO_HANDLER_PATH;
 import static org.apache.solr.common.params.CommonParams.METRICS_PATH;
 import static org.apache.solr.common.params.CommonParams.ZK_PATH;
+import static org.apache.solr.core.CorePropertiesLocator.PROPERTIES_FILENAME;
 import static org.apache.solr.security.AuthenticationPlugin.AUTHENTICATION_PLUGIN_PROP;
 
 /**
@@ -1130,7 +1131,37 @@ public class CoreContainer {
   }
 
 
-  // ---------------- Core name related methods --------------- 
+  // ---------------- Core name related methods ---------------
+
+  private CoreDescriptor reloadCoreDescriptor(CoreDescriptor oldDesc) {
+    if (oldDesc == null) {
+      return null;
+    }
+
+    CorePropertiesLocator cpl = new CorePropertiesLocator(null);
+    CoreDescriptor ret = cpl.buildCoreDescriptor(oldDesc.getInstanceDir().resolve(PROPERTIES_FILENAME), this);
+
+    // Ok, this little jewel is all because we still create core descriptors on the fly from lists of properties
+    // in tests particularly. Theoretically, there should be _no_ way to create a CoreDescriptor in the new world
+    // of core discovery without writing the core.properties file out first.
+    //
+    // TODO: remove core.properties from the conf directory in test files, it's in a bad place there anyway.
+    if (ret == null) {
+      oldDesc.loadExtraProperties(); // there may be changes to extra properties that we need to pick up.
+      return oldDesc;
+      
+    }
+    // The CloudDescriptor bit here is created in a very convoluted way, requiring access to private methods
+    // in ZkController. When reloading, this behavior is identical to what used to happen where a copy of the old
+    // CoreDescriptor was just re-used.
+    
+    if (ret.getCloudDescriptor() != null) {
+      ret.getCloudDescriptor().reload(oldDesc.getCloudDescriptor());
+    }
+
+    return ret;
+  }
+
   /**
    * Recreates a SolrCore.
    * While the new core is loading, requests will continue to be dispatched to
@@ -1141,11 +1172,10 @@ public class CoreContainer {
   public void reload(String name) {
     SolrCore core = solrCores.getCoreFromAnyList(name, false);
     if (core != null) {
+      
       // The underlying core properties files may have changed, we don't really know. So we have a (perhaps) stale
-      // CoreDescriptor we need to reload it if it's out there. 
-      CorePropertiesLocator cpl = new CorePropertiesLocator(null);
-      CoreDescriptor cd = cpl.reload(this, core.getCoreDescriptor());
-      if (cd == null) cd = core.getCoreDescriptor();
+      // CoreDescriptor and we need to reload it from the disk files
+      CoreDescriptor cd = reloadCoreDescriptor(core.getCoreDescriptor());
       solrCores.addCoreDescriptor(cd);
       try {
         solrCores.waitAddPendingCoreOps(cd.getName());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
index 385d11b..e942c9b 100644
--- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
+++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java
@@ -134,8 +134,10 @@ public class CorePropertiesLocator implements CoresLocator {
         public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
           if (file.getFileName().toString().equals(PROPERTIES_FILENAME)) {
             CoreDescriptor cd = buildCoreDescriptor(file, cc);
-            logger.debug("Found core {} in {}", cd.getName(), cd.getInstanceDir());
-            cds.add(cd);
+            if (cd != null) {
+              logger.debug("Found core {} in {}", cd.getName(), cd.getInstanceDir());
+              cds.add(cd);
+            }
             return FileVisitResult.SKIP_SIBLINGS;
           }
           return FileVisitResult.CONTINUE;
@@ -163,14 +165,6 @@ public class CorePropertiesLocator implements CoresLocator {
     return cds;
   }
 
-  @Override
-  public CoreDescriptor reload(CoreContainer cc, CoreDescriptor cd) {
-    if (cd == null) return null;
-    
-    Path coreProps = cd.getInstanceDir().resolve(CoreDescriptor.DEFAULT_EXTERNAL_PROPERTIES_FILE);
-    return buildCoreDescriptor(coreProps, cc);
-  }
-
   protected CoreDescriptor buildCoreDescriptor(Path propertiesFile, CoreContainer cc) {
 
     Path instanceDir = propertiesFile.getParent();
@@ -182,7 +176,9 @@ public class CorePropertiesLocator implements CoresLocator {
       for (String key : coreProperties.stringPropertyNames()) {
         propMap.put(key, coreProperties.getProperty(key));
       }
-      return new CoreDescriptor(name, instanceDir, propMap, cc.getContainerProperties(), cc.isZooKeeperAware());
+      CoreDescriptor ret = new CoreDescriptor(name, instanceDir, propMap, cc.getContainerProperties(), cc.isZooKeeperAware());
+      ret.loadExtraProperties();
+      return ret;
     }
     catch (IOException e) {
       logger.error("Couldn't load core descriptor from {}:{}", propertiesFile, e.toString());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/java/org/apache/solr/core/CoresLocator.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/CoresLocator.java b/solr/core/src/java/org/apache/solr/core/CoresLocator.java
index beaa690..52927f1 100644
--- a/solr/core/src/java/org/apache/solr/core/CoresLocator.java
+++ b/solr/core/src/java/org/apache/solr/core/CoresLocator.java
@@ -68,14 +68,4 @@ public interface CoresLocator {
    * @return a list of all CoreDescriptors found
    */
   public List<CoreDescriptor> discover(CoreContainer cc);
-
-  /**
-   * reload an existing CoreDescriptor, that is read it from disk.
-   * 
-   * @param cc the CoreContainer
-   * @param cd the old CoreDescriptor. If null, this is a no-op
-   * @return the reloaded coreDescriptor or null          
-   */
-  public CoreDescriptor reload(CoreContainer cc, CoreDescriptor cd);
-
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java b/solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java
index 9d415bc..8c49f6b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/SolrCloudExampleTest.java
@@ -18,24 +18,40 @@ package org.apache.solr.cloud;
 
 import java.io.File;
 import java.io.FilenameFilter;
+import java.io.StringReader;
 import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
 import java.util.Set;
 
 import org.apache.commons.cli.CommandLine;
+import org.apache.http.HttpEntity;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.util.EntityUtils;
 import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.request.ContentStreamUpdateRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.util.ExternalPaths;
 import org.apache.solr.util.SolrCLI;
 import org.junit.Test;
+import org.noggit.JSONParser;
+import org.noggit.ObjectBuilder;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static java.util.Arrays.asList;
+import static org.apache.solr.common.util.Utils.getObjectByPath;
+
 /**
  * Emulates bin/solr -e cloud -noprompt; bin/post -c gettingstarted example/exampledocs/*.xml;
  * this test is useful for catching regressions in indexing the example docs in collections that
@@ -129,10 +145,16 @@ public class SolrCloudExampleTest extends AbstractFullDistribZkTestBase {
       cloudClient.request(req);
     }
     cloudClient.commit();
-    Thread.sleep(1000);
 
-    QueryResponse qr = cloudClient.query(new SolrQuery("*:*"));
-    int numFound = (int)qr.getResults().getNumFound();
+    int numFound = 0;
+
+    // give the update a chance to take effect.
+    for (int idx = 0; idx < 100; ++idx) {
+      QueryResponse qr = cloudClient.query(new SolrQuery("*:*"));
+      numFound = (int) qr.getResults().getNumFound();
+      if (numFound == expectedXmlDocCount) break;
+      Thread.sleep(100);
+    }
     assertEquals("*:* found unexpected number of documents", expectedXmlDocCount, numFound);
 
     log.info("Updating Config for " + testCollectionName);
@@ -192,6 +214,9 @@ public class SolrCloudExampleTest extends AbstractFullDistribZkTestBase {
         "-value", maxTime.toString(),
         "-solrUrl", solrUrl
     };
+
+    Map<String, Long> startTimes = getSoftAutocommitInterval(testCollectionName);
+
     SolrCLI.ConfigTool tool = new SolrCLI.ConfigTool();
     CommandLine cli = SolrCLI.processCommandLineArgs(SolrCLI.joinCommonAndToolOptions(tool.getOptions()), args);
     log.info("Sending set-property '" + prop + "'=" + maxTime + " to SolrCLI.ConfigTool.");
@@ -201,5 +226,55 @@ public class SolrCloudExampleTest extends AbstractFullDistribZkTestBase {
     maxTimeFromConfig = SolrCLI.atPath("/config/updateHandler/autoSoftCommit/maxTime", configJson);
     assertNotNull(maxTimeFromConfig);
     assertEquals(maxTime, maxTimeFromConfig);
+
+    log.info("live_nodes_count :  " + cloudClient.getZkStateReader().getClusterState().getLiveNodes());
+
+    // Since it takes some time for this command to complete we need to make sure all the reloads for
+    // all the cores have been done.
+    boolean allGood = false;
+    Map<String, Long> curSoftCommitInterval = null;
+    for (int idx = 0; idx < 600 && allGood == false; ++idx) {
+      curSoftCommitInterval = getSoftAutocommitInterval(testCollectionName);
+      if (curSoftCommitInterval.size() > 0 && curSoftCommitInterval.size() == startTimes.size()) { // no point in even trying if they're not the same size!
+        allGood = true;
+        for (Map.Entry<String, Long> currEntry : curSoftCommitInterval.entrySet()) {
+          if (currEntry.getValue().equals(maxTime) == false) {
+            allGood = false;
+          }
+        }
+      }
+      if (allGood == false) {
+        Thread.sleep(100);
+      }
+    }
+    assertTrue("All cores should have been reloaded within 60 seconds!!!", allGood);
+  }
+
+  // Collect all of the autoSoftCommit intervals.
+  private Map<String, Long> getSoftAutocommitInterval(String collection) throws Exception {
+    Map<String, Long> ret = new HashMap<>();
+    DocCollection coll = cloudClient.getZkStateReader().getClusterState().getCollection(collection);
+    for (Slice slice : coll.getActiveSlices()) {
+      for (Replica replica : slice.getReplicas()) {
+        String uri = "" + replica.get(ZkStateReader.BASE_URL_PROP) + "/" + replica.get(ZkStateReader.CORE_NAME_PROP) + "/config?wt=json";
+        Map respMap = getAsMap(cloudClient, uri);
+        Long maxTime = (Long) (getObjectByPath(respMap, true, asList("config", "updateHandler", "autoSoftCommit", "maxTime")));
+        ret.put(replica.getCoreName(), maxTime);
+      }
+    }
+    return ret;
   }
+
+  private Map getAsMap(CloudSolrClient cloudClient, String uri) throws Exception {
+    HttpGet get = new HttpGet(uri);
+    HttpEntity entity = null;
+    try {
+      entity = cloudClient.getLbClient().getHttpClient().execute(get).getEntity();
+      String response = EntityUtils.toString(entity, StandardCharsets.UTF_8);
+      return (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+    } finally {
+      EntityUtils.consumeQuietly(entity);
+    }
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
index 91bbabb..2949e2e 100644
--- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
+++ b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java
@@ -386,11 +386,6 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
       return cores;
     }
 
-    @Override
-    public CoreDescriptor reload(CoreContainer cc, CoreDescriptor cd) {
-      return cd;
-    }
-
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
----------------------------------------------------------------------
diff --git a/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java b/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
index 3ad3ce2..3d11ff7 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/ReadOnlyCoresLocator.java
@@ -47,10 +47,4 @@ public abstract class ReadOnlyCoresLocator implements CoresLocator {
     // no-op
   }
 
-  @Override
-  public CoreDescriptor reload(CoreContainer cc, CoreDescriptor cd) {
-    return null; // no-op
-  }
-
-
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/e17b9877/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
----------------------------------------------------------------------
diff --git a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
index b8e1899..cefd75f 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/TestHarness.java
@@ -231,10 +231,6 @@ public class TestHarness extends BaseTestHarness {
           CoreDescriptor.CORE_SHARD, System.getProperty("shard", "shard1")));
     }
 
-    @Override
-    public CoreDescriptor reload(CoreContainer cc, CoreDescriptor cd) {
-      return cd;
-    }
   }
   
   public CoreContainer getCoreContainer() {