You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by Gvain <gi...@git.apache.org> on 2014/05/16 09:18:07 UTC
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
GitHub user Gvain opened a pull request:
https://github.com/apache/incubator-storm/pull/117
STORM-312 : add storm monitor tools to monitor throughtput interactively
This cmd line tool 'storm monitor' will use Nimbus.Client to get throughput information from Nimbus Server.
1)One can specify topology's name, component's name to monitor it's throughput interactively.
2) It will statistics 'emit' or 'transferred' throughput in a given time window and print it in a given time frequency
The implementation will be much like yahoo's storm-perf-test (http://yahooeng.tumblr.com/post/64758709722/making-storm-fly-with-netty)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Gvain/incubator-storm storm-monitor-cmd
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-storm/pull/117.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #117
----
commit d080e043d15d710d6729963bed9cb078b7dce78e
Author: Li Jiahong <gv...@users.noreply.github.com>
Date: 2014-03-05T04:36:22Z
Merge pull request #1 from apache/master
update from apache/incubator-storm
commit 33715a2b4613742dfb3294a097874312d4e6c3b6
Author: Li Jiahong <gv...@users.noreply.github.com>
Date: 2014-04-03T07:46:40Z
Merge pull request #2 from apache/master
update from apache/incubator-storm
commit b2a5102917ebb4b32d42b9b722427500f939e8b5
Author: Li Jiahong <gv...@users.noreply.github.com>
Date: 2014-04-15T08:05:55Z
Merge pull request #3 from apache/master
update from apache/incubator-storm
commit a45b53a167cd48a275bbb21279bcaa9e9437b97b
Author: Li Jiahong <gv...@users.noreply.github.com>
Date: 2014-05-07T12:09:34Z
Merge pull request #4 from apache/master
update from apache/incubator-storm
commit edcaf71b304cc5196eec48035b06d15aaa8df433
Author: jiahong.ljh <ji...@alibaba-inc.com>
Date: 2014-05-16T07:14:05Z
STORM-312 add storm monitor command line tools
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47515396
Sorry for putting you in such bad situation. I simply thought your code could be stepped forward to be a useful monitoring tool. And Thanks for your carefully review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14267820
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
--- End diff --
Can we separate out the argument parsing from the monitor class. The rest of the code uses clojure.tools.cli and I think it would be more consistent to use that then add in a new dependency. It would also allow the Monitor class to be separated out to be used as a library.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47795127
Looks good, I am +1, but like I said I would like some others to take a look too.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14635222
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ private static class Poller {
+ long startTime = 0;
+ long pollMs = 0;
+
+ public long nextPoll() {
+ long now = System.currentTimeMillis();
+ long cycle = (now - startTime) / pollMs;
+ long wakeupTime = startTime + (pollMs * (cycle + 1));
+ long sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ try {
+ Thread.sleep(sleepTime);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
--- End diff --
That's right. Very good suggestion.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14439496
--- Diff: bin/storm ---
@@ -390,6 +390,19 @@ def print_classpath():
"""
print get_classpath([])
+def monitor(*args):
+ """Syntax: [storm monitor [-i interval-secs] -t topology-name -m component-id [-s stream-id] [-w [emitted | transferred]]*]
--- End diff --
Yep, it is better to do in this way, I will fix it in time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47945649
@miguno Could you spare time to take a look at this please ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by danehammer <gi...@git.apache.org>.
Github user danehammer commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14616905
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ private static class Poller {
+ long startTime = 0;
+ long pollMs = 0;
+
+ public long nextPoll() {
+ long now = System.currentTimeMillis();
+ long cycle = (now - startTime) / pollMs;
+ long wakeupTime = startTime + (pollMs * (cycle + 1));
+ long sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ try {
+ Thread.sleep(sleepTime);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
--- End diff --
Probably want better logging here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14337281
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
--- End diff --
Sorry for putting you in this bad position. I simply thought that your code could be stepped forward to be a useful monitoring tool. And thanks for your carefully review.
As you say, it is much better to use clojure.tools.cli. I will fix this in time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14267944
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
--- End diff --
s/topologie/topology/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47695920
The code changes you made look good. I have a few new comments, also I would like another committer to take a look at the code, as I did kind of write some of it, although you have cleaned it up enough that it no longer looks like my code, which is a good thing.
Thanks for putting this together.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14268407
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
+ MetricsState state = new MetricsState();
+ long pollMs = poll * 1000;
+ long now = System.currentTimeMillis();
+ state.lastTime = now;
+ long startTime = now;
+ long cycle, sleepTime, wakeupTime;
+
+ while (metrics(client, name, component, stat, now, state, "WAITING")) {
--- End diff --
Previously the "WAITING" was intended to wait for the topology to be fully up before we started to collect metrics on it because it was used as a benchmark. I am not sure that it is needed just for displaying the current status.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14268626
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
+ MetricsState state = new MetricsState();
+ long pollMs = poll * 1000;
+ long now = System.currentTimeMillis();
+ state.lastTime = now;
+ long startTime = now;
+ long cycle, sleepTime, wakeupTime;
+
+ while (metrics(client, name, component, stat, now, state, "WAITING")) {
+ now = System.currentTimeMillis();
+ cycle = (now - startTime)/pollMs;
+ wakeupTime = startTime + (pollMs * (cycle + 1));
+ sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ Thread.sleep(sleepTime);
+ }
+ now = System.currentTimeMillis();
+ }
+
+ now = System.currentTimeMillis();
+ cycle = (now - startTime)/pollMs;
+ wakeupTime = startTime + (pollMs * (cycle + 1));
+ sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ Thread.sleep(sleepTime);
+ }
+ now = System.currentTimeMillis();
+ do {
+ metrics(client, name, component, stat, now, state, "RUNNING");
+ now = System.currentTimeMillis();
+ cycle = (now - startTime)/pollMs;
+ wakeupTime = startTime + (pollMs * (cycle + 1));
+ sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ Thread.sleep(sleepTime);
+ }
+ now = System.currentTimeMillis();
+ } while (true);
+ }
+
+ public boolean metrics(Nimbus.Client client, String name, String component, String stat, long now, MetricsState state, String message) throws Exception {
+ long totalStatted = 0;
+
+ boolean topologyFound = false;
+ boolean componentFound = false;
+ int slotsUsed = 0;
+ int executors = 0;
+ int executorsWithMetrics = 0;
+ ClusterSummary summary = client.getClusterInfo();
+ for (TopologySummary ts: summary.get_topologies()) {
+ if (name.equals(ts.get_name())) {
+ topologyFound = true;
+ slotsUsed = ts.get_num_workers();
+ String id = ts.get_id();
+ TopologyInfo info = client.getTopologyInfo(id);
+ for (ExecutorSummary es: info.get_executors()) {
+ if (component.equals(es.get_component_id())) {
+ componentFound = true;
+ executors ++;
+ ExecutorStats stats = es.get_stats();
+ if (stats != null) {
+ Map<String,Map<String,Long>> statted =
+ "emitted".equals(stat) ? stats.get_emitted() : stats.get_transferred();
+ if ( statted != null) {
+ Map<String, Long> e2 = statted.get(":all-time");
+ if (e2 != null) {
+ executorsWithMetrics++;
+ //topology messages are always on the default stream, so just count those
+ Long dflt = e2.get("default");
--- End diff --
This was only true for the simple test that we had set up. We probably want some way to handle more then just default.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14422603
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,178 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ private static class Poller {
+ long startTime = 0;
+ long pollMs = 0;
+
+ public long nextPoll() {
+ long now = System.currentTimeMillis();
+ long cycle = (now - startTime) / pollMs;
+ long wakeupTime = startTime + (pollMs * (cycle + 1));
+ long sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ try {
+ Thread.sleep(sleepTime);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ }
+ now = System.currentTimeMillis();
+ return now;
+ }
+ }
+
+ public void metrics(Nimbus.Client client) throws Exception {
+ if (_interval <= 0) {
+ throw new IllegalArgumentException("poll interval must be positive");
+ }
+
+ if (_topology == null || _topology.isEmpty()) {
+ throw new IllegalArgumentException("topology name must be something");
+ }
+
+ if (_component == null || _component.isEmpty()) {
+ throw new IllegalArgumentException("component name must be something");
+ }
+
+ if (_stream == null || _stream.isEmpty()) {
+ throw new IllegalArgumentException("stream name must be something");
+ }
+
+ if ( !WATCH_TRANSFERRED.equals(_watch) && !WATCH_EMITTED.equals(_watch)) {
+ throw new IllegalArgumentException("watch item must either be transferred or emitted");
+ }
+ System.out.println("topology\tslots\texecutors\texecutorsWithMetrics\tcomponent\tstream\ttime-diff ms\t" + _watch + "\tthroughput (Kt/s)");
--- End diff --
I'm not sure if slots, executors, and executorsWithMetrics make all that much since here. Perhaps parallelism of the component would be more informative.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14268446
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
+ MetricsState state = new MetricsState();
+ long pollMs = poll * 1000;
+ long now = System.currentTimeMillis();
+ state.lastTime = now;
+ long startTime = now;
+ long cycle, sleepTime, wakeupTime;
+
+ while (metrics(client, name, component, stat, now, state, "WAITING")) {
--- End diff --
So if we remove the waiting, we can remove the boolean return value from metrics and it should remove a lot of checks down in metrics as well.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14422509
--- Diff: bin/storm ---
@@ -390,6 +390,19 @@ def print_classpath():
"""
print get_classpath([])
+def monitor(*args):
+ """Syntax: [storm monitor [-i interval-secs] -t topology-name -m component-id [-s stream-id] [-w [emitted | transferred]]*]
--- End diff --
I would prefer to see topology-name be an argument not an option. It matches better with the other commands list kill and rebalance.
I would love it if we could make component-id optional, so that it would do one line per component if not supplied, or if you could do a list of component ids.
And I'm not sure that we want a '*' after the '[emitted | transferred]' to me that means that you can supply 0 or more of them, but the reality is only the last one will be used like stream-id.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47281922
Sorry this has taken me so long to review. I keep hoping I'll find more time, but never seem to.
I really shouldn't review code that I wrote myself, it makes me feel bad. You should find someone else to borrow code from :).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14439538
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,178 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ private static class Poller {
+ long startTime = 0;
+ long pollMs = 0;
+
+ public long nextPoll() {
+ long now = System.currentTimeMillis();
+ long cycle = (now - startTime) / pollMs;
+ long wakeupTime = startTime + (pollMs * (cycle + 1));
+ long sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ try {
+ Thread.sleep(sleepTime);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ }
+ now = System.currentTimeMillis();
+ return now;
+ }
+ }
+
+ public void metrics(Nimbus.Client client) throws Exception {
+ if (_interval <= 0) {
+ throw new IllegalArgumentException("poll interval must be positive");
+ }
+
+ if (_topology == null || _topology.isEmpty()) {
+ throw new IllegalArgumentException("topology name must be something");
+ }
+
+ if (_component == null || _component.isEmpty()) {
+ throw new IllegalArgumentException("component name must be something");
+ }
+
+ if (_stream == null || _stream.isEmpty()) {
+ throw new IllegalArgumentException("stream name must be something");
+ }
+
+ if ( !WATCH_TRANSFERRED.equals(_watch) && !WATCH_EMITTED.equals(_watch)) {
+ throw new IllegalArgumentException("watch item must either be transferred or emitted");
+ }
+ System.out.println("topology\tslots\texecutors\texecutorsWithMetrics\tcomponent\tstream\ttime-diff ms\t" + _watch + "\tthroughput (Kt/s)");
--- End diff --
Yes, parallelism makes sense
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by danehammer <gi...@git.apache.org>.
Github user danehammer commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14616888
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
--- End diff --
Since we're already introducing changes in Clojure, how would you feel about implementing some (or all) of this in Clojure? I feel like MetricsState and Poller are bad fits for Java, and that it could be a great place to use Clojure's concurrency features, such as an [agent](http://clojure.org/agents).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by danehammer <gi...@git.apache.org>.
Github user danehammer commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14617140
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ private static class Poller {
+ long startTime = 0;
+ long pollMs = 0;
+
+ public long nextPoll() {
+ long now = System.currentTimeMillis();
+ long cycle = (now - startTime) / pollMs;
+ long wakeupTime = startTime + (pollMs * (cycle + 1));
+ long sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ try {
+ Thread.sleep(sleepTime);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
--- End diff --
Also this wouldn't break out of our do/while loop below, which is probably surprising.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14635548
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
--- End diff --
Yes , the MerticState and Poller class are not well fits for Java. I will change them to better fits for Java.
But I don't quite see what concurrency features we really needs here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by danehammer <gi...@git.apache.org>.
Github user danehammer commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14616431
--- Diff: storm-core/src/clj/backtype/storm/command/monitor.clj ---
@@ -0,0 +1,37 @@
+;; 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.
+(ns backtype.storm.command.monitor
+ (:use [clojure.tools.cli :only [cli]])
+ (:use [backtype.storm thrift config log])
--- End diff --
I know we're trying to move to more idiomatic clojure, so here it'd be more readable to specify what you're using (which it looks like you're not using very much from each of these).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-48801447
@revans2 @d2r Thanks for your time.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14268294
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
+ MetricsState state = new MetricsState();
+ long pollMs = poll * 1000;
+ long now = System.currentTimeMillis();
+ state.lastTime = now;
+ long startTime = now;
+ long cycle, sleepTime, wakeupTime;
+
+ while (metrics(client, name, component, stat, now, state, "WAITING")) {
+ now = System.currentTimeMillis();
+ cycle = (now - startTime)/pollMs;
+ wakeupTime = startTime + (pollMs * (cycle + 1));
+ sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ Thread.sleep(sleepTime);
+ }
+ now = System.currentTimeMillis();
--- End diff --
Possibly have it be a static class that could encompass all of the logic in a generic way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by nathanmarz <gi...@git.apache.org>.
Github user nathanmarz commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47989471
@danehammer Actually the opposite, it's encouraged
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-48733303
The changes look good to me I am still a +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by danehammer <gi...@git.apache.org>.
Github user danehammer commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47949213
Just an attempt at manners: is there anything wrong with jumping in and making comments as a fellow storm user (novice contributor)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14268190
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
+ private String _name;
+
+ @Option(name="--component", aliases={"--componentName"}, metaVar="NAME",
+ usage="component name of the topology")
+ private String _component;
+
+ @Option(name="--stat", aliases={"--statItem"}, metaVar="ITEM",
+ usage="stat item [emitted | transferred]")
+ private String _stat = "emitted";
+
+ private static class MetricsState {
+ long lastStatted = 0;
+ long lastTime = 0;
+ }
+
+ public void metrics(Nimbus.Client client, int poll, String name, String component, String stat) throws Exception {
+ System.out.println("status\ttopologie\tslots\tcomponent\texecutors\texecutorsWithMetrics\ttime-diff ms\t" + stat + "\tthroughput (Kt/s)");
+ MetricsState state = new MetricsState();
+ long pollMs = poll * 1000;
+ long now = System.currentTimeMillis();
+ state.lastTime = now;
+ long startTime = now;
+ long cycle, sleepTime, wakeupTime;
+
+ while (metrics(client, name, component, stat, now, state, "WAITING")) {
+ now = System.currentTimeMillis();
+ cycle = (now - startTime)/pollMs;
+ wakeupTime = startTime + (pollMs * (cycle + 1));
+ sleepTime = wakeupTime - now;
+ if (sleepTime > 0) {
+ Thread.sleep(sleepTime);
+ }
+ now = System.currentTimeMillis();
--- End diff --
This sleep code gets used in 3 different places. It would be good to have it be pulled out into a separate function, and have the state become member variables.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-storm/pull/117
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14821664
--- Diff: storm-core/src/jvm/backtype/storm/utils/Monitor.java ---
@@ -0,0 +1,211 @@
+/**
+ * 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 backtype.storm.utils;
+
+import backtype.storm.generated.*;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+public class Monitor {
+ private static final String WATCH_TRANSFERRED = "transferred";
+ private static final String WATCH_EMITTED = "emitted";
+
+ private int _interval = 4;
+ private String _topology;
+ private String _component;
+ private String _stream;
+ private String _watch;
+
+ private static class MetricsState {
--- End diff --
I'm not sure we need agents as the code is not currently multi-threaded. Threads feel like they would be an unneeded complication. I do agree that the code could be a lot more concise in Clojure, but I personally don't see a reason to rewrite it at this point. If someone wants to, especially to start learning Clojure, then sure go right ahead.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-47747187
Your comments are quite appropriated, Thank you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14267890
--- Diff: storm-core/src/jvm/backtype/storm/command/Monitor.java ---
@@ -0,0 +1,197 @@
+/**
+ * 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 backtype.storm.command;
+
+import backtype.storm.generated.*;
+import backtype.storm.utils.NimbusClient;
+import backtype.storm.utils.Utils;
+import org.kohsuke.args4j.CmdLineException;
+import org.kohsuke.args4j.CmdLineParser;
+import org.kohsuke.args4j.Option;
+
+import java.util.Map;
+
+public class Monitor {
+ @Option(name="--help", aliases={"-h"}, usage="print help message")
+ private boolean _help = false;
+
+ @Option(name="--interval", aliases={"-i"}, usage="poll frequency in seconds")
+ private int _interval = 4;
+
+ @Option(name="--name", aliases={"--topologyName"}, metaVar="NAME",
+ usage="base name of the topology (numbers may be appended to the end)")
--- End diff --
Can we reword this. It is monitoring a single topology now, not multiple ones with generated names.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-48012674
@danehammer , Any comments are quite appreciated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by d2r <gi...@git.apache.org>.
Github user d2r commented on the pull request:
https://github.com/apache/incubator-storm/pull/117#issuecomment-48739149
Code looks good. Tried out the different options with word_count, seems to work.
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-storm pull request: STORM-312 : add storm monitor tools ...
Posted by Gvain <gi...@git.apache.org>.
Github user Gvain commented on a diff in the pull request:
https://github.com/apache/incubator-storm/pull/117#discussion_r14635009
--- Diff: storm-core/src/clj/backtype/storm/command/monitor.clj ---
@@ -0,0 +1,37 @@
+;; 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.
+(ns backtype.storm.command.monitor
+ (:use [clojure.tools.cli :only [cli]])
+ (:use [backtype.storm thrift config log])
--- End diff --
That's right. We are using :only [with-configured-nimbus-connection] from thift.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---