> On Aug. 28, 2017, 11: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?
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.
```
- Jordan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61918/#review184005
-----------------------------------------------------------
On Aug. 25, 2017, 8: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, 8: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
>
>
