> On Aug. 28, 2017, 4:08 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java > > Line 361 (original), 367 (patched) > > <https://reviews.apache.org/r/61918/diff/1/?file=1804129#file1804129line367> > > > > How about this method? > > Jordan Ly wrote: > I only copy on the getOffers call for performance. I believe that the > callers of getOffers do not expect the iterator to change. > > For `getWeaklyConsistentOffers`, we expect the caller to know that offers > may be removed while iterating -- see the comment in launchTask: > > ``` > // Guard against an offer being removed after we grabbed it from > the iterator. > // If that happens, the offer will not exist in hostOffers, and we > can immediately > // send it back to LOST for quick reschedule. > // Removing while iterating counts on the use of a > weakly-consistent iterator being used, > // which is a feature of ConcurrentSkipListSet. > ```
Can you add comments here to explain the goal and usecase for the 2 different implementations? (`getWeaklyConsistentOffers` and `getOffers`) - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61918/#review184005 ----------------------------------------------------------- On Aug. 25, 2017, 1:31 p.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61918/ > ----------------------------------------------------------- > > (Updated Aug. 25, 2017, 1:31 p.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/2/ > > > Testing > ------- > > Unit tests pass. > > Currently running end to end test and testing on a live cluster. > > > Thanks, > > Jordan Ly > >
