> On Sept. 4, 2014, 10:17 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java, > > line 121 > > <https://reviews.apache.org/r/25356/diff/1/?file=679020#file679020line121> > > > > Does it have to be synchronized? Could not find the reason based on its > > call chain.
This was reflex, using common locking for exposed methods. This is an immutable map, though, so i'll remove it here. > On Sept. 4, 2014, 10:17 p.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java, > > lines 94-96 > > <https://reviews.apache.org/r/25356/diff/1/?file=679021#file679021line94> > > > > If I am reading it right the update will still proceed in case > > updateOnlyTheseInstances specifies an unrecognized range (i.e. not > > intersecting with the full working set). The InstanceUpdater appears to > > handle the optional desiredState as no-op. This is the departure from the > > current implementation where a disjoint --shards value fails the update. I think it's best for that to happen higher in the stack. The thrift interface is probably the best place for this, to validate a JobUpdateConfiguration when it's received. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25356/#review52363 ----------------------------------------------------------- On Sept. 4, 2014, 9:19 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25356/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2014, 9:19 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Zameer Manji. > > > Bugs: AURORA-613 > https://issues.apache.org/jira/browse/AURORA-613 > > > Repository: aurora > > > Description > ------- > > This is the bridge between the top-level updater logic and the > OneWayJobUpdater - taking a user request and the direction we've decided to > move the job (forward or back), produce the appropriate OneWayJobUpdater. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java > 85196af38b6615e5aac7de30a4a601350e935c72 > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java > aa8b5f2ee81d42db003f27e682f79e94b6625d87 > > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactory.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java > dca55ef5612105c9dc8fd57e789fdf1df28ba447 > src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java > d7baaa679289998a9ea80c0934990a49e5c173d7 > > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterFactoryImplTest.java > PRE-CREATION > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java > e3e50d7d6526e8796dc5cd82b459190b09ceb72f > > Diff: https://reviews.apache.org/r/25356/diff/ > > > Testing > ------- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > >