Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Maxim Khutornenko 


> On June 23, 2015, 7:46 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, 
> > line 242
> > 
> >
> > s/manager/handler/

Done.


- Maxim


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


On June 23, 2015, 6:35 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 6:35 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> ae31bdb587c0ea8380fc15c71fcf34e7e19405a6 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 17b358533853596f0a577958a3a7303fa61bf299 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Maxim Khutornenko 

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

(Updated June 23, 2015, 8:39 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Bill's comment.


Bugs: AURORA-1334
https://issues.apache.org/jira/browse/AURORA-1334


Repository: aurora


Description
---

Replacing TaskLauncher abstraction with TaskStatusUpdateManager.


Diffs (updated)
-

  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
ae31bdb587c0ea8380fc15c71fcf34e7e19405a6 
  src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
cd55a6ee7424873c3e615d95422c9ecab6442f46 
  src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
17b358533853596f0a577958a3a7303fa61bf299 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
  src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
126001a90c819f9b3bfd448a060102bcef8b2a35 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 

Diff: https://reviews.apache.org/r/35761/diff/


Testing
---


Thanks,

Maxim Khutornenko

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Bill Farner 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java (line 
233)


s/manager/handler/


- Bill Farner


On June 23, 2015, 6:35 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 6:35 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> ae31bdb587c0ea8380fc15c71fcf34e7e19405a6 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 17b358533853596f0a577958a3a7303fa61bf299 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Aurora ReviewBot 

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

Ship it!


Master (68c4620) 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 June 23, 2015, 6:35 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 6:35 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> ae31bdb587c0ea8380fc15c71fcf34e7e19405a6 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 17b358533853596f0a577958a3a7303fa61bf299 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Maxim Khutornenko 

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

(Updated June 23, 2015, 6:35 p.m.)


Review request for Aurora and Bill Farner.


Changes
---

Rebased and addressed Bill's comments.


Bugs: AURORA-1334
https://issues.apache.org/jira/browse/AURORA-1334


Repository: aurora


Description (updated)
---

Replacing TaskLauncher abstraction with TaskStatusUpdateManager.


Diffs (updated)
-

  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
ae31bdb587c0ea8380fc15c71fcf34e7e19405a6 
  src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
cd55a6ee7424873c3e615d95422c9ecab6442f46 
  src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
17b358533853596f0a577958a3a7303fa61bf299 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
  src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
126001a90c819f9b3bfd448a060102bcef8b2a35 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 

Diff: https://reviews.apache.org/r/35761/diff/


Testing
---


Thanks,

Maxim Khutornenko

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Maxim Khutornenko 


> On June 23, 2015, 5:55 p.m., Bill Farner wrote:
> > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java, line 
> > 167
> > 
> >
> > Accessing the Impl suggests an encapsulagion problem.  Can you use the 
> > interface instead?

Yes, changed to AbstractExecutionThreadService.


> On June 23, 2015, 5:55 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java, 
> > line 21
> > 
> >
> > How about `TaskStatusReceiver` or `TaskStatusHandler`?

Going with TaskStatusHandler as it was on my list as well.


> On June 23, 2015, 5:55 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 176
> > 
> >
> > Please include a comment explaining the rationale behind the default.

Done.


- Maxim


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


On June 23, 2015, 2:29 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 2:29 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> Will not apply cleanly. Depends on https://reviews.apache.org/r/35760/.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 6edec22aba135136c4ce4066b9535f23de077db7 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 8bcac6c4f93eaca7250d43ab81a72102b91bf836 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Bill Farner 

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


LGTM overall, will give a ship once the rebase comes out green.


src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java (line 164)


Accessing the Impl suggests an encapsulagion problem.  Can you use the 
interface instead?



src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java (line 21)


How about `TaskStatusReceiver` or `TaskStatusHandler`?



src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java (line 176)


Please include a comment explaining the rationale behind the default.


- Bill Farner


On June 23, 2015, 2:29 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 2:29 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> Will not apply cleanly. Depends on https://reviews.apache.org/r/35760/.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 6edec22aba135136c4ce4066b9535f23de077db7 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 8bcac6c4f93eaca7250d43ab81a72102b91bf836 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Aurora ReviewBot 

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


This patch does not apply cleanly on master (2df2db9), do you need to rebase?

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

- Aurora ReviewBot


On June 23, 2015, 2:29 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35761/
> ---
> 
> (Updated June 23, 2015, 2:29 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-1334
> https://issues.apache.org/jira/browse/AURORA-1334
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Replacing TaskLauncher abstraction with TaskStatusUpdateManager.
> 
> Will not apply cleanly. Depends on https://reviews.apache.org/r/35760/.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
>   src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
> 6edec22aba135136c4ce4066b9535f23de077db7 
>   src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
> cd55a6ee7424873c3e615d95422c9ecab6442f46 
>   src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
> 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 8bcac6c4f93eaca7250d43ab81a72102b91bf836 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
> 126001a90c819f9b3bfd448a060102bcef8b2a35 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 
> 
> Diff: https://reviews.apache.org/r/35761/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Review Request 35761: Refactoring TaskLauncher.

2015-06-23 Thread Maxim Khutornenko 

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

Review request for Aurora and Bill Farner.


Bugs: AURORA-1334
https://issues.apache.org/jira/browse/AURORA-1334


Repository: aurora


Description
---

Replacing TaskLauncher abstraction with TaskStatusUpdateManager.

Will not apply cleanly. Depends on https://reviews.apache.org/r/35760/.


Diffs
-

  src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
4c63cc7af2fb5702fa649ce2cd4ee619139223e2 
  src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 
6edec22aba135136c4ce4066b9535f23de077db7 
  src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 
cd55a6ee7424873c3e615d95422c9ecab6442f46 
  src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 
6bfbf0c76399d569ac762a7b433a6c576bf0ee87 
  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
8bcac6c4f93eaca7250d43ab81a72102b91bf836 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
4f7a1bec8c32aef4b5f435c760f69de13888ea9d 
  src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 
126001a90c819f9b3bfd448a060102bcef8b2a35 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 

Diff: https://reviews.apache.org/r/35761/diff/


Testing
---


Thanks,

Maxim Khutornenko