> On Oct. 19, 2017, 3:25 p.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Line 329 (original), 319-324 (patched) > > <https://reviews.apache.org/r/63157/diff/2/?file=1864450#file1864450line330> > > > > Feels like a lot of stuff going on here. > > > > I think it might be cleaner if the caller is forced to call `remove` on > > the offer from the same agent. Additionally, maybe name this > > `addIfNotPresent` and a small comment stating that this method will return > > a `HostOffer` if we already have one from the particular agent.
> Feels like a lot of stuff going on here. For a function called `add()`, i agree. A rename and comment sounds warranted. > I think it might be cleaner if the caller is forced to call remove on the > offer from the same agent I'm not a fan of this. It's much easier to reason about consistency during concurrent access if this is all performed while holding a lock. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63157/#review188776 ----------------------------------------------------------- On Oct. 19, 2017, 1:23 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63157/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2017, 1:23 p.m.) > > > Review request for Aurora and Jordan Ly. > > > Repository: aurora > > > Description > ------- > > Increasing the offer hold time to effectively disable offer declines is a > trap, as the queue of asynchronous declines will grow without bound. This > introduces a command line argument to explicitly disable declining. > > > Diffs > ----- > > RELEASE-NOTES.md f4cc4163972ce516fd07747d004e0b8bfe5b2bd7 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 7d3766829161457b1b3ba50bce128047d10b2c58 > src/main/java/org/apache/aurora/scheduler/offers/Deferment.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > e8334310a2a46a0ccb09ee6e4122c515892d3996 > src/main/java/org/apache/aurora/scheduler/offers/OfferSettings.java > 4c6fd546a450917b7542329b020f42ef6379f3b7 > src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java > ab98addf6b63c3e4b97374c3d5adcd79ceec78c0 > src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java > 8f4f63c73c2c2133c7beaf22e1abccfd966f542c > src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java > 815a7e8200ad7b25080556bddd54d407a74678cc > > > Diff: https://reviews.apache.org/r/63157/diff/2/ > > > Testing > ------- > > > Thanks, > > Bill Farner > >
