You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ro...@apache.org on 2016/07/08 13:30:01 UTC

[1/3] lucene-solr:branch_6x: SOLR-9181: Add some logging to ZkStateReader to try and debug test failures

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x b0386f0d6 -> fda3d8b7c


SOLR-9181: Add some logging to ZkStateReader to try and debug test failures


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

Branch: refs/heads/branch_6x
Commit: 60232cd028e41c427b686a6cab720ac3989ba289
Parents: b0386f0
Author: Alan Woodward <ro...@apache.org>
Authored: Tue Jul 5 10:46:43 2016 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Fri Jul 8 14:29:34 2016 +0100

----------------------------------------------------------------------
 .../solr/cloud/overseer/ZkStateWriterTest.java  | 12 +++++--
 .../apache/solr/common/cloud/SolrZkClient.java  | 37 ++++++++++----------
 .../apache/solr/common/cloud/ZkStateReader.java |  4 +++
 3 files changed, 31 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/60232cd0/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java
index deca8b0..86aac4d 100644
--- a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateWriterTest.java
@@ -16,6 +16,10 @@
  */
 package org.apache.solr.cloud.overseer;
 
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.lucene.util.IOUtils;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.cloud.AbstractZkTestCase;
@@ -31,12 +35,13 @@ import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.util.Utils;
 import org.apache.zookeeper.KeeperException;
-
-import java.util.HashMap;
-import java.util.Map;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class ZkStateWriterTest extends SolrTestCaseJ4 {
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
   public void testZkStateWriterBatching() throws Exception {
     String zkDir = createTempDir("testZkStateWriterBatching").toFile().getAbsolutePath();
 
@@ -319,6 +324,7 @@ public class ZkStateWriterTest extends SolrTestCaseJ4 {
       // get the most up-to-date state
       reader.forceUpdateCollection("c2");
       state = reader.getClusterState();
+      log.info("Cluster state: {}", state);
       assertTrue(state.hasCollection("c2"));
       assertEquals(sharedClusterStateVersion, (int) state.getZkClusterStateVersion());
       assertEquals(stateFormat2Version + 1, state.getCollection("c2").getZNodeVersion());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/60232cd0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
index a6bbd73..af6f9be 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java
@@ -16,6 +16,24 @@
  */
 package org.apache.solr.common.cloud;
 
+import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Source;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.stream.StreamResult;
+import javax.xml.transform.stream.StreamSource;
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
+import java.io.StringReader;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.regex.Pattern;
+
 import org.apache.commons.io.FileUtils;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.common.SolrException;
@@ -38,25 +56,6 @@ import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.xml.transform.OutputKeys;
-import javax.xml.transform.Source;
-import javax.xml.transform.Transformer;
-import javax.xml.transform.TransformerFactory;
-import javax.xml.transform.stream.StreamResult;
-import javax.xml.transform.stream.StreamSource;
-
-import java.io.Closeable;
-import java.io.File;
-import java.io.IOException;
-import java.io.StringReader;
-import java.io.StringWriter;
-import java.lang.invoke.MethodHandles;
-import java.nio.charset.StandardCharsets;
-import java.nio.file.Path;
-import java.util.List;
-import java.util.concurrent.ExecutorService;
-import java.util.regex.Pattern;
-
 /**
  *
  * All Solr ZooKeeper interactions should go through this class rather than

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/60232cd0/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index f106b9c..8b3beb4 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -290,6 +290,7 @@ public class ZkStateReader implements Closeable {
 
     synchronized (getUpdateLock()) {
       if (clusterState == null) {
+        LOG.info("ClusterState watchers have not been initialized");
         return;
       }
 
@@ -297,6 +298,7 @@ public class ZkStateReader implements Closeable {
       if (ref == null || legacyCollectionStates.containsKey(collection)) {
         // We either don't know anything about this collection (maybe it's new?) or it's legacy.
         // First update the legacy cluster state.
+        LOG.info("Checking legacy cluster state for collection {}", collection);
         refreshLegacyClusterState(null);
         if (!legacyCollectionStates.containsKey(collection)) {
           // No dice, see if a new collection just got created.
@@ -307,6 +309,7 @@ public class ZkStateReader implements Closeable {
           }
         }
       } else if (ref.isLazilyLoaded()) {
+        LOG.info("Refreshing lazily-loaded state for collection {}", collection);
         if (ref.get() != null) {
           return;
         }
@@ -314,6 +317,7 @@ public class ZkStateReader implements Closeable {
         refreshLegacyClusterState(null);
       } else if (watchedCollectionStates.containsKey(collection)) {
         // Exists as a watched collection, force a refresh.
+        LOG.info("Forcing refresh of watched collection state for {}", collection);
         DocCollection newState = fetchCollectionState(collection, null);
         if (updateWatchedCollection(collection, newState)) {
           constructState(Collections.singletonMap(collection, newState));


[3/3] lucene-solr:branch_6x: SOLR-9181: Fix race in constructState() and missing call in forceUpdateCollection()

Posted by ro...@apache.org.
SOLR-9181: Fix race in constructState() and missing call in forceUpdateCollection()


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

Branch: refs/heads/branch_6x
Commit: fda3d8b7c2069d9cbd2445b397e9cceb38851be6
Parents: 86d8d3a
Author: Alan Woodward <ro...@apache.org>
Authored: Tue Jul 5 15:30:07 2016 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Fri Jul 8 14:29:49 2016 +0100

----------------------------------------------------------------------
 .../apache/solr/common/cloud/ZkStateReader.java | 38 ++++++++++----------
 1 file changed, 19 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fda3d8b7/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index ffd85ca..3f422fa 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -23,7 +23,6 @@ import java.net.URLDecoder;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -271,11 +270,11 @@ public class ZkStateReader implements Closeable {
       refreshLegacyClusterState(null);
       // Need a copy so we don't delete from what we're iterating over.
       Collection<String> safeCopy = new ArrayList<>(watchedCollectionStates.keySet());
-      Map<String, DocCollection> updatedCollections = new HashMap<>();
+      Set<String> updatedCollections = new HashSet<>();
       for (String coll : safeCopy) {
         DocCollection newState = fetchCollectionState(coll, null);
         if (updateWatchedCollection(coll, newState)) {
-          updatedCollections.put(coll, newState);
+          updatedCollections.add(coll);
         }
       }
       constructState(updatedCollections);
@@ -305,7 +304,9 @@ public class ZkStateReader implements Closeable {
           LazyCollectionRef tryLazyCollection = new LazyCollectionRef(collection);
           if (tryLazyCollection.get() != null) {
             // What do you know, it exists!
+            LOG.info("Adding lazily-loaded reference for collection {}", collection);
             lazyCollectionStates.putIfAbsent(collection, tryLazyCollection);
+            constructState(Collections.singleton(collection));
           }
         }
       } else if (ref.isLazilyLoaded()) {
@@ -320,10 +321,9 @@ public class ZkStateReader implements Closeable {
         LOG.info("Forcing refresh of watched collection state for {}", collection);
         DocCollection newState = fetchCollectionState(collection, null);
         if (updateWatchedCollection(collection, newState)) {
-          constructState(Collections.singletonMap(collection, newState));
+          constructState(Collections.singleton(collection));
         }
-      }
-      else {
+      } else {
         LOG.error("Collection {} is not lazy or watched!", collection);
       }
     }
@@ -349,7 +349,7 @@ public class ZkStateReader implements Closeable {
       if (nu.getZNodeVersion() > collection.getZNodeVersion()) {
         if (updateWatchedCollection(coll, nu)) {
           synchronized (getUpdateLock()) {
-            constructState(Collections.singletonMap(coll, nu));
+            constructState(Collections.singleton(coll));
           }
         }
         collection = nu;
@@ -385,7 +385,7 @@ public class ZkStateReader implements Closeable {
     refreshCollectionList(new CollectionsChildWatcher());
 
     synchronized (ZkStateReader.this.getUpdateLock()) {
-      constructState(Collections.emptyMap());
+      constructState(Collections.emptySet());
 
       zkClient.exists(ALIASES,
           new Watcher() {
@@ -482,7 +482,7 @@ public class ZkStateReader implements Closeable {
    * @param changedCollections collections that have changed since the last call,
    *                           and that should fire notifications
    */
-  private void constructState(Map<String, DocCollection> changedCollections) {
+  private void constructState(Set<String> changedCollections) {
 
     Set<String> liveNodes = this.liveNodes; // volatile read
 
@@ -518,9 +518,10 @@ public class ZkStateReader implements Closeable {
           clusterState.getCollectionStates());
     }
 
-    for (Map.Entry<String, DocCollection> entry : changedCollections.entrySet()) {
-      notifyStateWatchers(liveNodes, entry.getKey(), entry.getValue());
+    for (String collection : changedCollections) {
+      notifyStateWatchers(liveNodes, collection, clusterState.getCollectionOrNull(collection));
     }
+
   }
 
   /**
@@ -536,7 +537,7 @@ public class ZkStateReader implements Closeable {
           // Nothing to do, someone else updated same or newer.
           return;
         }
-        Map<String, DocCollection> updatedCollections = new HashMap<>();
+        Set<String> updatedCollections = new HashSet<>();
         for (String coll : this.collectionWatches.keySet()) {
           ClusterState.CollectionRef ref = this.legacyCollectionStates.get(coll);
           // legacy collections are always in-memory
@@ -548,7 +549,7 @@ public class ZkStateReader implements Closeable {
             newState = watchedCollectionStates.get(coll);
           }
           if (!Objects.equals(oldState, newState)) {
-            updatedCollections.put(coll, newState);
+            updatedCollections.add(coll);
           }
         }
         this.legacyCollectionStates = loadedData.getCollectionStates();
@@ -560,7 +561,7 @@ public class ZkStateReader implements Closeable {
       synchronized (getUpdateLock()) {
         this.legacyCollectionStates = emptyMap();
         this.legacyClusterStateVersion = 0;
-        constructState(Collections.emptyMap());
+        constructState(Collections.emptySet());
       }
     }
   }
@@ -582,7 +583,7 @@ public class ZkStateReader implements Closeable {
    *
    * A stateFormat=1 collection which is not interesting to us can also
    * be put into the {@link #lazyCollectionStates} map here. But that is okay
-   * because {@link #constructState(Map)} will give priority to collections in the
+   * because {@link #constructState(Set)} will give priority to collections in the
    * shared collection state over this map.
    * In fact this is a clever way to avoid doing a ZK exists check on
    * the /collections/collection_name/state.json znode
@@ -615,7 +616,6 @@ public class ZkStateReader implements Closeable {
           // Double check contains just to avoid allocating an object.
           LazyCollectionRef existing = lazyCollectionStates.get(coll);
           if (existing == null) {
-            LOG.info("Adding lazy collectionRef for collection {}", coll);
             lazyCollectionStates.putIfAbsent(coll, new LazyCollectionRef(coll));
           }
         }
@@ -954,7 +954,7 @@ public class ZkStateReader implements Closeable {
         DocCollection newState = fetchCollectionState(coll, this);
         updateWatchedCollection(coll, newState);
         synchronized (getUpdateLock()) {
-          constructState(Collections.singletonMap(coll, newState));
+          constructState(Collections.singleton(coll));
         }
 
       } catch (KeeperException.SessionExpiredException | KeeperException.ConnectionLossException e) {
@@ -1015,7 +1015,7 @@ public class ZkStateReader implements Closeable {
       LOG.info("A collections change: [{}], has occurred - updating...", event);
       refreshAndWatch();
       synchronized (getUpdateLock()) {
-        constructState(Collections.emptyMap());
+        constructState(Collections.emptySet());
       }
     }
 
@@ -1164,7 +1164,7 @@ public class ZkStateReader implements Closeable {
     });
     if (reconstructState.get()) {
       synchronized (getUpdateLock()) {
-        constructState(Collections.emptyMap());
+        constructState(Collections.emptySet());
       }
     }
   }


[2/3] lucene-solr:branch_6x: SOLR-9181: More logging

Posted by ro...@apache.org.
SOLR-9181: More logging


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

Branch: refs/heads/branch_6x
Commit: 86d8d3a937802f47add8408bdd05117ec0fc2137
Parents: 60232cd
Author: Alan Woodward <ro...@apache.org>
Authored: Tue Jul 5 15:17:24 2016 +0100
Committer: Alan Woodward <ro...@apache.org>
Committed: Fri Jul 8 14:29:42 2016 +0100

----------------------------------------------------------------------
 .../src/java/org/apache/solr/common/cloud/ZkStateReader.java     | 4 ++++
 1 file changed, 4 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/86d8d3a9/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 8b3beb4..ffd85ca 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -323,6 +323,9 @@ public class ZkStateReader implements Closeable {
           constructState(Collections.singletonMap(collection, newState));
         }
       }
+      else {
+        LOG.error("Collection {} is not lazy or watched!", collection);
+      }
     }
 
   }
@@ -612,6 +615,7 @@ public class ZkStateReader implements Closeable {
           // Double check contains just to avoid allocating an object.
           LazyCollectionRef existing = lazyCollectionStates.get(coll);
           if (existing == null) {
+            LOG.info("Adding lazy collectionRef for collection {}", coll);
             lazyCollectionStates.putIfAbsent(coll, new LazyCollectionRef(coll));
           }
         }