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



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.

- David McLaughlin


On Feb. 28, 2017, 12:43 a.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57061/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2017, 12:43 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, `MesosSchedulerImpl` was extended to implement the new
> callback as well as the old callback. The code and tests were reshufled to
> maxmize logic reuse.
> 
> 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/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/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/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/
> 
> 
> Testing
> -------
> 
> The e2e test has been run three times, each time with a different driver 
> option.
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Reply via email to