> On Feb. 28, 2017, 1:44 a.m., David McLaughlin wrote:
> > Overall approach LGTM, although I agree with Stephan that I'd like to see a 
> > bit more separation between the drivers. 
> > 
> > A more thorough review to follow, but are you able to run this in 
> > production and give some feedback on performance? It would be good to know 
> > if we can get rid of the old native driver or we're going to have to carry 
> > the different implementations around for a while. We could also try and run 
> > this patch in our scale test cluster.
> 
> Zameer Manji wrote:
>     I know that `V0Mesos` performs the same as the scheduler driver since it 
> is a very thin API wrapper around the scheduler driver.
>     
>     I have no concrete data on the HTTP API just yet. I suspect it is much 
> slower per some tickets in the MESOS queue.
>     
>     I will think about seperation between the scheduler callbacks. I might 
> extract all the logic into a seperate class and we can have very thin 
> callback implementations.
> 
> David McLaughlin wrote:
>     +1 to having the different Scheduler interfaces just take in a 
> SchedulerCallbacksImpl.
> 
> Zameer Manji wrote:
>     Done.

I noticed you still had a single driver that implements both interfaces. Does 
SchedulerCallbacksImpl now make it easy to split those out a la Stephan's 
suggestion?


- David


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


On March 1, 2017, 6:46 a.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57061/
> -----------------------------------------------------------
> 
> (Updated March 1, 2017, 6:46 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Mehrdad Nurolahzade, and Stephan 
> Erb.
> 
> 
> Bugs: AURORA-1887 and AURORA-1888
>     https://issues.apache.org/jira/browse/AURORA-1887
>     https://issues.apache.org/jira/browse/AURORA-1888
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This patch completes the design doc[1] and enables operators to choose between
> two V1 Mesos API implementations. The first is `V0Mesos` which offers the V1 
> API
> backed by the scheduler driver and the second is `V1Mesos` which offers the V1
> API backed by a new HTTP API implementation.
> 
> There are three sets of changes in this patch.
> 
> First, the V1 Mesos code requires a Scheduler callback with a different API. 
> To
> maximize code reuse, event handling logic was extracted into a 
> `MesosCallbackHandler`
> class. `MesosSchedulerImpl` was extended to implement the new
> callback as well as the old callback. This callback now just uses the handler
> class.
> 
> Second, a new driver implementation using the new API was created. All of the
> logic for the new driver is encapsulated in the
> `VersionedSchedulerDriverService` class.
> 
> Third, some wiring changes were done to allow for Guice to do it's work and
> allow for operators to select between the different driver implementations.
> 
> [1] 
> https://docs.google.com/document/d/1bWK8ldaQSsRXvdKwTh8tyR_0qMxAlnMW70eOKoU3myo
> 
> 
> Diffs
> -----
> 
>   examples/vagrant/upstart/aurora-scheduler.conf 
> 49fdcbd8b7406a59ae7882473b9eddbfce3ece7c 
>   src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 
> 6c2bf46c0d55ac6a85fed7244cba24d74e1b34aa 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> e2ef9c30720698263106f22e3e24db5d0468b155 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 805e9de9bc45396cb8fc6e33ddb3d7428312c608 
>   src/main/java/org/apache/aurora/scheduler/mesos/LibMesosLoadingModule.java 
> e1a23590c795a489e753b77b0835d30d3be174b5 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> eb210962c54cd0d33e3760b32f5b0ca1a7079204 
>   src/main/java/org/apache/aurora/scheduler/mesos/ProtosConversion.java 
> bc9e23b7410c00b7d5ffa4f23db93a51e9d0f405 
>   src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 
> 5519323079b2c957a23e093dcc77929148b4a59a 
>   src/main/java/org/apache/aurora/scheduler/mesos/VersionedDriverFactory.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/mesos/VersionedSchedulerDriverService.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> 05518048ca5518a007281269aa402a7d0710eb62 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/MesosCallbackHandlerTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> c599fe30bc903b3a3fb178df70a46d2421b6c45e 
>   
> src/test/java/org/apache/aurora/scheduler/mesos/VersionedSchedulerDriverServiceTest.java
>  PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> f2275c757ebfa52179e31b95bf0c02b6753fb7e3 
> 
> 
> Diff: https://reviews.apache.org/r/57061/diff/10/
> 
> 
> Testing
> -------
> 
> The e2e test has been run three times, each time with a different driver 
> option.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Reply via email to