You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by bo...@apache.org on 2017/09/13 14:49:09 UTC

[2/3] storm git commit: Check for empty nimbus info list before attempting to update the blobstore.

Check for empty nimbus info list before attempting to update the blobstore.


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

Branch: refs/heads/1.x-branch
Commit: a3fe9388714d42563a329e92214760d091cab4c2
Parents: bc36e06
Author: Heather McCartney <he...@idioplatform.com>
Authored: Mon Sep 11 11:40:20 2017 +0100
Committer: Heather McCartney <he...@idioplatform.com>
Committed: Wed Sep 13 11:44:33 2017 +0100

----------------------------------------------------------------------
 .../apache/storm/blobstore/BlobStoreUtils.java  |   4 +
 .../storm/blobstore/BlobStoreUtilsTest.java     | 135 +++++++++++++++++++
 .../blobstore/MockZookeeperClientBuilder.java   | 100 ++++++++++++++
 storm-core/test/resources/log4j2-test.xml       |   2 +
 4 files changed, 241 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/a3fe9388/storm-core/src/jvm/org/apache/storm/blobstore/BlobStoreUtils.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/blobstore/BlobStoreUtils.java b/storm-core/src/jvm/org/apache/storm/blobstore/BlobStoreUtils.java
index 5d696c3..f1eb2f4 100644
--- a/storm-core/src/jvm/org/apache/storm/blobstore/BlobStoreUtils.java
+++ b/storm-core/src/jvm/org/apache/storm/blobstore/BlobStoreUtils.java
@@ -17,6 +17,7 @@
  */
 package org.apache.storm.blobstore;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.storm.Config;
 import org.apache.storm.generated.AuthorizationException;
@@ -245,6 +246,9 @@ public class BlobStoreUtils {
                 return;
             }
             stateInfo = zkClient.getChildren().forPath(BLOBSTORE_SUBTREE + "/" + key);
+            if (CollectionUtils.isEmpty(stateInfo)) {
+                return;
+            }
             LOG.debug("StateInfo for update {}", stateInfo);
             Set<NimbusInfo> nimbusInfoList = getNimbodesWithLatestSequenceNumberOfBlob(zkClient, key);
 

http://git-wip-us.apache.org/repos/asf/storm/blob/a3fe9388/storm-core/test/jvm/org/apache/storm/blobstore/BlobStoreUtilsTest.java
----------------------------------------------------------------------
diff --git a/storm-core/test/jvm/org/apache/storm/blobstore/BlobStoreUtilsTest.java b/storm-core/test/jvm/org/apache/storm/blobstore/BlobStoreUtilsTest.java
new file mode 100644
index 0000000..10e44e8
--- /dev/null
+++ b/storm-core/test/jvm/org/apache/storm/blobstore/BlobStoreUtilsTest.java
@@ -0,0 +1,135 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.blobstore;
+
+import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.storm.nimbus.NimbusInfo;
+import org.junit.Test;
+
+public class BlobStoreUtilsTest {
+
+    private static final String KEY = "key";
+    private static final String BLOBSTORE_KEY = "/blobstore/" + KEY;
+
+    @SuppressWarnings("unchecked")
+    private Map<String, Object> conf = (Map<String, Object>)mock(Map.class);
+    private MockZookeeperClientBuilder zkClientBuilder = new MockZookeeperClientBuilder();
+    private BlobStore blobStore = mock(BlobStore.class);
+    private NimbusInfo nimbusDetails = mock(NimbusInfo.class);
+
+    /**
+     * If nimbusDetails are null, the method returns without any Zookeeper calls.
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_nullNimbusInfo() {
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, null);
+
+        zkClientBuilder.verifyExists(false);
+        zkClientBuilder.verifyGetChildren(false);
+        verify(nimbusDetails, never()).getHost();
+        verify(conf, never()).get(anyString());
+    }
+
+    /**
+     * If the node doesn't exist, the method returns before attempting to fetch children.
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_missingNode() {
+        zkClientBuilder.withExists(BLOBSTORE_KEY, false);
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, nimbusDetails);
+
+        zkClientBuilder.verifyExists(true);
+        zkClientBuilder.verifyGetChildren(false);
+        verify(nimbusDetails, never()).getHost();
+        verify(conf, never()).get(anyString());
+    }
+
+    /**
+     * If the node has null children, the method will exit before calling downloadUpdatedBlob
+     * (the config map is first accessed by downloadUpdatedBlob).
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_nodeWithNullChildren() {
+        zkClientBuilder.withExists(BLOBSTORE_KEY, true);
+        zkClientBuilder.withGetChildren(BLOBSTORE_KEY, (List<String>)null);
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, nimbusDetails);
+
+        zkClientBuilder.verifyExists(true);
+        zkClientBuilder.verifyGetChildren();
+        verify(nimbusDetails, never()).getHost();
+        verify(conf, never()).get(anyString());
+    }
+
+    /**
+     * If the node has no children, the method behaves the same as for null children.
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_nodeWithEmptyChildren() {
+        zkClientBuilder.withExists(BLOBSTORE_KEY, true);
+        zkClientBuilder.withGetChildren(BLOBSTORE_KEY);
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, nimbusDetails);
+
+        zkClientBuilder.verifyExists(true);
+        zkClientBuilder.verifyGetChildren();
+        verify(nimbusDetails, never()).getHost();
+        verify(conf, never()).get(anyString());
+    }
+
+    /**
+     * If the node has children, their hostnames will be checked and if they match,
+     * downloadUpdatedBlob will not be called.
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_hostsMatch() {
+        zkClientBuilder.withExists(BLOBSTORE_KEY, true);
+        zkClientBuilder.withGetChildren(BLOBSTORE_KEY, "localhost:1111-1");
+        when(nimbusDetails.getHost()).thenReturn("localhost");
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, nimbusDetails);
+
+        zkClientBuilder.verifyExists(true);
+        zkClientBuilder.verifyGetChildren(2);
+        verify(nimbusDetails).getHost();
+        verify(conf, never()).get(anyString());
+    }
+
+    /**
+     * If the node has children, their hostnames will be checked and if they don't match,
+     * downloadUpdatedBlob will be called.
+     */
+    @Test
+    public void testUpdateKeyForBlobStore_noMatch() {
+        zkClientBuilder.withExists(BLOBSTORE_KEY, true);
+        zkClientBuilder.withGetChildren(BLOBSTORE_KEY, "localhost:1111-1");
+        when(nimbusDetails.getHost()).thenReturn("no match");
+        BlobStoreUtils.updateKeyForBlobStore(conf, blobStore, zkClientBuilder.build(), KEY, nimbusDetails);
+
+        zkClientBuilder.verifyExists(true);
+        zkClientBuilder.verifyGetChildren(2);
+        verify(nimbusDetails).getHost();
+        verify(conf, atLeastOnce()).get(anyString());
+    }
+}

http://git-wip-us.apache.org/repos/asf/storm/blob/a3fe9388/storm-core/test/jvm/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
----------------------------------------------------------------------
diff --git a/storm-core/test/jvm/org/apache/storm/blobstore/MockZookeeperClientBuilder.java b/storm-core/test/jvm/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
new file mode 100644
index 0000000..c3e2b84
--- /dev/null
+++ b/storm-core/test/jvm/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
@@ -0,0 +1,100 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.storm.blobstore;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.apache.curator.framework.CuratorFramework;
+import org.apache.curator.framework.api.ExistsBuilder;
+import org.apache.curator.framework.api.GetChildrenBuilder;
+import org.apache.curator.framework.api.Pathable;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+public class MockZookeeperClientBuilder {
+
+    private static final Logger LOG = Logger.getLogger(MockZookeeperClientBuilder.class);
+
+    private CuratorFramework zkClient = mock(CuratorFramework.class);
+
+    private ExistsBuilder existsBuilder = mock(ExistsBuilder.class);
+    private GetChildrenBuilder getChildrenBuilder = mock(GetChildrenBuilder.class);
+
+    public MockZookeeperClientBuilder() {
+        when(zkClient.checkExists()).thenReturn(existsBuilder);
+        when(zkClient.getChildren()).thenReturn(getChildrenBuilder);
+    }
+
+    public CuratorFramework build() {
+        return zkClient;
+    }
+
+    private <T extends Pathable<U>, U> T mockForPath(T pathable, String path, U toReturn) {
+        try {
+            when(pathable.forPath(path)).thenReturn(toReturn);
+        } catch (Exception e) {
+            LOG.warn(e.toString());
+        }
+        return pathable;
+    }
+
+    public ExistsBuilder withExists(String path, boolean returnValue) {
+        when(existsBuilder.watched()).thenReturn(existsBuilder);
+        Stat stat = returnValue ? new Stat() : null;
+        return mockForPath(existsBuilder, path, stat);
+    }
+
+    public GetChildrenBuilder withGetChildren(String path, String... returnValue) {
+        return withGetChildren(path, Arrays.asList(returnValue));
+    }
+
+    public GetChildrenBuilder withGetChildren(String path, List<String> returnValue) {
+        when(getChildrenBuilder.watched()).thenReturn(getChildrenBuilder);
+        return mockForPath(getChildrenBuilder, path, returnValue);
+    }
+
+    public void verifyExists() {
+        verifyExists(true);
+    }
+
+    public void verifyExists(boolean happened) {
+        verifyExists(happened ? 1 : 0);
+    }
+
+    public void verifyExists(int times) {
+        verify(zkClient, times(times)).checkExists();
+    }
+
+    public void verifyGetChildren() {
+        verifyGetChildren(true);
+    }
+
+    public void verifyGetChildren(boolean happened) {
+        verifyGetChildren(happened ? 1 : 0);
+    }
+
+    public void verifyGetChildren(int times) {
+        verify(zkClient, times(times)).getChildren();
+    }
+}

http://git-wip-us.apache.org/repos/asf/storm/blob/a3fe9388/storm-core/test/resources/log4j2-test.xml
----------------------------------------------------------------------
diff --git a/storm-core/test/resources/log4j2-test.xml b/storm-core/test/resources/log4j2-test.xml
index e8ae19e..54429a0 100644
--- a/storm-core/test/resources/log4j2-test.xml
+++ b/storm-core/test/resources/log4j2-test.xml
@@ -23,6 +23,8 @@
         </Console>
     </Appenders>
     <Loggers>
+        <!-- suppress ERROR org.apache.storm.blobstore.BlobStoreUtils - Could not update the blob with key: key when testing -->
+        <Logger name="org.apache.storm.blobstore" level="FATAL" />
         <Logger name="org.apache.zookeeper" level="WARN"/>
         <Root level="${env:LOG_LEVEL:-INFO}">
             <AppenderRef ref="Console"/>