----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29915/#review68195 -----------------------------------------------------------
Ship it! Master (a350982) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing "@ReviewBot retry" - Aurora ReviewBot On Jan. 15, 2015, 2:27 a.m., Zameer Manji wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29915/ > ----------------------------------------------------------- > > (Updated Jan. 15, 2015, 2:27 a.m.) > > > Review request for Aurora and Bill Farner. > > > Bugs: AURORA-1016 > https://issues.apache.org/jira/browse/AURORA-1016 > > > Repository: aurora > > > Description > ------- > > It is possible for CachedClusterState to add a null SlaveID to the multimap. > This null key can propagate to the Preemptor and cause the following NPE: > ```` > W0114 20:57:59.565 THREAD149 > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl.schedule: > Task scheduling unexpectedly > failed, will be retried > java.lang.NullPointerException > at > com.google.common.base.Preconditions.checkNotNull(Preconditions.java:213) > at > com.google.common.collect.ImmutableCollection$ArrayBasedBuilder.add(ImmutableCollection.java:339) > at > com.google.common.collect.ImmutableSet$Builder.add(ImmutableSet.java:480) > at > com.google.common.collect.ImmutableSet$Builder.add(ImmutableSet.java:456) > at > com.google.common.collect.ImmutableCollection$Builder.addAll(ImmutableCollection.java:282) > at > com.google.common.collect.ImmutableCollection$ArrayBasedBuilder.addAll(ImmutableCollection.java:360) > at > com.google.common.collect.ImmutableSet$Builder.addAll(ImmutableSet.java:508) > at > org.apache.aurora.scheduler.async.preemptor.PreemptorImpl.findPreemptionSlotFor(PreemptorImpl.java:321) > at > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl.maybePreemptFor(TaskScheduler.java:249) > at > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl.scheduleTask(TaskScheduler.java:220) > at > com.twitter.common.inject.TimedInterceptor.invoke(TimedInterceptor.java:87) > at > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl$3.apply(TaskScheduler.java:192) > at > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl$3.apply(TaskScheduler.java:189) > at > org.apache.aurora.scheduler.storage.log.LogStorage$24.apply(LogStorage.java:608) > at > org.apache.aurora.scheduler.storage.log.LogStorage$24.apply(LogStorage.java:605) > at > org.apache.aurora.scheduler.storage.mem.MemStorage$3.apply(MemStorage.java:147) > at > org.apache.aurora.scheduler.storage.mem.MemStorage$3.apply(MemStorage.java:144) > at > org.apache.aurora.scheduler.storage.db.DbStorage.write(DbStorage.java:137) > at > org.mybatis.guice.transactional.TransactionalMethodInterceptor.invoke(TransactionalMethodInterceptor.java:101) > at > org.apache.aurora.scheduler.storage.mem.MemStorage.write(MemStorage.java:144) > at > com.twitter.common.inject.TimedInterceptor.invoke(TimedInterceptor.java:87) > at > org.apache.aurora.scheduler.storage.log.LogStorage.doInTransaction(LogStorage.java:605) > at > org.apache.aurora.scheduler.storage.log.LogStorage.write(LogStorage.java:638) > at > org.apache.aurora.scheduler.storage.CallOrderEnforcingStorage.write(CallOrderEnforcingStorage.java:122) > at > org.apache.aurora.scheduler.async.TaskScheduler$TaskSchedulerImpl.schedule(TaskScheduler.java:189) > at > com.twitter.common.inject.TimedInterceptor.invoke(TimedInterceptor.java:87) > at > org.apache.aurora.scheduler.async.TaskGroups$1.schedule(TaskGroups.java:136) > at > org.apache.aurora.scheduler.async.TaskGroups$2.run(TaskGroups.java:158) > at > java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471) > at java.util.concurrent.FutureTask.run(FutureTask.java:262) > at > java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:178) > at > java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:292) > at > java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145) > at > java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615) > at java.lang.Thread.run(Thread.java:745) > ```` > > This patch prevents the addition of the null key into the multimap and > instead throws an `IllegalStateException`. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterState.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterStateTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/29915/diff/ > > > Testing > ------- > > ./gradlew build -Pq > > > Thanks, > > Zameer Manji > >
