Repository: aurora Updated Branches: refs/heads/master aa2db8e43 -> bcb477429
Modify ClusterStateImpl to be thread safe. ClusterStateImpl exposes a synchronized multimap which does not have a thread-safe implementation of `keySet`. This patch revises the `ClusterStateImpl` to return an immutable copy. Bugs closed: AURORA-1510 Reviewed at https://reviews.apache.org/r/39670/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/bcb47742 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/bcb47742 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/bcb47742 Branch: refs/heads/master Commit: bcb477429d000177983f7b40db99ec03a2670623 Parents: aa2db8e Author: Zameer Manji <[email protected]> Authored: Thu Oct 29 14:04:23 2015 -0700 Committer: Zameer Manji <[email protected]> Committed: Thu Oct 29 14:04:32 2015 -0700 ---------------------------------------------------------------------- .../apache/aurora/scheduler/preemptor/ClusterStateImpl.java | 9 +++++---- .../aurora/scheduler/preemptor/ClusterStateImplTest.java | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/bcb47742/src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java b/src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java index 42e2ca4..5574e9b 100644 --- a/src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/preemptor/ClusterStateImpl.java @@ -14,8 +14,8 @@ package org.apache.aurora.scheduler.preemptor; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.Multimap; -import com.google.common.collect.Multimaps; import com.google.common.eventbus.Subscribe; import org.apache.aurora.scheduler.base.Tasks; @@ -27,12 +27,13 @@ import org.apache.aurora.scheduler.events.PubsubEvent.TaskStateChange; */ public class ClusterStateImpl implements ClusterState, PubsubEvent.EventSubscriber { - private final Multimap<String, PreemptionVictim> victims = - Multimaps.synchronizedMultimap(HashMultimap.create()); + private final Multimap<String, PreemptionVictim> victims = HashMultimap.create(); @Override public Multimap<String, PreemptionVictim> getSlavesToActiveTasks() { - return Multimaps.unmodifiableMultimap(victims); + synchronized (victims) { + return ImmutableSetMultimap.copyOf(victims); + } } @Subscribe http://git-wip-us.apache.org/repos/asf/aurora/blob/bcb47742/src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java b/src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java index a1ac922..881bb20 100644 --- a/src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java +++ b/src/test/java/org/apache/aurora/scheduler/preemptor/ClusterStateImplTest.java @@ -13,8 +13,8 @@ */ package org.apache.aurora.scheduler.preemptor; -import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableMultimap; +import com.google.common.collect.ImmutableSetMultimap; import org.apache.aurora.gen.AssignedTask; import org.apache.aurora.gen.JobKey; @@ -109,11 +109,11 @@ public class ClusterStateImplTest { } private void assertVictims(IAssignedTask... tasks) { - ImmutableMultimap.Builder<String, PreemptionVictim> victims = ImmutableMultimap.builder(); + ImmutableMultimap.Builder<String, PreemptionVictim> victims = ImmutableSetMultimap.builder(); for (IAssignedTask task : tasks) { victims.put(task.getSlaveId(), PreemptionVictim.fromTask(task)); } - assertEquals(HashMultimap.create(victims.build()), state.getSlavesToActiveTasks()); + assertEquals(victims.build(), state.getSlavesToActiveTasks()); } private IAssignedTask makeTask(String taskId, String slaveId) {
