----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28193/#review62575 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/ResourceSlot.java <https://reviews.apache.org/r/28193/#comment104640> tabbing is off src/main/java/org/apache/aurora/scheduler/configuration/Resources.java <https://reviews.apache.org/r/28193/#comment104644> This is only used in tests outside of this class. Consider reverting to private. src/main/java/org/apache/aurora/scheduler/configuration/Resources.java <https://reviews.apache.org/r/28193/#comment104647> +1 on moving it closer to its only consumer. That's a general guideline we follow everywhere. src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104649> Not related to your change but consider renaming it to something different (e.g. ExecutorSettings) to avoid naming collision with the thrift object. src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104651> "-100MB" looks like negative resource and is confusing. src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104660> Why not MIN_EXECUTOR_RESOURCES? We normally abstract out from the process framework concept in the scheduler code. src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104654> Any justification for the min resources chosen similar to the above? src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104655> Convert to // for inline comments. src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java <https://reviews.apache.org/r/28193/#comment104665> Unless I am missing something, all you need to do here is to make sure neither containerResources nor executorResources is less than min. Can you do something like: finalTaskResources = Resources.maxElements(containerResources, MIN_TASK_RESOURCES); and replace ".addAllResources(MIN_THERMOS_RESOURCES.toResourceList())" with .addAllResources(Resources.maxElements(executorOverhead, MIN_THERMOS_RESOURCES))? - Maxim Khutornenko On Nov. 21, 2014, 5:01 a.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28193/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2014, 5:01 a.m.) > > > Review request for Aurora, Maxim Khutornenko and Bill Farner. > > > Bugs: AURORA-928 > https://issues.apache.org/jira/browse/AURORA-928 > > > Repository: aurora > > > Description > ------- > > Mesos rejects tasks and executors that are zero sized. This patch > reconfigures Aurora to ensure no zero sized tasks and executors are created. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/ResourceSlot.java > ed60447c798a97daceda4a3bba6ee9bcdcaedd0f > src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java > 40b652c679d8e340f585e28cbed066335d9d760d > src/main/java/org/apache/aurora/scheduler/configuration/Resources.java > 65c4b526c89a4d5607af4424ebe49bb48e296ae9 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > bb227fd86f7c4c692f6ae2aef1c15a94913354b7 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 899416fceae498353880012b8a93491cff461064 > src/test/java/org/apache/aurora/scheduler/configuration/ResourcesTest.java > d6febb8998e05257cabe8d193cefa0b6c79f197e > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > 953c1edb6802d8983ab324aa56361e5c8fbe2e68 > > Diff: https://reviews.apache.org/r/28193/diff/ > > > Testing > ------- > > ./gradlew clean build -Pq > > > Thanks, > > Zameer Manji > >
