You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by danny0405 <gi...@git.apache.org> on 2018/04/01 03:12:11 UTC
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
GitHub user danny0405 opened a pull request:
https://github.com/apache/storm/pull/2619
[STORM-3018] Fix integration test DemoTest#testExclamationTopology fa…
See [STORM-3018](https://issues.apache.org/jira/projects/STORM/issues/STORM-3018?filter=allopenissues)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/danny0405/storm fix-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2619.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 #2619
----
commit 433632744a4125467ff4db3f74780583c6b9123c
Author: chenyuzhao <ch...@...>
Date: 2018-04-01T03:08:19Z
[STORM-3018] Fix integration test DemoTest#testExclamationTopology fail problem
----
---
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2619#discussion_r178456030
--- Diff: conf/defaults.yaml ---
@@ -195,7 +195,7 @@ worker.log.level.reset.poll.secs: 30
# control how many worker receiver threads we need per worker
topology.worker.receiver.thread.count: 1
-# Executor metrics reporting interval.
+# Executor metrics reporting interval, keep default val same with topology.builtin.metrics.bucket.size.secs
--- End diff --
Ok, got your idea.
---
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2619#discussion_r178453508
--- Diff: conf/defaults.yaml ---
@@ -195,7 +195,7 @@ worker.log.level.reset.poll.secs: 30
# control how many worker receiver threads we need per worker
topology.worker.receiver.thread.count: 1
-# Executor metrics reporting interval.
+# Executor metrics reporting interval, keep default val same with topology.builtin.metrics.bucket.size.secs
--- End diff --
Nit: Could you add to the comment the reason why these must be kept in sync?
---
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
Posted by srdo <gi...@git.apache.org>.
Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2619#discussion_r178455098
--- Diff: conf/defaults.yaml ---
@@ -195,7 +195,7 @@ worker.log.level.reset.poll.secs: 30
# control how many worker receiver threads we need per worker
topology.worker.receiver.thread.count: 1
-# Executor metrics reporting interval.
+# Executor metrics reporting interval, keep default val same with topology.builtin.metrics.bucket.size.secs
--- End diff --
Thanks, but I meant add a note to the comment. I can't tell from the current comment why the two properties need to be in sync, so I'd have to look up this PR to see why.
---
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
Posted by danny0405 <gi...@git.apache.org>.
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2619#discussion_r178454116
--- Diff: conf/defaults.yaml ---
@@ -195,7 +195,7 @@ worker.log.level.reset.poll.secs: 30
# control how many worker receiver threads we need per worker
topology.worker.receiver.thread.count: 1
-# Executor metrics reporting interval.
+# Executor metrics reporting interval, keep default val same with topology.builtin.metrics.bucket.size.secs
--- End diff --
Because:
1. the internal metrics interval is set up by conf: topology.builtin.metrics.bucket.size.secs, we should keep sync with it
2. Now use ZK to store our metrics and the ui only show built in metrics, the metrics consumer's collecting interval are also default to 60 seconds
---
[GitHub] storm pull request #2619: [STORM-3018] Fix integration test DemoTest#testExc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2619
---