[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures

2018-10-01 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/storm/pull/2852


---

[GitHub] storm pull request #2852: STORM-3237 track Nimbus mkAssignment failures

2018-09-28 Thread agresch 
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 pull request #2852: STORM-3237 track Nimbus mkAssignment failures

2018-09-28 Thread revans2 
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

2018-09-28 Thread kishorvpatil 
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

2018-09-28 Thread agresch 
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 pull request #2852: STORM-3237 track Nimbus mkAssignment failures

2018-09-28 Thread kishorvpatil 
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 pull request #2852: STORM-3237 track Nimbus mkAssignment failures

2018-09-27 Thread agresch 
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 
Date:   2018-09-27T18:30:59Z

STORM-3237 track Nimbus mkAssignment failures




---