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 15:23:20 UTC
storm git commit: Merge branch 'STORM-2736' of
https://github.com/hmcc/storm into STORM-2736-1.x
Repository: storm
Updated Branches:
refs/heads/1.1.x-branch 3202257ab -> 3d6727a57
Merge branch 'STORM-2736' of https://github.com/hmcc/storm into STORM-2736-1.x
STORM-2736: fix o.a.s.b.BlobStoreUtils [ERROR] "Could not update the blob with key"
This closes #2320
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/3d6727a5
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/3d6727a5
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/3d6727a5
Branch: refs/heads/1.1.x-branch
Commit: 3d6727a57a8c7cb8137ab308594c7ac0c76e2369
Parents: 3202257
Author: Robert Evans <ev...@yahoo-inc.com>
Authored: Wed Sep 13 08:26:19 2017 -0500
Committer: Robert Evans <ev...@yahoo-inc.com>
Committed: Wed Sep 13 09:49:37 2017 -0500
----------------------------------------------------------------------
.../apache/storm/blobstore/BlobStoreUtils.java | 8 +-
.../storm/blobstore/BlobStoreUtilsTest.java | 135 +++++++++++++++++++
.../blobstore/MockZookeeperClientBuilder.java | 100 ++++++++++++++
storm-core/test/resources/log4j2-test.xml | 2 +
4 files changed, 243 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/3d6727a5/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 d62a71b..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;
@@ -159,7 +160,7 @@ public class BlobStoreUtils {
}
if (!isSuccess) {
- LOG.error("Could not download blob with key" + key);
+ LOG.error("Could not download blob with key {}", key);
}
return isSuccess;
}
@@ -204,7 +205,7 @@ public class BlobStoreUtils {
}
if (!isSuccess) {
- LOG.error("Could not update the blob with key" + key);
+ LOG.error("Could not update the blob with key {}", key);
}
return isSuccess;
}
@@ -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/3d6727a5/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/3d6727a5/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/3d6727a5/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"/>