aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: Prevent job updates from allowing unbounded instance events
Date Wed, 15 Jul 2015 22:02:39 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 545e8396d -> 4b117395a


Prevent job updates from allowing unbounded instance events

Bugs closed: AURORA-1096

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


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

Branch: refs/heads/master
Commit: 4b117395ab4b04f507f10b7a6ad4e6be0387a023
Parents: 545e839
Author: Joe Smith <yasumoto7@gmail.com>
Authored: Wed Jul 15 15:02:24 2015 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Wed Jul 15 15:02:24 2015 -0700

----------------------------------------------------------------------
 .../thrift/SchedulerThriftInterface.java        | 26 ++++++++++++++--
 .../thrift/SchedulerThriftInterfaceTest.java    | 31 +++++++++++++++++++-
 2 files changed, 53 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/4b117395/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
index dc0cd2d..22786de 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -161,9 +161,20 @@ import static org.apache.aurora.scheduler.thrift.Responses.ok;
  */
 @DecoratedThrift
 class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
+  private static final int DEFAULT_MAX_TASKS_PER_JOB = 4000;
+  private static final int DEFAULT_MAX_UPDATE_INSTANCE_FAILURES =
+      DEFAULT_MAX_TASKS_PER_JOB * 5;
+
   @Positive
   @CmdLine(name = "max_tasks_per_job", help = "Maximum number of allowed tasks in a single
job.")
-  public static final Arg<Integer> MAX_TASKS_PER_JOB = Arg.create(4000);
+  public static final Arg<Integer> MAX_TASKS_PER_JOB = Arg.create(DEFAULT_MAX_TASKS_PER_JOB);
+
+  @Positive
+  @CmdLine(name = "max_update_instance_failures", help = "Upper limit on the number of "
+      + "failures allowed during a job update. This helps cap potentially unbounded entries
into "
+      + "storage.")
+  public static final Arg<Integer> MAX_UPDATE_INSTANCE_FAILURES = Arg.create(
+      DEFAULT_MAX_UPDATE_INSTANCE_FAILURES);
 
   // This number is derived from the maximum file name length limit on most UNIX systems,
less
   // the number of characters we've observed being added by mesos for the executor ID, prefix,
and
@@ -1113,6 +1124,11 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
       return invalidRequest(INVALID_MAX_FAILED_INSTANCES);
     }
 
+    if (settings.getMaxPerInstanceFailures() * mutableRequest.getInstanceCount()
+            > MAX_UPDATE_INSTANCE_FAILURES.get()) {
+      return invalidRequest(TOO_MANY_POTENTIAL_FAILED_INSTANCES);
+    }
+
     if (settings.getMinWaitInInstanceRunningMs() < 0) {
       return invalidRequest(INVALID_MIN_WAIT_TO_RUNNING);
     }
@@ -1376,8 +1392,12 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
   static final String INVALID_MAX_FAILED_INSTANCES = "maxFailedInstances must be non-negative.";
 
   @VisibleForTesting
-  static final String INVALID_MAX_INSTANCE_FAILURES =
-      "maxPerInstanceFailures must be non-negative.";
+  static final String TOO_MANY_POTENTIAL_FAILED_INSTANCES = "Your update allows too many
failures "
+      + "to occur, consider decreasing the per-instance failures or maxFailedInstances.";
+
+  @VisibleForTesting
+  static final String INVALID_MAX_INSTANCE_FAILURES
+      = "maxPerInstanceFailures must be non-negative.";
 
   @VisibleForTesting
   static final String INVALID_MIN_WAIT_TO_RUNNING =

http://git-wip-us.apache.org/repos/asf/aurora/blob/4b117395/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
index d28baba..94ea0c8 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -53,6 +53,7 @@ import org.apache.aurora.gen.JobKey;
 import org.apache.aurora.gen.JobUpdate;
 import org.apache.aurora.gen.JobUpdateInstructions;
 import org.apache.aurora.gen.JobUpdatePulseStatus;
+import org.apache.aurora.gen.JobUpdateQuery;
 import org.apache.aurora.gen.JobUpdateRequest;
 import org.apache.aurora.gen.JobUpdateSettings;
 import org.apache.aurora.gen.JobUpdateSummary;
@@ -1932,6 +1933,20 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
   }
 
   @Test
+  public void testStartUpdateFailsTooManyPerInstanceFailures() throws Exception {
+    control.replay();
+
+    JobUpdateRequest updateRequest = buildServiceJobUpdateRequest();
+    updateRequest.getSettings()
+        .setMaxPerInstanceFailures(SchedulerThriftInterface.MAX_UPDATE_INSTANCE_FAILURES.get()
+            + 10);
+
+    assertEquals(
+        invalidResponse(SchedulerThriftInterface.TOO_MANY_POTENTIAL_FAILED_INSTANCES),
+        thrift.startJobUpdate(updateRequest, AUDIT_MESSAGE, SESSION));
+  }
+
+  @Test
   public void testStartUpdateFailsInvalidMaxFailedInstances() throws Exception {
     control.replay();
 
@@ -2329,7 +2344,8 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     control.replay();
 
     assertEquals(
-        okResponse(Result.pulseJobUpdateResult(new PulseJobUpdateResult(JobUpdatePulseStatus.OK))),
+        okResponse(Result.pulseJobUpdateResult(
+            new PulseJobUpdateResult(JobUpdatePulseStatus.OK))),
         thrift.pulseJobUpdate(UPDATE_KEY.newBuilder(), SESSION));
   }
 
@@ -2366,6 +2382,19 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
     assertResponse(AUTH_FAILED, thrift.pulseJobUpdate(UPDATE_KEY.newBuilder(), SESSION));
   }
 
+  @Test
+  public void testGetJobUpdateSummaries() throws Exception {
+    Response updateSummary = Responses.empty()
+        .setResponseCode(OK)
+        .setDetails(ImmutableList.of(new ResponseDetail("summary")));
+
+    expect(readOnlyScheduler.getJobUpdateSummaries(
+        anyObject(JobUpdateQuery.class))).andReturn(updateSummary);
+    control.replay();
+
+    assertEquals(updateSummary, thrift.getJobUpdateSummaries(new JobUpdateQuery()));
+  }
+
   private static final String AUTH_DENIED_MESSAGE = "Denied!";
 
   private IExpectationSetters<?> expectAuth(Set<String> roles, boolean allowed)


Mime
View raw message