> On Feb. 24, 2017, 9:21 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/base/JobKeys.java, line 114 > > <https://reviews.apache.org/r/56691/diff/4/?file=1643847#file1643847line114> > > > > I don't think you need the `components.size() == 4` here.
Vanished. Done. > On Feb. 24, 2017, 9:21 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java, line > > 74 > > <https://reviews.apache.org/r/56691/diff/4/?file=1643850#file1643850line74> > > > > Shouldn't we query for tasks in the `ACTIVE_STATES` and not just > > `RUNNING` here? > > > > To me this seems like a task could be in `PENDING` waiting for it's > > reserved offer back and we unreserve it here. You only want to unreserve tasks that are already running. We do not want to unreserve offers if there is a small chance that the task is making it's way through the system. RUNNING is the only safe state. If task is in PENDING then we do not want to unreserve the offer. > On Feb. 24, 2017, 9:21 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java, line > > 86 > > <https://reviews.apache.org/r/56691/diff/4/?file=1643850#file1643850line86> > > > > We prefer `ImmutableList.of()` for lists of one item. Done. > On Feb. 24, 2017, 9:21 p.m., Zameer Manji wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java, line > > 73 > > <https://reviews.apache.org/r/56691/diff/4/?file=1643850#file1643850line73> > > > > Could the `offerAdded` event handler handle the exception instead of > > letting it go up to the `EventBus`? We could log an errror and increment an > > metric. Done. - Dmitriy ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56691/#review166759 ----------------------------------------------------------- On Feb. 24, 2017, 11:02 p.m., Dmitriy Shirchenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56691/ > ----------------------------------------------------------- > > (Updated Feb. 24, 2017, 11:02 p.m.) > > > Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji. > > > Bugs: AURORA-1816 > https://issues.apache.org/jira/browse/AURORA-1816 > > > Repository: aurora > > > Description > ------- > > Second patch that implements mechanism for unreserving previously reserved > resources for dynamic reservations proposal. > > > Diffs > ----- > > src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java > 6f2ca35c5d83dde29c24865b4826d4932e96da80 > src/main/java/org/apache/aurora/scheduler/HostOffer.java > 23f0600d64e1e15f4856f397e839e3d1c87f3b96 > src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java > b12ac83168401c15fb1d30179ea8e4816f09cd3d > src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java > 0637eb7f85125cf70b588d56fa7dc88130947837 > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 > src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java > 202cae96ffc5b49e638b973a273f7983137b5baf > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java > 30699596a1c95199df7504f62c5c18cab1be1c6c > src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java > 49d4e82cc03144b80292fe43066a6cc4d7aed88f > src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/56691/diff/ > > > Testing > ------- > > Ran locally on vagrant. > > > Thanks, > > Dmitriy Shirchenko > >
