-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29915/
-----------------------------------------------------------

Review request for Aurora.


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

Reply via email to