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 13:25:06 UTC
[1/2] storm git commit: Check for empty nimbus info list before
attempting to update the blobstore.
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 <he...@idioplatform.com>
Authored: Tue Sep 12 10:43:48 2017 +0100
Committer: Heather McCartney <he...@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
[2/2] storm git commit: Merge branch 'STORM-2736-master' of
https://github.com/hmcc/storm into STORM-2736
Posted by bo...@apache.org.
Merge branch 'STORM-2736-master' of https://github.com/hmcc/storm into STORM-2736
STORM-2736: fix o.a.s.b.BlobStoreUtils [ERROR] "Could not update the blob with key"
This closes #2323
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/c365bcae
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/c365bcae
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/c365bcae
Branch: refs/heads/master
Commit: c365bcae0b3f951a2a4f749495226945354e98cc
Parents: 6d23b8b 1dba0b4
Author: Robert Evans <ev...@yahoo-inc.com>
Authored: Wed Sep 13 08:08:04 2017 -0500
Committer: Robert Evans <ev...@yahoo-inc.com>
Committed: Wed Sep 13 08:08:04 2017 -0500
----------------------------------------------------------------------
.../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(-)
----------------------------------------------------------------------