Re: Review Request 34121: Disable state transition logging for unknown tasks.

2015-05-12 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34121/#review83484
---

Ship it!


Master (759ef68) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing @ReviewBot 
retry

- Aurora ReviewBot


On May 12, 2015, 8:40 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34121/
 ---
 
 (Updated May 12, 2015, 8:40 p.m.)
 
 
 Review request for Aurora and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Quieting down state transition logging in preparation for task state 
 reconciliation.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
 2a943cf51d0a41260ada6965cea5f55db4c3f846 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 afb7db8eefa63b84d370877742870acdec58899c 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 b30a0737bb0d60cd8b58f7be0fff5db20f808347 
 
 Diff: https://reviews.apache.org/r/34121/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34121: Disable state transition logging for unknown tasks.

2015-05-12 Thread Maxim Khutornenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34121/#review83477
---


@ReviewBot retry

- Maxim Khutornenko


On May 12, 2015, 8:40 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34121/
 ---
 
 (Updated May 12, 2015, 8:40 p.m.)
 
 
 Review request for Aurora and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Quieting down state transition logging in preparation for task state 
 reconciliation.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
 2a943cf51d0a41260ada6965cea5f55db4c3f846 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 afb7db8eefa63b84d370877742870acdec58899c 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 b30a0737bb0d60cd8b58f7be0fff5db20f808347 
 
 Diff: https://reviews.apache.org/r/34121/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34121: Disable state transition logging for unknown tasks.

2015-05-12 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34121/#review83481
---


What's the motivation here?  Seems like attempted state transitions for unknown 
tasks should be transient and/or represent a bug.  In those cases, i would 
assume we definitely want logging.  Is there somthing i'm overlooking?


src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
https://reviews.apache.org/r/34121/#comment134492

This is pretty weird behavior - 'no logging unless you ask really nicely'.

Rather than the enableLogging flag, how about we let the caller pass a 
Logger, and for the finest-only logging, we have a logger that has its level 
set appropriately?


- Bill Farner


On May 12, 2015, 8:40 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34121/
 ---
 
 (Updated May 12, 2015, 8:40 p.m.)
 
 
 Review request for Aurora and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Quieting down state transition logging in preparation for task state 
 reconciliation.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
 2a943cf51d0a41260ada6965cea5f55db4c3f846 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 afb7db8eefa63b84d370877742870acdec58899c 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 b30a0737bb0d60cd8b58f7be0fff5db20f808347 
 
 Diff: https://reviews.apache.org/r/34121/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 34121: Disable state transition logging for unknown tasks.

2015-05-12 Thread Maxim Khutornenko


 On May 12, 2015, 9:22 p.m., Bill Farner wrote:
  What's the motivation here?  Seems like attempted state transitions for 
  unknown tasks should be transient and/or represent a bug.  In those cases, 
  i would assume we definitely want logging.  Is there somthing i'm 
  overlooking?

The idea is to avoid any logging for any state transitions coming from task 
reconciliation. I just realized though that the current approach addresses only 
implicit reconciliation and will still log explicit state transitions. I am 
going to rework this diff to support a higher level switch. Ignore it for now.


 On May 12, 2015, 9:22 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java, line 
  567
  https://reviews.apache.org/r/34121/diff/1/?file=956801#file956801line567
 
  This is pretty weird behavior - 'no logging unless you ask really 
  nicely'.
  
  Rather than the enableLogging flag, how about we let the caller pass a 
  Logger, and for the finest-only logging, we have a logger that has its 
  level set appropriately?

If the logger is passed from the caller, we will have no way to enable logging 
when needed (e.g. for debugging state reconciliation task transitions). Having 
a static logger let's us dynamically override (reenable) TaskStateMachine 
logging via /logconfig endpoint.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34121/#review83481
---


On May 12, 2015, 8:40 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34121/
 ---
 
 (Updated May 12, 2015, 8:40 p.m.)
 
 
 Review request for Aurora and Bill Farner.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Quieting down state transition logging in preparation for task state 
 reconciliation.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 
 2a943cf51d0a41260ada6965cea5f55db4c3f846 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 afb7db8eefa63b84d370877742870acdec58899c 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 b30a0737bb0d60cd8b58f7be0fff5db20f808347 
 
 Diff: https://reviews.apache.org/r/34121/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
 
 
 Thanks,
 
 Maxim Khutornenko