storm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bo...@apache.org
Subject [1/2] storm git commit: Check for empty nimbus info list before attempting to update the blobstore.
Date Wed, 13 Sep 2017 13:25:06 GMT
Repository: storm
Updated Branches:
  refs/heads/master 6d23b8b7e -> c365bcae0


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/1dba0b4d
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/1dba0b4d
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/1dba0b4d

Branch: refs/heads/master
Commit: 1dba0b4d8f324ae561bc7d407f079d32e2730939
Parents: 6d23b8b
Author: Heather McCartney <heather.mccartney@idioplatform.com>
Authored: Tue Sep 12 10:43:48 2017 +0100
Committer: Heather McCartney <heather.mccartney@idioplatform.com>
Committed: Wed Sep 13 11:47:02 2017 +0100

----------------------------------------------------------------------
 .../apache/storm/blobstore/BlobStoreUtils.java  |  25 ++--
 .../storm/blobstore/BlobStoreUtilsTest.java     | 135 +++++++++++++++++++
 .../blobstore/MockZookeeperClientBuilder.java   | 100 ++++++++++++++
 storm-server/src/test/resources/log4j2.xml      |  32 +++++
 4 files changed, 282 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/1dba0b4d/storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java b/storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java
index d6412a8..9aca91c 100644
--- a/storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java
+++ b/storm-server/src/main/java/org/apache/storm/blobstore/BlobStoreUtils.java
@@ -17,6 +17,18 @@
  */
 package org.apache.storm.blobstore;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.security.auth.Subject;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.curator.framework.CuratorFramework;
 import org.apache.storm.Config;
 import org.apache.storm.generated.AuthorizationException;
 import org.apache.storm.generated.KeyAlreadyExistsException;
@@ -27,22 +39,12 @@ import org.apache.storm.security.auth.NimbusPrincipal;
 import org.apache.storm.utils.CuratorUtils;
 import org.apache.storm.utils.NimbusClient;
 import org.apache.storm.utils.ZookeeperAuthInfo;
-import org.apache.curator.framework.CuratorFramework;
 import org.apache.thrift.transport.TTransportException;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.KeeperException.NoNodeException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.security.auth.Subject;
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-
 public class BlobStoreUtils {
     private static final String BLOBSTORE_SUBTREE="/blobstore";
     private static final Logger LOG = LoggerFactory.getLogger(BlobStoreUtils.class);
@@ -243,6 +245,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/1dba0b4d/storm-server/src/test/java/org/apache/storm/blobstore/BlobStoreUtilsTest.java
----------------------------------------------------------------------
diff --git a/storm-server/src/test/java/org/apache/storm/blobstore/BlobStoreUtilsTest.java
b/storm-server/src/test/java/org/apache/storm/blobstore/BlobStoreUtilsTest.java
new file mode 100644
index 0000000..e861db8
--- /dev/null
+++ b/storm-server/src/test/java/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.Matchers.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/1dba0b4d/storm-server/src/test/java/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
----------------------------------------------------------------------
diff --git a/storm-server/src/test/java/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
b/storm-server/src/test/java/org/apache/storm/blobstore/MockZookeeperClientBuilder.java
new file mode 100644
index 0000000..c3e2b84
--- /dev/null
+++ b/storm-server/src/test/java/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/1dba0b4d/storm-server/src/test/resources/log4j2.xml
----------------------------------------------------------------------
diff --git a/storm-server/src/test/resources/log4j2.xml b/storm-server/src/test/resources/log4j2.xml
new file mode 100644
index 0000000..fe097c6
--- /dev/null
+++ b/storm-server/src/test/resources/log4j2.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+<Configuration>
+  <Appenders>
+    <Console name="console" target="SYSTEM_OUT">
+      <PatternLayout pattern="%d{yyy-MM-dd HH:mm:ss.SSS} [%t] %-5level %logger{36} - %msg%n"/>
+    </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" />
+    <Root level="ERROR">
+      <appender-ref ref="console" />
+    </Root>
+  </Loggers>
+</Configuration>
+      
\ No newline at end of file


Mime
View raw message