aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Fixing consumption double counting in quota checks.
Date Tue, 30 Sep 2014 17:27:31 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 25b7b3cc8 -> 0454e85bd


Fixing consumption double counting in quota checks.

Bugs closed: AURORA-768

Reviewed at https://reviews.apache.org/r/26160/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/0454e85b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/0454e85b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/0454e85b

Branch: refs/heads/master
Commit: 0454e85bdf46958ac10339c0392e4e47e9bb3457
Parents: 25b7b3c
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Tue Sep 30 10:27:06 2014 -0700
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Tue Sep 30 10:27:06 2014 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/quota/QuotaManager.java    | 24 +++++----
 .../aurora/scheduler/updater/JobDiff.java       |  6 +--
 .../scheduler/quota/QuotaManagerImplTest.java   | 54 ++++++++++++++++++++
 3 files changed, 72 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/0454e85b/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java b/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
index a0da417..2442b06 100644
--- a/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java
@@ -24,6 +24,7 @@ import com.google.common.base.Predicate;
 import com.google.common.base.Predicates;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableRangeSet;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Range;
 import com.google.common.collect.RangeSet;
 import com.google.common.collect.Sets;
@@ -42,6 +43,7 @@ import org.apache.aurora.scheduler.storage.Storage.Work;
 import org.apache.aurora.scheduler.storage.entities.IInstanceTaskConfig;
 import org.apache.aurora.scheduler.storage.entities.IJobKey;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdate;
+import org.apache.aurora.scheduler.storage.entities.IJobUpdateInstructions;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateQuery;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateSummary;
 import org.apache.aurora.scheduler.storage.entities.IRange;
@@ -200,13 +202,15 @@ public interface QuotaManager {
           Optional<IJobUpdate> update = Optional.fromNullable(
               roleJobUpdates.get(JobKeys.from(input.getAssignedTask().getTask())));
 
-          // TODO(maxim): Address AURORA-768 to avoid double counting.
-          if (update.isPresent() && update.get().getInstructions().isSetDesiredState())
{
-            IInstanceTaskConfig configs =
-                update.get().getInstructions().getDesiredState();
-            RangeSet<Integer> desiredInstances = rangesToRangeSet(configs.getInstances());
+          if (update.isPresent()) {
+            IJobUpdateInstructions instructions = update.get().getInstructions();
+            RangeSet<Integer> initialInstances = instanceRangeSet(instructions.getInitialState());
+            RangeSet<Integer> desiredInstances = instanceRangeSet(instructions.isSetDesiredState()
+                ? ImmutableSet.of(instructions.getDesiredState())
+                : ImmutableSet.<IInstanceTaskConfig>of());
 
-            return !desiredInstances.contains(input.getAssignedTask().getInstanceId());
+            int instanceId = input.getAssignedTask().getInstanceId();
+            return !initialInstances.contains(instanceId) && !desiredInstances.contains(instanceId);
           }
           return true;
         }
@@ -242,10 +246,12 @@ public interface QuotaManager {
           .setUpdateStatuses(JobUpdateController.ACTIVE_JOB_UPDATE_STATES));
     }
 
-    private static RangeSet<Integer> rangesToRangeSet(Set<IRange> ranges) {
+    private static RangeSet<Integer> instanceRangeSet(Set<IInstanceTaskConfig>
configs) {
       ImmutableRangeSet.Builder<Integer> builder = ImmutableRangeSet.builder();
-      for (IRange range : ranges) {
-        builder.add(Range.closed(range.getFirst(), range.getLast()));
+      for (IInstanceTaskConfig config : configs) {
+        for (IRange range : config.getInstances()) {
+          builder.add(Range.closed(range.getFirst(), range.getLast()));
+        }
       }
 
       return builder.build();

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/0454e85b/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java b/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
index 9d02474..5daefff 100644
--- a/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
+++ b/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java
@@ -216,9 +216,9 @@ public final class JobDiff {
     JobDiff diff = JobDiff.compute(
         taskStore,
         job,
-        ImmutableMap.copyOf(instructions.isSetDesiredState()
-            ? asMap(instructions.getDesiredState())
-            : ImmutableMap.<Integer, ITaskConfig>of()),
+        instructions.isSetDesiredState()
+            ? ImmutableMap.copyOf(asMap(instructions.getDesiredState()))
+            : ImmutableMap.<Integer, ITaskConfig>of(),
         instructions.getSettings().getUpdateOnlyTheseInstances());
     return diff.getReplacedInstances().isEmpty() && diff.getReplacementInstances().isEmpty();
   }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/0454e85b/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
index f961494..b58c8f3 100644
--- a/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java
@@ -374,6 +374,60 @@ public class QuotaManagerImplTest extends EasyMockTest {
   }
 
   @Test
+  public void testCheckQuotaUpdateInitialConfigsUsedForFiltering() {
+    expectQuota(IResourceAggregate.build(new ResourceAggregate(6, 6, 6))).times(2);
+    expectTasks(createProdTask("foo", 2, 2, 2), createProdTask(JOB_NAME, 2, 2, 2)).times(2);
+
+    String updateId = "u1";
+    ITaskConfig config = createTaskConfig(2, 2, 2, true);
+    List<IJobUpdateSummary> summaries = buildJobUpdateSummaries(updateId, JobKeys.from(config));
+    IJobUpdate update = buildJobUpdate(summaries.get(0), config, 1, config, 1);
+    JobUpdate builder = update.newBuilder();
+    builder.getInstructions().unsetDesiredState();
+
+    expect(jobUpdateStore.fetchJobUpdateSummaries(updateQuery(config.getOwner().getRole())))
+        .andReturn(summaries).times(2);
+
+    expect(jobUpdateStore.fetchJobUpdate(updateId))
+        .andReturn(Optional.of(IJobUpdate.build(builder))).times(2);
+
+    control.replay();
+
+    QuotaCheckResult checkQuota = quotaManager.checkQuota(ROLE, prodResource(1, 1, 1));
+    assertEquals(SUFFICIENT_QUOTA, checkQuota.getResult());
+    assertEquals(
+        IResourceAggregate.build(new ResourceAggregate(4, 4, 4)),
+        quotaManager.getQuotaInfo(ROLE).getProdConsumption());
+  }
+
+  @Test
+  public void testCheckQuotaUpdateDesiredConfigsUsedForFiltering() {
+    expectQuota(IResourceAggregate.build(new ResourceAggregate(6, 6, 6))).times(2);
+    expectTasks(createProdTask("foo", 2, 2, 2), createProdTask(JOB_NAME, 2, 2, 2)).times(2);
+
+    String updateId = "u1";
+    ITaskConfig config = createTaskConfig(2, 2, 2, true);
+    List<IJobUpdateSummary> summaries = buildJobUpdateSummaries(updateId, JobKeys.from(config));
+    IJobUpdate update = buildJobUpdate(summaries.get(0), config, 1, config, 1);
+    JobUpdate builder = update.newBuilder();
+    builder.getInstructions().setInitialState(ImmutableSet.<InstanceTaskConfig>of());
+
+    expect(jobUpdateStore.fetchJobUpdateSummaries(updateQuery(config.getOwner().getRole())))
+        .andReturn(summaries).times(2);
+
+    expect(jobUpdateStore.fetchJobUpdate(updateId))
+        .andReturn(Optional.of(IJobUpdate.build(builder))).times(2);
+
+    control.replay();
+
+    QuotaCheckResult checkQuota = quotaManager.checkQuota(ROLE, prodResource(1, 1, 1));
+    assertEquals(SUFFICIENT_QUOTA, checkQuota.getResult());
+    assertEquals(
+        IResourceAggregate.build(new ResourceAggregate(4, 4, 4)),
+        quotaManager.getQuotaInfo(ROLE).getProdConsumption());
+  }
+
+  @Test
   public void testCheckQuotaNoDesiredState() {
     expectQuota(IResourceAggregate.build(new ResourceAggregate(6, 6, 6))).times(2);
     expectTasks(createProdTask("foo", 2, 2, 2), createProdTask("bar", 2, 2, 2)).times(2);


Mime
View raw message