You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2016/04/05 20:20:12 UTC

[1/3] lucene-solr:solr-5750: SOLR-8875 SOLR-5750: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer.

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x 88d2b3bfb -> 4cfd33f74
  refs/heads/master ee98f6ab8 -> 3bbf8aaa8
  refs/heads/solr-5750 fd9c4d59e -> 099680a06


SOLR-8875 SOLR-5750: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer.


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

Branch: refs/heads/solr-5750
Commit: 099680a062bda5747ada44944e97a6f5e274314c
Parents: fd9c4d5
Author: David Smiley <ds...@apache.org>
Authored: Tue Apr 5 14:03:03 2016 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Tue Apr 5 14:03:03 2016 -0400

----------------------------------------------------------------------
 .../src/java/org/apache/solr/cloud/Overseer.java     | 15 +--------------
 .../apache/solr/cloud/overseer/ZkStateWriter.java    |  4 ++--
 2 files changed, 3 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/099680a0/solr/core/src/java/org/apache/solr/cloud/Overseer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 7244514..933aa64 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -140,7 +140,6 @@ public class Overseer implements Closeable {
       try {
         ZkStateWriter zkStateWriter = null;
         ClusterState clusterState = null;
-        //nocommit: can't this simply be the same as clusterState being null?
         boolean refreshClusterState = true; // let's refresh in the first iteration
         while (!this.isClosed) {
           isLeader = amILeader();
@@ -156,7 +155,6 @@ public class Overseer implements Closeable {
             try {
               reader.updateClusterState();
               clusterState = reader.getClusterState();
-              assert clusterState != null : "should clusterState be null?";//nocommit
               zkStateWriter = new ZkStateWriter(reader, stats);
               refreshClusterState = false;
 
@@ -171,14 +169,12 @@ public class Overseer implements Closeable {
                 // force flush to ZK after each message because there is no fallback if workQueue items
                 // are removed from workQueue but fail to be written to ZK
                 clusterState = processQueueItem(message, clusterState, zkStateWriter, false, null);
-                assert clusterState != null : "should clusterState be null?";//nocommit
                 workQueue.poll(); // poll-ing removes the element we got by peek-ing
                 data = workQueue.peek();
               }
               // force flush at the end of the loop
               if (hadWorkItems) {
                 clusterState = zkStateWriter.writePendingUpdates();
-                assert clusterState != null : "should clusterState be null?";//nocommit
               }
             } catch (KeeperException e) {
               if (e.code() == KeeperException.Code.SESSIONEXPIRED) {
@@ -192,8 +188,6 @@ public class Overseer implements Closeable {
             } catch (Exception e) {
               log.error("Exception in Overseer work queue loop", e);
             }
-          } else {
-            assert clusterState != null : "should clusterState be null?";//nocommit
           }
 
           byte[] head = null;
@@ -231,7 +225,6 @@ public class Overseer implements Closeable {
                   while (workQueue.poll() != null);
                 }
               });
-              assert clusterState != null : "should clusterState be null?";//nocommit
 
               // it is safer to keep this poll here because an invalid message might never be queued
               // and therefore we can't rely on the ZkWriteCallback to remove the item
@@ -243,13 +236,7 @@ public class Overseer implements Closeable {
             }
             // we should force write all pending updates because the next iteration might sleep until there
             // are more items in the main queue
-
-            ClusterState clusterState2 = zkStateWriter.writePendingUpdates();//note: may return null?
-            if (clusterState2 == null) {
-              log.error("zkStateWriter.writePendingUpdates returned null clusterState");
-            } else {
-              clusterState = clusterState2;
-            }
+            clusterState = zkStateWriter.writePendingUpdates();
             // clean work queue
             while (workQueue.poll() != null);
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/099680a0/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
index 9fb3ada..ec67ed7 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud.overseer;
 
+import java.lang.invoke.MethodHandles;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -35,8 +36,6 @@ import org.slf4j.LoggerFactory;
 
 import static java.util.Collections.singletonMap;
 
-import java.lang.invoke.MethodHandles;
-
 /**
  * ZkStateWriter is responsible for writing updates to the cluster state stored in ZooKeeper for
  * both stateFormat=1 collection (stored in shared /clusterstate.json in ZK) and stateFormat=2 collections
@@ -84,6 +83,7 @@ public class ZkStateWriter {
 
     this.reader = zkStateReader;
     this.stats = stats;
+    this.clusterState = zkStateReader.getClusterState();
   }
 
   /**


[2/3] lucene-solr:master: SOLR-8875: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer.

Posted by ds...@apache.org.
SOLR-8875: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer.


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

Branch: refs/heads/master
Commit: 3bbf8aaa8a9f57c43d64e9c361184c379d90b9c9
Parents: ee98f6a
Author: David Smiley <ds...@apache.org>
Authored: Tue Apr 5 14:15:31 2016 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Tue Apr 5 14:15:31 2016 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                                 | 3 +++
 solr/core/src/java/org/apache/solr/cloud/Overseer.java           | 1 +
 .../src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java   | 4 ++--
 3 files changed, 6 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3bbf8aaa/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5ab52b2..9e6da65 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -101,6 +101,9 @@ Bug Fixes
 * SOLR-8902: Make sure ReturnFields only returns the requested fields from (fl=) evn when 
   DocumentTransformers ask for getExtraRequestFields()  (ryan)
 
+* SOLR-8875: SolrCloud Overseer clusterState could unexpectedly be null resulting in NPE.
+  (Scott Blum via David Smiley)
+
 Optimizations
 ----------------------
 * SOLR-8722: Don't force a full ZkStateReader refresh on every Overseer operation.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3bbf8aaa/solr/core/src/java/org/apache/solr/cloud/Overseer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 0e5bded..f25cab7 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -151,6 +151,7 @@ public class Overseer implements Closeable {
             continue; // not a no, not a yes, try ask again
           }
 
+          //TODO consider removing 'refreshClusterState' and simply check if clusterState is null
           if (refreshClusterState) {
             try {
               reader.updateClusterState();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/3bbf8aaa/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
index 9fb3ada..ec67ed7 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud.overseer;
 
+import java.lang.invoke.MethodHandles;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -35,8 +36,6 @@ import org.slf4j.LoggerFactory;
 
 import static java.util.Collections.singletonMap;
 
-import java.lang.invoke.MethodHandles;
-
 /**
  * ZkStateWriter is responsible for writing updates to the cluster state stored in ZooKeeper for
  * both stateFormat=1 collection (stored in shared /clusterstate.json in ZK) and stateFormat=2 collections
@@ -84,6 +83,7 @@ public class ZkStateWriter {
 
     this.reader = zkStateReader;
     this.stats = stats;
+    this.clusterState = zkStateReader.getClusterState();
   }
 
   /**


[3/3] lucene-solr:branch_6x: SOLR-8875: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer. (cherry picked from commit 3bbf8aa)

Posted by ds...@apache.org.
SOLR-8875: Fix null clusterState from ZkStateWriter. Revert my sanity check asserts in Overseer.
(cherry picked from commit 3bbf8aa)


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

Branch: refs/heads/branch_6x
Commit: 4cfd33f7434509d535fa8d5faaf3d20ced1935ce
Parents: 88d2b3b
Author: David Smiley <ds...@apache.org>
Authored: Tue Apr 5 14:15:31 2016 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Tue Apr 5 14:18:27 2016 -0400

----------------------------------------------------------------------
 solr/CHANGES.txt                                                | 5 +++--
 solr/core/src/java/org/apache/solr/cloud/Overseer.java          | 1 +
 .../src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java  | 4 ++--
 3 files changed, 6 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4cfd33f7/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 17e949a..cc2a43e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -70,7 +70,8 @@ Bug Fixes
 * SOLR-8902: Make sure ReturnFields only returns the requested fields from (fl=) evn when 
   DocumentTransformers ask for getExtraRequestFields()  (ryan)
 
-* SOLR-8855: The HDFS BlockDirectory should not clean up it's cache on shutdown. (Mark Miller)
+* SOLR-8875: SolrCloud Overseer clusterState could unexpectedly be null resulting in NPE.
+  (Scott Blum via David Smiley)
 
 Optimizations
 ----------------------
@@ -499,7 +500,7 @@ Other Changes
 
 * SOLR-8758: Add a new SolrCloudTestCase class, using MiniSolrCloudCluster (Alan
   Woodward)
-  
+
 * SOLR-8764: Remove all deprecated methods and classes from master prior to the 6.0 release. (Steve Rowe)
 
 * SOLR-8780: Remove unused OverseerCollectionMessageHandler#getClusterStatus method. (Varun Thacker)

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4cfd33f7/solr/core/src/java/org/apache/solr/cloud/Overseer.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 0e5bded..f25cab7 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -151,6 +151,7 @@ public class Overseer implements Closeable {
             continue; // not a no, not a yes, try ask again
           }
 
+          //TODO consider removing 'refreshClusterState' and simply check if clusterState is null
           if (refreshClusterState) {
             try {
               reader.updateClusterState();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/4cfd33f7/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
index 9fb3ada..ec67ed7 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cloud.overseer;
 
+import java.lang.invoke.MethodHandles;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -35,8 +36,6 @@ import org.slf4j.LoggerFactory;
 
 import static java.util.Collections.singletonMap;
 
-import java.lang.invoke.MethodHandles;
-
 /**
  * ZkStateWriter is responsible for writing updates to the cluster state stored in ZooKeeper for
  * both stateFormat=1 collection (stored in shared /clusterstate.json in ZK) and stateFormat=2 collections
@@ -84,6 +83,7 @@ public class ZkStateWriter {
 
     this.reader = zkStateReader;
     this.stats = stats;
+    this.clusterState = zkStateReader.getClusterState();
   }
 
   /**