You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Ben Mahler <be...@gmail.com> on 2015/05/02 00:55:14 UTC
Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 10, 2015, 12:10 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 211
> > <https://reviews.apache.org/r/33689/diff/3/?file=951771#file951771line211>
> >
> > You mentioned that updates should be processed in FIFO order. As we have effectively lost the entire batch here, do we have to clear the pendingUpdates queue to prevent our of order processing?
Out of order processing occurs when the scheduler processes updates in a different order than the order in which they were delivered (via the statusUpdate() callback). Dropping a batch doesn't cause a re-ordering, unless there's a bug that I'm missing here. Why do you think that ordering is an issue when a batch is dropped?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83184
-----------------------------------------------------------
On May 7, 2015, 12:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 12:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 10, 2015, 12:10 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 211
> > <https://reviews.apache.org/r/33689/diff/3/?file=951771#file951771line211>
> >
> > You mentioned that updates should be processed in FIFO order. As we have effectively lost the entire batch here, do we have to clear the pendingUpdates queue to prevent our of order processing?
>
> Ben Mahler wrote:
> Out of order processing occurs when the scheduler processes updates in a different order than the order in which they were delivered (via the statusUpdate() callback). Dropping a batch doesn't cause a re-ordering, unless there's a bug that I'm missing here. Why do you think that ordering is an issue when a batch is dropped?
Chatting with Maxim, it probably wasn't clear for you that status updates are not pipelined. That is, only 1 update for a task will be in-flight at a time. Until you acknowledge U1, you will not be sent U2. If this were not the case it would definitely lead to re-odering as you said. When pipelining is added, it will be done in a manner that doesn't break existing schedulers. :)
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83184
-----------------------------------------------------------
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Stephan Erb <st...@dev.static-void.de>.
> On May 10, 2015, 2:10 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 211
> > <https://reviews.apache.org/r/33689/diff/3/?file=951771#file951771line211>
> >
> > You mentioned that updates should be processed in FIFO order. As we have effectively lost the entire batch here, do we have to clear the pendingUpdates queue to prevent our of order processing?
>
> Ben Mahler wrote:
> Out of order processing occurs when the scheduler processes updates in a different order than the order in which they were delivered (via the statusUpdate() callback). Dropping a batch doesn't cause a re-ordering, unless there's a bug that I'm missing here. Why do you think that ordering is an issue when a batch is dropped?
>
> Ben Mahler wrote:
> Chatting with Maxim, it probably wasn't clear for you that status updates are not pipelined. That is, only 1 update for a task will be in-flight at a time. Until you acknowledge U1, you will not be sent U2. If this were not the case it would definitely lead to re-odering as you said. When pipelining is added, it will be done in a manner that doesn't break existing schedulers. :)
That answers my question. Thanks!
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83184
-----------------------------------------------------------
On May 11, 2015, 8:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 8:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83184
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134073>
You mentioned that updates should be processed in FIFO order. As we have effectively lost the entire batch here, do we have to clear the pendingUpdates queue to prevent our of order processing?
- Stephan Erb
On May 7, 2015, 2:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 2:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Zameer Manji <zm...@apache.org>.
> On May 7, 2015, 3:24 p.m., Maxim Khutornenko wrote:
> > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java, line 191
> > <https://reviews.apache.org/r/33689/diff/3/?file=951768#file951768line191>
> >
> > Delete TODO.
+1
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82916
-----------------------------------------------------------
On May 6, 2015, 5:27 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 6, 2015, 5:27 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java, line 191
> > <https://reviews.apache.org/r/33689/diff/3/?file=951768#file951768line191>
> >
> > Delete TODO.
>
> Zameer Manji wrote:
> +1
Done.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 60
> > <https://reviews.apache.org/r/33689/diff/3/?file=951770#file951770line60>
> >
> > @Positive
Done.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, lines 66-67
> > <https://reviews.apache.org/r/33689/diff/3/?file=951770#file951770line66>
> >
> > Suggest rephrasing adding a notion that this is the argument controlling interruption rate of otherwise blocking wait calls.
Re-phrased, let me know if you still think it's unclear!
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 106
> > <https://reviews.apache.org/r/33689/diff/3/?file=951770#file951770line106>
> >
> > Drop, there is already a binding like that above.
Done, moved that one down to keep all the UserTaskLauncher bindings in the same place.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 171
> > <https://reviews.apache.org/r/33689/diff/3/?file=951771#file951771line171>
> >
> > Just use "maxBatchSize - updates.size()" to avoid second guessing "-1" origin.
I like it, great suggestion! :)
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, line 84
> > <https://reviews.apache.org/r/33689/diff/3/?file=951776#file951776line84>
> >
> > Use 0L instead to avoid any delays in unit tests.
Switched to 1ms, 0ms will make it a busy loop, are you ok with that?
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, line 111
> > <https://reviews.apache.org/r/33689/diff/3/?file=951776#file951776line111>
> >
> > .once() is redundant
Removed.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, line 124
> > <https://reviews.apache.org/r/33689/diff/3/?file=951776#file951776line124>
> >
> > s/Object/Void
Done
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, line 141
> > <https://reviews.apache.org/r/33689/diff/3/?file=951776#file951776line141>
> >
> > Suggest using 5 seconds as it's more than enough time to run a single update.
Ok, 5 sounds fine as well.
> On May 7, 2015, 10:24 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, lines 99-104
> > <https://reviews.apache.org/r/33689/diff/3/?file=951770#file951770line99>
> >
> > Suggest wrapping these into a UserTaskLauncherSettings as a more scalable/lower overhead solution. See example here: https://github.com/apache/aurora/blob/ef0975655c04f0c2f3ecb6599d4e4beb9547f091/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java#L259
Done!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82916
-----------------------------------------------------------
On May 7, 2015, 12:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 12:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82916
-----------------------------------------------------------
Looks good overall. A few suggestions below.
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java
<https://reviews.apache.org/r/33689/#comment133768>
Delete TODO.
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133746>
@Positive
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133748>
Suggest rephrasing adding a notion that this is the argument controlling interruption rate of otherwise blocking wait calls.
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133749>
Suggest wrapping these into a UserTaskLauncherSettings as a more scalable/lower overhead solution. See example here: https://github.com/apache/aurora/blob/ef0975655c04f0c2f3ecb6599d4e4beb9547f091/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java#L259
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133762>
Drop, there is already a binding like that above.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133753>
Just use "maxBatchSize - updates.size()" to avoid second guessing "-1" origin.
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/33689/#comment133771>
Use 0L instead to avoid any delays in unit tests.
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/33689/#comment133773>
.once() is redundant
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/33689/#comment133778>
s/Object/Void
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/33689/#comment133775>
Suggest using 5 seconds as it's more than enough time to run a single update.
- Maxim Khutornenko
On May 7, 2015, 12:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 12:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 11, 2015, 10:02 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 83
> > <https://reviews.apache.org/r/33689/diff/4/?file=955534#file955534line83>
> >
> > I don't see any tests using it, drop the annotation?
I've gone back to not having the Settings class since we now only have 1 parameter.
> On May 11, 2015, 10:02 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 88
> > <https://reviews.apache.org/r/33689/diff/4/?file=955534#file955534line88>
> >
> > same here
Ditto here.
> On May 11, 2015, 10:02 p.m., Maxim Khutornenko wrote:
> > src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java, line 285
> > <https://reviews.apache.org/r/33689/diff/4/?file=955539#file955539line285>
> >
> > IAnswer<Void>
Done. Interestingly, it looks like I can leave out the <Void> here and just have Void on the override, but I'm not sure if you guys prefer one style over the other?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83293
-----------------------------------------------------------
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 11, 2015, 10:02 p.m., Maxim Khutornenko wrote:
> >
Thanks! Bill is taking a look as well, so I will wait for his feedback before updating.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83293
-----------------------------------------------------------
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83293
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134207>
I don't see any tests using it, drop the annotation?
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134208>
same here
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
<https://reviews.apache.org/r/33689/#comment134216>
IAnswer<Void>
- Maxim Khutornenko
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83268
-----------------------------------------------------------
Ship it!
Master (aabb692) 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 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 12, 2015, 12:18 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 172
> > <https://reviews.apache.org/r/33689/diff/4/?file=955534#file955534line172>
> >
> > The histogram would be interesting, but possibly overkill. We should be able to get good signal with stats we already have - number of status updates vs number of log writes
Good to know about the log write counter, I'll remove the TODO.
> On May 12, 2015, 12:18 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 156
> > <https://reviews.apache.org/r/33689/diff/4/?file=955534#file955534line156>
> >
> > This comment was slightly misleading, and probably belongs down near `continue;`, to communicate that the timeout was met and we are going to try again if the service is still running.
> >
> > However, i don't see why you can't get access to the thread. You could get it from `Thread.currentThread()` here, and use an `AtomicReference` to keep a ref.
Great suggestion, thanks Bill!
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83304
-----------------------------------------------------------
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83304
-----------------------------------------------------------
Ship it!
Overall LGTM. I'd like to see the move away from the polling loop if possible. See comment below.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134239>
This comment was slightly misleading, and probably belongs down near `continue;`, to communicate that the timeout was met and we are going to try again if the service is still running.
However, i don't see why you can't get access to the thread. You could get it from `Thread.currentThread()` here, and use an `AtomicReference` to keep a ref.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134246>
The histogram would be interesting, but possibly overkill. We should be able to get good signal with stats we already have - number of status updates vs number of log writes
- Bill Farner
On May 11, 2015, 6:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 11, 2015, 6:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83514
-----------------------------------------------------------
Ship it!
Master (c1b0dce) 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, 10:56 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 10:56 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83689
-----------------------------------------------------------
Ship it!
Master (2dc1d59) 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 13, 2015, 11:32 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 13, 2015, 11:32 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 8da488d024d2f595a3a31929a43d3e5dc38a2ee5
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 13, 2015, 11:32 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Rebased against master, resolved a minor merge conflict.
Used lambdas in the tests now that Java 8 is in use.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 8da488d024d2f595a3a31929a43d3e5dc38a2ee5
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 12, 2015, 10:56 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Restore the interrupt per bill's comment.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83507
-----------------------------------------------------------
Ship it!
Master (c1b0dce) 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, 10:08 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 10:08 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 12, 2015, 10:40 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 157
> > <https://reviews.apache.org/r/33689/diff/5/?file=956962#file956962line157>
> >
> > It's good form/practice to restore the interrupt flag:
> > `Thread.currentThread().interrupt();`.
Added.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83509
-----------------------------------------------------------
On May 12, 2015, 10:56 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 10:56 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review83509
-----------------------------------------------------------
Ship it!
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment134507>
It's good form/practice to restore the interrupt flag:
`Thread.currentThread().interrupt();`.
- Bill Farner
On May 12, 2015, 10:08 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 12, 2015, 10:08 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 12, 2015, 10:08 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Rely on interruption to unblock the status update consumer thread.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 11, 2015, 6:55 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Maxim's feedback.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82780
-----------------------------------------------------------
Ship it!
Master (fef7194) 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 7, 2015, 12:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 12:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 7, 2015, 12:27 a.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Fixed a style regression.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82774
-----------------------------------------------------------
Master (fef7194) is red with this patch.
./build-support/jenkins/build.sh
:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJava
Download http://maven.twttr.com/com/google/testing/test-libraries-for-java/1.1.1/test-libraries-for-java-1.1.1.pom
Download http://maven.twttr.com/com/google/testing/test-libraries-for-java/1.1.1/test-libraries-for-java-1.1.1.jar
Note: Writing file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh[ant:checkstyle] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java:261:32: 'userTaskLauncher' hides a field.
FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':checkstyleJmh'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/jmh.xml
* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.
BUILD FAILED
Total time: 3 mins 19.548 secs
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 6, 2015, 11:34 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 6, 2015, 11:34 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/
-----------------------------------------------------------
(Updated May 6, 2015, 11:34 p.m.)
Review request for Aurora, Maxim Khutornenko and Bill Farner.
Changes
-------
Updated per reviews:
* No longer using an executor for update processing.
* Parameters are now configurable via flags.
Bugs: AURORA-1228
https://issues.apache.org/jira/browse/AURORA-1228
Repository: aurora
Description
-------
Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
Diffs (updated)
-----
src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
Diff: https://reviews.apache.org/r/33689/diff/
Testing
-------
Ran the benchmark to confirm that this improves status update throughput substantially:
Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
Updated unit tests for the new invariants:
* TaskLaunchers are responsible for acknowledging updates.
* UserTaskLauncher processes updates asynchronously.
Thanks,
Ben Mahler
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Maxim Khutornenko <ma...@apache.org>.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 199-203
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line199>
> >
> > This isn't necessary. You can use `.execute(...)` instead and rely on a single threaded executor to enforce sequential item processing.
>
> Ben Mahler wrote:
> You mean the "same thread" executor, yes? Single threaded executor is still asynchronous IIUC.
>
> I was a bit hesitant to have the code assume a certain implementation of ExecutorService, seems brittle? If the implementation changes then the throughput can regress substantially.
No, I meant the async executor with a single thread in it. Actually, thinking more about perf here, I think this approach may result in subpar concurrency due to draining updates "too fast" and thus never reaching the max batch size.
What are your thoughts about dropping the thread-isolation requirement completely and implementing batch processing directly on a service thread? We would have to handle RuntimeExceptions explicitly to isolate single batch failures but that should still be easier to reason than "a single-threaded executor within a serial service thread loop".
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82433
-----------------------------------------------------------
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 199-203
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line199>
> >
> > This isn't necessary. You can use `.execute(...)` instead and rely on a single threaded executor to enforce sequential item processing.
You mean the "same thread" executor, yes? Single threaded executor is still asynchronous IIUC.
I was a bit hesitant to have the code assume a certain implementation of ExecutorService, seems brittle? If the implementation changes then the throughput can regress substantially.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82433
-----------------------------------------------------------
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 156
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line156>
> >
> > why "-1"?
Because there is already 1 item in the queue :)
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, lines 199-203
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line199>
> >
> > This isn't necessary. You can use `.execute(...)` instead and rely on a single threaded executor to enforce sequential item processing.
>
> Ben Mahler wrote:
> You mean the "same thread" executor, yes? Single threaded executor is still asynchronous IIUC.
>
> I was a bit hesitant to have the code assume a certain implementation of ExecutorService, seems brittle? If the implementation changes then the throughput can regress substantially.
>
> Maxim Khutornenko wrote:
> No, I meant the async executor with a single thread in it. Actually, thinking more about perf here, I think this approach may result in subpar concurrency due to draining updates "too fast" and thus never reaching the max batch size.
>
> What are your thoughts about dropping the thread-isolation requirement completely and implementing batch processing directly on a service thread? We would have to handle RuntimeExceptions explicitly to isolate single batch failures but that should still be easier to reason than "a single-threaded executor within a serial service thread loop".
Maxim and I chatted offline, for posterity: the purpose of batching is to accumulate work while processing a batch, which no longer occurs once we don't wait like this, so the throughput tanks as expected unless one uses the same thread executor. We'll drop the executor complexity altogether and just catch exceptions here.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 93
> > <https://reviews.apache.org/r/33689/diff/1/?file=947577#file947577line93>
> >
> > Register with lifecycle instead, e.g.:
> > ```
> > SchedulerServicesModule.addSchedulerActiveServiceBinding(binder()).to(UserTaskLauncher.class);
> > ```
Done.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 82
> > <https://reviews.apache.org/r/33689/diff/1/?file=947577#file947577line82>
> >
> > Remove the TODO.
This has gone away now that we no longer use an executor.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 83
> > <https://reviews.apache.org/r/33689/diff/1/?file=947577#file947577line83>
> >
> > Suggest wrapping into a PrivateModule to avoid binding annotations. The `provideTaskLaunchers` approach will go away once we deprecate/refactor TaskLauncher concept.
No longer needed now that we avoid the executor entirely.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java, line 84
> > <https://reviews.apache.org/r/33689/diff/1/?file=947577#file947577line84>
> >
> > This should be `AsyncUtil.loggingExecutor(1,1...)` to make sure there is always at most one thread regardless of the queue size. You can also expose the `BlockingQueue` size stat easily this way.
No longer needed now that we avoid the executor entirely.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 70
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line70>
> >
> > Expose as Arg<Interger> via SchedulerModule.
Done.
> On May 4, 2015, 9:56 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 141
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line141>
> >
> > This should be Arg<Amount<Long, Time>> exposed via SchedulerModule.
Done.
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82433
-----------------------------------------------------------
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82433
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133164>
Remove the TODO.
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133168>
Suggest wrapping into a PrivateModule to avoid binding annotations. The `provideTaskLaunchers` approach will go away once we deprecate/refactor TaskLauncher concept.
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133183>
This should be `AsyncUtil.loggingExecutor(1,1...)` to make sure there is always at most one thread regardless of the queue size. You can also expose the `BlockingQueue` size stat easily this way.
src/main/java/org/apache/aurora/scheduler/SchedulerModule.java
<https://reviews.apache.org/r/33689/#comment133171>
Register with lifecycle instead, e.g.:
```
SchedulerServicesModule.addSchedulerActiveServiceBinding(binder()).to(UserTaskLauncher.class);
```
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133167>
Expose as Arg<Interger> via SchedulerModule.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133176>
This should be Arg<Amount<Long, Time>> exposed via SchedulerModule.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133177>
why "-1"?
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133186>
This isn't necessary. You can use `.execute(...)` instead and rely on a single threaded executor to enforce sequential item processing.
- Maxim Khutornenko
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Ben Mahler <be...@gmail.com>.
> On May 3, 2015, 2:01 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 123
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line123>
> >
> > The `synchronized` seems unnecessary. The queue is already thread-safe.
Sure, I'll remove it since willUse doesn't have it. The final is also not needed anymore, so I'll remove it.
> On May 3, 2015, 2:01 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 70
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line70>
> >
> > Any reason why you have chosen this particular value?
Are you asking why a maximum, or why this particular maximum? The idea behind the maximum is to bound the amount of work that we do in the critical section (since it's a shared resource, we're not the only ones using storage), but behind this particular value, no reason. I've made it configurable via a flag.
> On May 3, 2015, 2:01 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 136
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line136>
> >
> > Could be a loop-invariant ArrayList which is cleared at the beginning of each interation.
> >
> > The tiny performance improvement is probably not measurable at all. However, using a LinkedList here seems rather uncommon.
Hm.. ArrayList does not implement Queue, do you mean ArrayDeque? As you said, it's likely not important, but what is important here is that we make it clear that updates is FIFO (this is why Queue is used).
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82343
-----------------------------------------------------------
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Stephan Erb <st...@dev.static-void.de>.
> On May 3, 2015, 4:01 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 70
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line70>
> >
> > Any reason why you have chosen this particular value?
>
> Ben Mahler wrote:
> Are you asking why a maximum, or why this particular maximum? The idea behind the maximum is to bound the amount of work that we do in the critical section (since it's a shared resource, we're not the only ones using storage), but behind this particular value, no reason. I've made it configurable via a flag.
Yeah, I somehow wondered why you chose 1000. But thinking about it, it sounds like a very sane default.
> On May 3, 2015, 4:01 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java, line 136
> > <https://reviews.apache.org/r/33689/diff/1/?file=947578#file947578line136>
> >
> > Could be a loop-invariant ArrayList which is cleared at the beginning of each interation.
> >
> > The tiny performance improvement is probably not measurable at all. However, using a LinkedList here seems rather uncommon.
>
> Ben Mahler wrote:
> Hm.. ArrayList does not implement Queue, do you mean ArrayDeque? As you said, it's likely not important, but what is important here is that we make it clear that updates is FIFO (this is why Queue is used).
Ok, sounds reasonable.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82343
-----------------------------------------------------------
On May 7, 2015, 2:27 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 7, 2015, 2:27 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Stephan Erb <st...@dev.static-void.de>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82343
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133058>
Any reason why you have chosen this particular value?
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133059>
The `synchronized` seems unnecessary. The queue is already thread-safe.
src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java
<https://reviews.apache.org/r/33689/#comment133060>
Could be a loop-invariant ArrayList which is cleared at the beginning of each interation.
The tiny performance improvement is probably not measurable at all. However, using a LinkedList here seems rather uncommon.
- Stephan Erb
On May 2, 2015, 12:55 a.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 2, 2015, 12:55 a.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>
Re: Review Request 33689: Updated scheduler to process status updates
asynchronously in batches.
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33689/#review82293
-----------------------------------------------------------
This patch does not apply cleanly on master (5fc7baf), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On May 1, 2015, 10:55 p.m., Ben Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33689/
> -----------------------------------------------------------
>
> (Updated May 1, 2015, 10:55 p.m.)
>
>
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
>
>
> Bugs: AURORA-1228
> https://issues.apache.org/jira/browse/AURORA-1228
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually.
>
>
> Diffs
> -----
>
> src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION
> src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35
> src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2
> src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095
> src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce
> src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e
> src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4
> src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8
> src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff
> src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419
> src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4
>
> Diff: https://reviews.apache.org/r/33689/diff/
>
>
> Testing
> -------
>
> Ran the benchmark to confirm that this improves status update throughput substantially:
>
> Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies.
> After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency.
>
> Updated unit tests for the new invariants:
>
> * TaskLaunchers are responsible for acknowledging updates.
> * UserTaskLauncher processes updates asynchronously.
>
>
> Thanks,
>
> Ben Mahler
>
>