----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61918/#review184140 -----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java Lines 356-358 (original), 366-371 (patched) <https://reviews.apache.org/r/61918/#comment260200> I think we can avoid extra copies by replacing this with ``` return FluentIterable.from(offers).filter( e -> !globallyBannedOffers.contains(e.getOffer().getId()) ).toSet(); ``` `toSet()` will only copy `offers` that pass the filter, and `globallyBannedOffers` will be not modified, as it will be called in synchronized context. - Dmitry Zhuk On Aug. 30, 2017, 2:06 a.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61918/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2017, 2:06 a.m.) > > > Review request for Aurora, David McLaughlin, Dmitry Zhuk, Santhosh Kumar > Shanmugham, and Stephan Erb. > > > Repository: aurora > > > Description > ------- > > Currently, `getOffers` and `getWeaklyConsistentOffers` provide iterables that > use lambdas (in filter) not backed by concurrent data structures > (`globallyBannedOffers` and `staticallyBannedOffers`). This can lead to a > race condition where we have unknown behavior if reading and modification > happens at the same time. > > To fix this, in `getOffers` we will revert to the previous behavior (before > patch https://reviews.apache.org/r/61804/) where we create a copy of the > current offers as well as a copy of the banned offers for consistency. > > For `getWeaklyConsistentOffers`, we will use concurrent data structures for > `globallyBannedOffers` and `staticallyBannedOffers` so we don't have a race > that may produce unknown side effects. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > 5697d2e2eb001042511924442ff2dbe358451642 > > > Diff: https://reviews.apache.org/r/61918/diff/3/ > > > Testing > ------- > > Unit tests pass. > > Currently running end to end test and testing on a live cluster. > > > Thanks, > > Jordan Ly > >
