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/05/22 17:08:56 UTC
[1/4] storm git commit: STORM-2503: Fix lgtm.com alerts on equality
and comparison operations.
Repository: storm
Updated Branches:
refs/heads/master 8a4fe363b -> d7fb0fbc4
STORM-2503: Fix lgtm.com alerts on equality and comparison operations.
Fixes several alerts involving comparison operations
found at https://lgtm.com/projects/g/apache/storm/alerts/.
Fixes comparator logic in `DefaultResourceAwareStrategy`
so that components with fewer connections are considered smaller.
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/9c1ab5b8
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/9c1ab5b8
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/9c1ab5b8
Branch: refs/heads/master
Commit: 9c1ab5b8c97dfacda3daa199d8a2383495b69374
Parents: 7742398
Author: Aditya Sharad <ad...@semmle.com>
Authored: Tue May 2 22:16:58 2017 +0100
Committer: Aditya Sharad <ad...@semmle.com>
Committed: Mon May 15 22:11:25 2017 +0100
----------------------------------------------------------------------
.../jvm/org/apache/storm/starter/TransactionalWords.java | 2 +-
.../jvm/org/apache/storm/container/cgroup/SubSystem.java | 9 +++++++++
.../apache/storm/scheduler/SchedulerAssignmentImpl.java | 9 +++++++++
.../scheduling/DefaultResourceAwareStrategy.java | 6 +++---
.../trident/windowing/StoreBasedTridentWindowManager.java | 2 +-
.../main/java/org/apache/storm/daemon/nimbus/Nimbus.java | 10 +++++-----
.../apache/storm/daemon/supervisor/ReadClusterState.java | 2 +-
7 files changed, 29 insertions(+), 11 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java
----------------------------------------------------------------------
diff --git a/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java b/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java
index e922bab..7f9e060 100644
--- a/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java
+++ b/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java
@@ -125,7 +125,7 @@ public class TransactionalWords {
for (String key : _counts.keySet()) {
CountValue val = COUNT_DATABASE.get(key);
CountValue newVal;
- if (val == null || !val.txid.equals(_id)) {
+ if (val == null || !val.txid.equals(_id.getTransactionId())) {
newVal = new CountValue();
newVal.txid = _id.getTransactionId();
if (val != null) {
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-client/src/jvm/org/apache/storm/container/cgroup/SubSystem.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/container/cgroup/SubSystem.java b/storm-client/src/jvm/org/apache/storm/container/cgroup/SubSystem.java
index e354fb0..99b50b0 100755
--- a/storm-client/src/jvm/org/apache/storm/container/cgroup/SubSystem.java
+++ b/storm-client/src/jvm/org/apache/storm/container/cgroup/SubSystem.java
@@ -68,6 +68,15 @@ public class SubSystem {
public void setEnable(boolean enable) {
this.enable = enable;
}
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((type == null) ? 0 : type.hashCode());
+ result = prime * result + hierarchyID;
+ return result;
+ }
@Override
public boolean equals(Object object) {
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java b/storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java
index 1b7d7f8..e4ecd8b 100644
--- a/storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java
+++ b/storm-client/src/jvm/org/apache/storm/scheduler/SchedulerAssignmentImpl.java
@@ -51,6 +51,15 @@ public class SchedulerAssignmentImpl implements SchedulerAssignment {
}
@Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((topologyId == null) ? 0 : topologyId.hashCode());
+ result = prime * result + ((executorToSlot == null) ? 0 : executorToSlot.hashCode());
+ return result;
+ }
+
+ @Override
public boolean equals(Object other) {
if (other == this) return true;
if (other instanceof SchedulerAssignmentImpl) {
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-client/src/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/DefaultResourceAwareStrategy.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/DefaultResourceAwareStrategy.java b/storm-client/src/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/DefaultResourceAwareStrategy.java
index df3300c..d81ef1e 100644
--- a/storm-client/src/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/DefaultResourceAwareStrategy.java
+++ b/storm-client/src/jvm/org/apache/storm/scheduler/resource/strategies/scheduling/DefaultResourceAwareStrategy.java
@@ -541,7 +541,7 @@ public class DefaultResourceAwareStrategy implements IStrategy {
connections2 += (componentMap.get(childId).execs.size() * o2.execs.size());
}
- if (connections1 > connections1) {
+ if (connections1 > connections2) {
return -1;
} else if (connections1 < connections2) {
return 1;
@@ -567,9 +567,9 @@ public class DefaultResourceAwareStrategy implements IStrategy {
public int compare(Component o1, Component o2) {
int connections1 = o1.execs.size() * thisComp.execs.size();
int connections2 = o2.execs.size() * thisComp.execs.size();
- if (connections1 > connections2) {
+ if (connections1 < connections2) {
return -1;
- } else if (connections1 < connections2) {
+ } else if (connections1 > connections2) {
return 1;
} else {
return o1.id.compareTo(o2.id);
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-client/src/jvm/org/apache/storm/trident/windowing/StoreBasedTridentWindowManager.java
----------------------------------------------------------------------
diff --git a/storm-client/src/jvm/org/apache/storm/trident/windowing/StoreBasedTridentWindowManager.java b/storm-client/src/jvm/org/apache/storm/trident/windowing/StoreBasedTridentWindowManager.java
index 87c1a0f..801a5f2 100644
--- a/storm-client/src/jvm/org/apache/storm/trident/windowing/StoreBasedTridentWindowManager.java
+++ b/storm-client/src/jvm/org/apache/storm/trident/windowing/StoreBasedTridentWindowManager.java
@@ -121,7 +121,7 @@ public class StoreBasedTridentWindowManager extends AbstractTridentWindowManager
}
String trimKey = key.substring(0, lastSepIndex);
int secondLastSepIndex = trimKey.lastIndexOf(WindowsStore.KEY_SEPARATOR);
- if (lastSepIndex < 0) {
+ if (secondLastSepIndex < 0) {
throw new IllegalArgumentException("key "+key+" does not have second key separator '" + WindowsStore.KEY_SEPARATOR + "'");
}
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
index f00c7d5..2da9849 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
@@ -2159,14 +2159,14 @@ public class Nimbus implements Iface, Shutdownable, DaemonCommon {
synchronized(lock) {
Credentials origCreds = state.credentials(id, null);
if (origCreds != null) {
- Map<String, String> orig = origCreds.get_creds();
- Map<String, String> newCreds = new HashMap<>(orig);
+ Map<String, String> origCredsMap = origCreds.get_creds();
+ Map<String, String> newCredsMap = new HashMap<>(origCredsMap);
for (ICredentialsRenewer renewer: renewers) {
LOG.info("Renewing Creds For {} with {}", id, renewer);
- renewer.renew(newCreds, topoConf);
+ renewer.renew(newCredsMap, topoConf);
}
- if (!newCreds.equals(origCreds)) {
- state.setCredentials(id, new Credentials(newCreds), topoConf);
+ if (!newCredsMap.equals(origCredsMap)) {
+ state.setCredentials(id, new Credentials(newCredsMap), topoConf);
}
}
}
http://git-wip-us.apache.org/repos/asf/storm/blob/9c1ab5b8/storm-server/src/main/java/org/apache/storm/daemon/supervisor/ReadClusterState.java
----------------------------------------------------------------------
diff --git a/storm-server/src/main/java/org/apache/storm/daemon/supervisor/ReadClusterState.java b/storm-server/src/main/java/org/apache/storm/daemon/supervisor/ReadClusterState.java
index 81cffc5..8fdd47c 100644
--- a/storm-server/src/main/java/org/apache/storm/daemon/supervisor/ReadClusterState.java
+++ b/storm-server/src/main/java/org/apache/storm/daemon/supervisor/ReadClusterState.java
@@ -195,7 +195,7 @@ public class ReadClusterState implements Runnable, AutoCloseable {
}
if (version == null) {
// ignore
- } else if (version == recordedVersion) {
+ } else if (version.equals(recordedVersion)) {
updateAssignmentVersion.put(topoId, locAssignment);
} else {
VersionedData<Assignment> assignmentVersion = stormClusterState.assignmentInfoWithVersion(topoId, callback);
[4/4] storm git commit: Added STORM-2503 to Changelog
Posted by bo...@apache.org.
Added STORM-2503 to Changelog
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/d7fb0fbc
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/d7fb0fbc
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/d7fb0fbc
Branch: refs/heads/master
Commit: d7fb0fbc4916afe734e69be9705d1b839af4f617
Parents: 83f11bd
Author: Robert Evans <ev...@yahoo-inc.com>
Authored: Mon May 22 12:08:25 2017 -0500
Committer: Robert Evans <ev...@yahoo-inc.com>
Committed: Mon May 22 12:08:25 2017 -0500
----------------------------------------------------------------------
CHANGELOG.md | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/d7fb0fbc/CHANGELOG.md
----------------------------------------------------------------------
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 86d4db3..f3fba33 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,4 +1,5 @@
## 2.0.0
+ * STORM-2503: Fix lgtm.com alerts on equality and comparison operations.
* STORM-2499: Add Serialization plugin for EventHub System Properties
* STORM-2520: AutoHDFS should prefer cluster-wise hdfs kerberos principal
* STORM-2519: Modify AbstractAutoCreds to look for configKeys in both nimbus and topology configs
[2/4] storm git commit: Fixed test failure
Posted by bo...@apache.org.
Fixed test failure
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/0a88924b
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/0a88924b
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/0a88924b
Branch: refs/heads/master
Commit: 0a88924bdaccb6471ed34b35de4d34e217d54d39
Parents: 9c1ab5b
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Authored: Mon May 15 15:55:49 2017 -0500
Committer: Aditya Sharad <ad...@semmle.com>
Committed: Mon May 15 22:11:26 2017 +0100
----------------------------------------------------------------------
.../TestDefaultResourceAwareStrategy.java | 65 +++++---------------
1 file changed, 17 insertions(+), 48 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/0a88924b/storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java
----------------------------------------------------------------------
diff --git a/storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java b/storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java
index aff08c8..14cfa7f 100644
--- a/storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java
+++ b/storm-server/src/test/java/org/apache/storm/scheduler/resource/strategies/scheduling/TestDefaultResourceAwareStrategy.java
@@ -26,6 +26,7 @@ import org.apache.storm.scheduler.Cluster;
import org.apache.storm.scheduler.ExecutorDetails;
import org.apache.storm.scheduler.INimbus;
import org.apache.storm.scheduler.SchedulerAssignmentImpl;
+import org.apache.storm.scheduler.SchedulerAssignment;
import org.apache.storm.scheduler.SupervisorDetails;
import org.apache.storm.scheduler.Topologies;
import org.apache.storm.scheduler.TopologyDetails;
@@ -48,6 +49,7 @@ import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.HashSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
@@ -55,7 +57,6 @@ import java.util.List;
import java.util.Map;
import java.util.TreeSet;
-
public class TestDefaultResourceAwareStrategy {
private static final Logger LOG = LoggerFactory.getLogger(TestDefaultResourceAwareStrategy.class);
@@ -112,55 +113,23 @@ public class TestDefaultResourceAwareStrategy {
rs.prepare(conf);
rs.schedule(topologies, cluster);
- Map<String, List<String>> nodeToComps = new HashMap<String, List<String>>();
- for (Map.Entry<ExecutorDetails, WorkerSlot> entry : cluster.getAssignments().get("testTopology-id").getExecutorToSlot().entrySet()) {
- WorkerSlot ws = entry.getValue();
- ExecutorDetails exec = entry.getKey();
- if (!nodeToComps.containsKey(ws.getNodeId())) {
- nodeToComps.put(ws.getNodeId(), new LinkedList<String>());
- }
- nodeToComps.get(ws.getNodeId()).add(topo.getExecutorToComponent().get(exec));
+ HashSet<HashSet<ExecutorDetails>> expectedScheduling = new HashSet<>();
+ expectedScheduling.add(new HashSet<>(Arrays.asList(new ExecutorDetails(0, 0)))); //Spout
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(2, 2), //bolt-1
+ new ExecutorDetails(4, 4), //bolt-2
+ new ExecutorDetails(6, 6)))); //bolt-3
+ expectedScheduling.add(new HashSet<>(Arrays.asList(
+ new ExecutorDetails(1, 1), //bolt-1
+ new ExecutorDetails(3, 3), //bolt-2
+ new ExecutorDetails(5, 5)))); //bolt-3
+ HashSet<HashSet<ExecutorDetails>> foundScheduling = new HashSet<>();
+ SchedulerAssignment assignment = cluster.getAssignmentById("testTopology-id");
+ for (Collection<ExecutorDetails> execs : assignment.getSlotToExecutors().values()) {
+ foundScheduling.add(new HashSet<>(execs));
}
- /**
- * check for correct scheduling
- * Since all the resource availabilites on nodes are the same in the beginining
- * DefaultResourceAwareStrategy can arbitrarily pick one thus we must find if a particular scheduling
- * exists on a node the the cluster.
- */
-
- //one node should have the below scheduling
- List<String> node1 = new LinkedList<>();
- node1.add("spout");
- node1.add("bolt-1");
- node1.add("bolt-2");
- Assert.assertTrue("Check DefaultResourceAwareStrategy scheduling", checkDefaultStrategyScheduling(nodeToComps, node1));
-
- //one node should have the below scheduling
- List<String> node2 = new LinkedList<>();
- node2.add("bolt-3");
- node2.add("bolt-1");
- node2.add("bolt-2");
-
- Assert.assertTrue("Check DefaultResourceAwareStrategy scheduling", checkDefaultStrategyScheduling(nodeToComps, node2));
-
- //one node should have the below scheduling
- List<String> node3 = new LinkedList<>();
- node3.add("bolt-3");
-
- Assert.assertTrue("Check DefaultResourceAwareStrategy scheduling", checkDefaultStrategyScheduling(nodeToComps, node3));
-
- //three used and one node should be empty
- Assert.assertEquals("only three nodes should be used", 3, nodeToComps.size());
- }
-
- private boolean checkDefaultStrategyScheduling(Map<String, List<String>> nodeToComps, List<String> schedulingToFind) {
- for (List<String> entry : nodeToComps.values()) {
- if (schedulingToFind.containsAll(entry) && entry.containsAll(schedulingToFind)) {
- return true;
- }
- }
- return false;
+ Assert.assertEquals(expectedScheduling, foundScheduling);
}
/**
[3/4] storm git commit: Merge branch 'fix/lgtm-alerts' of
https://github.com/adityasharad/storm into STORM-2503
Posted by bo...@apache.org.
Merge branch 'fix/lgtm-alerts' of https://github.com/adityasharad/storm into STORM-2503
STORM-2503: Fix lgtm.com alerts on equality and comparison operations.
Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/83f11bd9
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/83f11bd9
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/83f11bd9
Branch: refs/heads/master
Commit: 83f11bd9e34948ae72ff50a55628298e36e62ec6
Parents: 8a4fe36 0a88924
Author: Robert Evans <ev...@yahoo-inc.com>
Authored: Mon May 22 10:34:57 2017 -0500
Committer: Robert Evans <ev...@yahoo-inc.com>
Committed: Mon May 22 10:34:57 2017 -0500
----------------------------------------------------------------------
.../storm/starter/TransactionalWords.java | 2 +-
.../storm/container/cgroup/SubSystem.java | 9 +++
.../scheduler/SchedulerAssignmentImpl.java | 9 +++
.../DefaultResourceAwareStrategy.java | 6 +-
.../StoreBasedTridentWindowManager.java | 2 +-
.../org/apache/storm/daemon/nimbus/Nimbus.java | 10 +--
.../daemon/supervisor/ReadClusterState.java | 2 +-
.../TestDefaultResourceAwareStrategy.java | 65 +++++---------------
8 files changed, 46 insertions(+), 59 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/83f11bd9/examples/storm-starter/src/jvm/org/apache/storm/starter/TransactionalWords.java
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/storm/blob/83f11bd9/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
----------------------------------------------------------------------