You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by agresch <gi...@git.apache.org> on 2018/09/27 18:31:47 UTC
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
GitHub user agresch opened a pull request:
https://github.com/apache/storm/pull/2852
STORM-3237 track Nimbus mkAssignment failures
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/agresch/storm agresch_mkAssignments_fail
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/2852.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 #2852
----
commit eb7886e1c53682e4c42b3951bf99cf35050f3cfa
Author: Aaron Gresch <ag...@...>
Date: 2018-09-27T18:30:59Z
STORM-3237 track Nimbus mkAssignment failures
----
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by agresch <gi...@git.apache.org>.
Github user agresch commented on a diff in the pull request:
https://github.com/apache/storm/pull/2852#discussion_r221264813
--- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -2850,6 +2853,7 @@ public void launchServer() throws Exception {
}
doCleanup();
} catch (Exception e) {
+ this.mkAssignmentsErrors.mark();
--- End diff --
My thought was any exception would be worth investigating in any case. I would hope that these issues would be rare. Let me know if you want it more narrowly applied just to mkAssignment.
---
[GitHub] storm issue #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by agresch <gi...@git.apache.org>.
Github user agresch commented on the issue:
https://github.com/apache/storm/pull/2852
@kishorvpatil - made the change you requested.
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/2852#discussion_r221288492
--- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -2850,6 +2853,7 @@ public void launchServer() throws Exception {
}
doCleanup();
} catch (Exception e) {
+ this.mkAssignmentsErrors.mark();
--- End diff --
Yes, let's apply this narrowly to just `mkAssignments`
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/2852
---
[GitHub] storm issue #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/2852
Oh wait sorry. I forgot that we added docs for all of the metrics. Could you update docs/ClusterMetrics.md to include this too?
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2852#discussion_r221331973
--- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -2162,7 +2165,12 @@ private boolean isReadyForMKAssignments() throws Exception {
}
private void mkAssignments() throws Exception {
- mkAssignments(null);
+ try {
+ mkAssignments(null);
--- End diff --
Do we want to make the change inside `mkAssignments(String scratchTopoId)` so we can count errors during the rebalance command as well?
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by kishorvpatil <gi...@git.apache.org>.
Github user kishorvpatil commented on a diff in the pull request:
https://github.com/apache/storm/pull/2852#discussion_r221257606
--- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -2850,6 +2853,7 @@ public void launchServer() throws Exception {
}
doCleanup();
} catch (Exception e) {
+ this.mkAssignmentsErrors.mark();
--- End diff --
should we have separate tracker for cleanup failures and `doCleanup()` ?
---
[GitHub] storm issue #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by agresch <gi...@git.apache.org>.
Github user agresch commented on the issue:
https://github.com/apache/storm/pull/2852
@revans2 @kishorvpatil - made the change requested by Bobby.
---
[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by agresch <gi...@git.apache.org>.
Github user agresch commented on a diff in the pull request:
https://github.com/apache/storm/pull/2852#discussion_r221349119
--- Diff: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java ---
@@ -2162,7 +2165,12 @@ private boolean isReadyForMKAssignments() throws Exception {
}
private void mkAssignments() throws Exception {
- mkAssignments(null);
+ try {
+ mkAssignments(null);
--- End diff --
I can change that.
---
[GitHub] storm issue #2852: STORM-3237 track Nimbus mkAssignment failures
Posted by agresch <gi...@git.apache.org>.
Github user agresch commented on the issue:
https://github.com/apache/storm/pull/2852
@revans2 - added documentation
---