----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47050/#review132357 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java (line 75) <https://reviews.apache.org/r/47050/#comment196531> Building on my feedback on the review that introduced ResourceBag, do you think it'd make sense to have a helper method on that class like `streamResourceVectors()` to say on the redundant `getResourceVectors().entrySet().stream()` that are popping up all over the place now? Or maybe just `Set<ResourceType> ResourceBag#getTypes()` which returns `getResourceVectors().getEntrySet()`? src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java (lines 113 - 124) <https://reviews.apache.org/r/47050/#comment196539> Any reason we still create an anonymous `Function` impl, rather than just using a lambda here? src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java (lines 137 - 156) <https://reviews.apache.org/r/47050/#comment196544> This seems relatively inefficient. Worst case scenario we'd be iterating the length of the list 4 times, where, in theory, we'd only need to do it once? Roughly, something like this should work? Or am I missing something? Could probably be rewritten to use `Stream#reduce` if desired. boolean hasGreater = hasLessThan = false; for (entry : left.resourceTypes()) { int comparison = entry.getValue().compareTo(right.valueOf(entry.getKey())); if (comparison < 0) { hasLessThan = true; } else if (comparison > 0) { hasGreater = true; } } return !hasGreater && !hasLessThan ? 0 : hasGreater ? 1 : 0; - Joshua Cohen On May 6, 2016, 8:22 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47050/ > ----------------------------------------------------------- > > (Updated May 6, 2016, 8:22 p.m.) > > > Review request for Aurora, Joshua Cohen and Bill Farner. > > > Repository: aurora > > > Description > ------- > > Migrating some `ResourceSlot` customers to `ResourceBag`. The selection here > is somewhat arbitrary to minimize the diff size. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettings.java > 501e6431f21822d9816952377546586da02ce42a > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java > 1ee2cfa231d32d91b620dddd7aee6c1047424cb3 > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java > 6b5b12bce8052b93e921d060dd0e196b2554eefd > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > b325106c7f45b1ad1657221aaa39e3a428719ab0 > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictim.java > 98be997b0343de85d3c376a770d574437ede69a6 > > src/main/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter.java > 032ab2d4a4aa07ac78aba36149801cd545bd6f5b > src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java > 99f034f58d2858863568f85177fc42753bcd6f17 > src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java > 6d0d120e563b403f6fbe171a1ca23a84ff242e40 > src/main/java/org/apache/aurora/scheduler/resources/ResourceBag.java > 7916ec024b92b1900a50f004aefdca9ae9229031 > src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java > 69087e6ff140ee84e02731840be1b3dce2d9bb7e > src/main/java/org/apache/aurora/scheduler/resources/ResourceSlot.java > 13922bc650636ccd28cf60bcd893bce78e751306 > src/main/java/org/apache/aurora/scheduler/sla/SlaGroup.java > 21121bcf5f9f42ff60f86a1861aca2333bc9e99e > src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java > b6e43d798fd7d4de9e9a67a05f54c58b8663c95a > src/main/java/org/apache/aurora/scheduler/stats/AsyncStatsModule.java > 1d1415ac9ccddc607384fac59fa523b701346f3d > > src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java > 1474fa90eab3d018a499e8aefc7f9266e892f72a > > src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java > 94a885fae8179f0f52bff6985fd9052b857f6b6f > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > ad397c6924b025f1eefb2bb02a6dc1e1f10ca078 > src/test/java/org/apache/aurora/scheduler/mesos/Offers.java > b266554b19a571f794edcc1aefa8571f1c406891 > > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java > 4efa696b6ee196837f775349a9b2b8f655fa4575 > src/test/java/org/apache/aurora/scheduler/resources/ResourceBagTest.java > 48724d5d75cc78555e98f29b2ed8ace9901ccdb7 > > src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java > 333db30d7b0b229b8e66a91a0786a5c22268c299 > src/test/java/org/apache/aurora/scheduler/resources/ResourceSlotTest.java > 842572c0b42de1c287f3de22bc8ae9f2fc940fc5 > src/test/java/org/apache/aurora/scheduler/resources/ResourceTestUtil.java > e0cca4b89f2cf9ac32d88f4544ff85c68c62b585 > src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java > 81baa7826896f4179be47c53336e4235b35b97b6 > > Diff: https://reviews.apache.org/r/47050/diff/ > > > Testing > ------- > > ./gradlew -Pq build > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > > Thanks, > > Maxim Khutornenko > >
