aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Added an optional JobKey set filter into the TaskQuery.
Date Fri, 07 Mar 2014 18:22:34 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master e63ea1aa9 -> 2a335d188


Added an optional JobKey set filter into the TaskQuery.

Bugs closed: AURORA-235

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


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

Branch: refs/heads/master
Commit: 2a335d1882aabf35f01503503ce0aeccf5430d70
Parents: e63ea1a
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Fri Mar 7 10:22:08 2014 -0800
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Fri Mar 7 10:22:08 2014 -0800

----------------------------------------------------------------------
 .../apache/aurora/scheduler/base/JobKeys.java   | 36 +++++++++++---
 .../org/apache/aurora/scheduler/base/Query.java | 50 ++++++++++++++------
 .../scheduler/state/SchedulerCoreImpl.java      |  5 +-
 .../scheduler/storage/mem/MemTaskStore.java     |  8 +++-
 .../thrift/org/apache/aurora/gen/api.thrift     |  1 +
 .../state/BaseSchedulerCoreImplTest.java        |  4 +-
 .../scheduler/storage/mem/MemTaskStoreTest.java | 36 ++++++++++++++
 .../org/apache/aurora/gen/api.thrift.md5        |  2 +-
 8 files changed, 113 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java b/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
index 5f684bc..db1bec4 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/JobKeys.java
@@ -15,12 +15,14 @@
  */
 package org.apache.aurora.scheduler.base;
 
+import java.util.Set;
 import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
 import com.google.common.base.Functions;
 import com.google.common.base.Optional;
 import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableSet;
 
 import org.apache.aurora.gen.JobKey;
 import org.apache.aurora.gen.TaskQuery;
@@ -152,19 +154,39 @@ public final class JobKeys {
   }
 
   /**
-   * Attempt to extract a job key from the given query if it is scoped to a single job.
+   * Attempt to extract job keys from the given query if it is job scoped.
    *
-   * @param query Query to extract the key from.
-   * @return A present if one can be extracted, absent otherwise.
+   * @param query Query to extract the keys from.
+   * @return A present if keys can be extracted, absent otherwise.
    */
-  public static Optional<IJobKey> from(Query.Builder query) {
+  public static Optional<Set<IJobKey>> from(Query.Builder query) {
     if (Query.isJobScoped(query)) {
       TaskQuery taskQuery = query.get();
-      return Optional.of(
-          from(taskQuery.getOwner().getRole(), taskQuery.getEnvironment(), taskQuery.getJobName()));
-
+      ImmutableSet.Builder<IJobKey> builder = ImmutableSet.builder();
+
+      if (taskQuery.isSetJobName()) {
+        builder.add(from(
+            taskQuery.getOwner().getRole(),
+            taskQuery.getEnvironment(),
+            taskQuery.getJobName()));
+      }
+
+      if (taskQuery.isSetJobKeys()) {
+        builder.addAll(IJobKey.setFromBuilders(taskQuery.getJobKeys()));
+      }
+      return Optional.of(assertValid(builder.build()));
     } else {
       return Optional.absent();
     }
   }
+
+  private static Set<IJobKey> assertValid(Set<IJobKey> jobKeys)
+      throws IllegalArgumentException {
+
+    for (IJobKey jobKey : jobKeys) {
+      checkArgument(isValid(jobKey), "Invalid job key format:" + jobKey);
+    }
+
+    return jobKeys;
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/main/java/org/apache/aurora/scheduler/base/Query.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/base/Query.java b/src/main/java/org/apache/aurora/scheduler/base/Query.java
index d6f27fd..1e586c5 100644
--- a/src/main/java/org/apache/aurora/scheduler/base/Query.java
+++ b/src/main/java/org/apache/aurora/scheduler/base/Query.java
@@ -16,11 +16,13 @@
 package org.apache.aurora.scheduler.base;
 
 import java.util.EnumSet;
+import java.util.Set;
 
 import com.google.common.base.Objects;
 import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.primitives.Ints;
 
 import org.apache.aurora.gen.Identity;
@@ -31,8 +33,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobKey;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import static org.apache.commons.lang.StringUtils.isEmpty;
-
 /**
  * A utility class to construct storage queries.
  * TODO(Sathya): Add some basic unit tests for isJobScoped and isOnlyJobScoped.
@@ -45,30 +45,33 @@ public final class Query {
 
   /**
    * Checks whether a query is scoped to a specific job.
-   * A query scoped to a job specifies a role and job name.
+   * A query scoped to a job specifies either:
+   * <ol>
+   *   <li>a role, environment and job name, or</li>
+   *   <li>a set of JobKey instances</li>
+   * </ol>
    *
    * @param taskQuery Query to test.
-   * @return {@code true} if the query specifies at least a role and job name,
-   *         otherwise {@code false}.
+   * @return {@code true} if the query specifies at least one job key, otherwise {@code false}.
    */
   public static boolean isJobScoped(Builder taskQuery) {
-    TaskQuery query = taskQuery.get();
-    return (query.getOwner() != null)
-        && !isEmpty(query.getOwner().getRole())
-        && !isEmpty(query.getEnvironment())
-        && !isEmpty(query.getJobName());
+    TaskQuery q = taskQuery.get();
+    return (q.isSetOwner() && q.getOwner().isSetRole() && q.isSetEnvironment()
&& q.isSetJobName())
+        || q.isSetJobKeys();
   }
 
   /**
    * Checks whether a query is strictly scoped to a specific job. A query is strictly job
scoped,
-   * iff it has the role, environment and jobName set.
+   * iff the only fields that are set in the query are: role, environment and job name.
    *
    * @param query Query to test.
-   * @return {@code true} if the query is strictly job scoped, otherwise {@code false}.
+   * @return {@code true} if the query is strictly single job scoped, otherwise {@code false}.
    */
-  public static boolean isOnlyJobScoped(Builder query) {
-    Optional<IJobKey> jobKey = JobKeys.from(query);
-    return jobKey.isPresent() && Query.jobScoped(jobKey.get()).equals(query);
+  public static boolean isSingleJobScoped(Builder query) {
+    Optional<Set<IJobKey>> jobKey = JobKeys.from(query);
+    return jobKey.isPresent()
+        && jobKey.get().size() == 1
+        && Query.jobScoped(Iterables.getOnlyElement(jobKey.get())).equals(query);
   }
 
   public static Builder arbitrary(TaskQuery query) {
@@ -91,6 +94,10 @@ public final class Query {
     return unscoped().byJob(jobKey);
   }
 
+  public static Builder jobScoped(Iterable<IJobKey> jobKeys) {
+    return unscoped().byJobKeys(jobKeys);
+  }
+
   public static Builder instanceScoped(InstanceKey instanceKey) {
     return instanceScoped(IJobKey.build(instanceKey.getJobKey()), instanceKey.getInstanceId());
   }
@@ -363,6 +370,19 @@ public final class Query {
     }
 
     /**
+     * Returns a new builder scoped to the jobs uniquely identified by the given jobKeys.
A
+     * builder can only be scoped to jobs once.
+     *
+     * @param jobKeys The job keys to scope this builder to.
+     * @return A new Builder scoped to the job keys.
+     */
+    public Builder byJobKeys(Iterable<IJobKey> jobKeys) {
+      checkNotNull(jobKeys);
+
+      return new Builder(query.deepCopy().setJobKeys(IJobKey.toBuildersSet(jobKeys)));
+    }
+
+    /**
      * A convenience method to scope this builder to {@link Tasks#ACTIVE_STATES}.
      *
      * @return A new Builder scoped to Tasks#ACTIVE_STATES.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
index 336e91b..3ca4ee5 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java
@@ -267,10 +267,11 @@ class SchedulerCoreImpl implements SchedulerCore {
 
     boolean jobDeleted = false;
 
-    if (Query.isOnlyJobScoped(query)) {
+    if (Query.isSingleJobScoped(query)) {
       // If this looks like a query for all tasks in a job, instruct the scheduler modules
to
       // delete the job.
-      IJobKey jobKey = JobKeys.from(query).get();
+      // TODO(maxim): Should be trivial to support killing multiple jobs instead.
+      IJobKey jobKey = Iterables.getOnlyElement(JobKeys.from(query).get());
       for (JobManager manager : jobManagers) {
         if (manager.deleteJob(jobKey)) {
           jobDeleted = true;

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
index 421b330..8589010 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
@@ -76,8 +76,7 @@ class MemTaskStore implements TaskStore.Mutable {
       new Function<Query.Builder, Optional<Set<IJobKey>>>() {
         @Override
         public Optional<Set<IJobKey>> apply(Query.Builder query) {
-          Optional<IJobKey> jobkey = JobKeys.from(query);
-          return jobkey.isPresent() ? Optional.of(jobkey.asSet()) : Optional.<Set<IJobKey>>absent();
+            return JobKeys.from(query);
         }
       };
   private static final Function<Query.Builder, Optional<Set<String>>> QUERY_TO_SLAVE_HOST
=
@@ -255,6 +254,11 @@ class MemTaskStore implements TaskStore.Mutable {
           }
         }
 
+        if (query.getJobKeysSize() > 0) {
+          if (!query.getJobKeys().contains(JobKeys.from(config).newBuilder())) {
+            return false;
+          }
+        }
         if (query.getTaskIds() != null) {
           if (!query.getTaskIds().contains(Tasks.id(task))) {
             return false;

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/main/thrift/org/apache/aurora/gen/api.thrift
----------------------------------------------------------------------
diff --git a/src/main/thrift/org/apache/aurora/gen/api.thrift b/src/main/thrift/org/apache/aurora/gen/api.thrift
index 159bdaa..d72b28c 100644
--- a/src/main/thrift/org/apache/aurora/gen/api.thrift
+++ b/src/main/thrift/org/apache/aurora/gen/api.thrift
@@ -364,6 +364,7 @@ struct TaskQuery {
   5: set<ScheduleStatus> statuses
   7: set<i32> instanceIds
   10: set<string> slaveHosts
+  11: set<JobKey> jobKeys
 }
 
 struct HostStatus {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
b/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
index 095cc4a..033ebd2 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
@@ -925,8 +925,8 @@ public abstract class BaseSchedulerCoreImplTest extends EasyMockTest {
   public void testIsStrictlyJobScoped() throws Exception {
     // TODO(Sathya): Remove this after adding a unit test for Query utility class.
     control.replay();
-    assertTrue(Query.isOnlyJobScoped(Query.jobScoped(KEY_A)));
-    assertFalse(Query.isOnlyJobScoped(Query.jobScoped(KEY_A).byId("xyz")));
+    assertTrue(Query.isSingleJobScoped(Query.jobScoped(KEY_A)));
+    assertFalse(Query.isSingleJobScoped(Query.jobScoped(KEY_A).byId("xyz")));
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java
index 0c1b144..380d2d9 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java
@@ -119,6 +119,42 @@ public class MemTaskStoreTest {
   }
 
   @Test
+  public void testQueryByJobKeys() {
+    assertStoreContents();
+    store.saveTasks(ImmutableSet.of(TASK_A, TASK_B, TASK_C, TASK_D));
+
+    assertQueryResults(
+        Query.jobScoped(ImmutableSet.of(
+            JobKeys.from("role-a", "env-a", "job-a"),
+            JobKeys.from("role-b", "env-b", "job-b"),
+            JobKeys.from("role-c", "env-c", "job-c"))),
+        TASK_A, TASK_B, TASK_C);
+
+    // Conflicting jobs will produce empty result as TaskQuery fields are ANDed.
+    assertEquals(
+        0,
+        store.fetchTasks(
+            Query.jobScoped(JobKeys.from("role-a", "env-a", "job-a"))
+                .byJobKeys(ImmutableSet.of(JobKeys.from("role-b", "env-b", "job-b")))).size());
+
+    // Matching jobs would return successfully.
+    assertQueryResults(
+        Query.jobScoped(JobKeys.from("role-a", "env-a", "job-a"))
+            .byJobKeys(ImmutableSet.of(
+                JobKeys.from("role-b", "env-b", "job-b"),
+                JobKeys.from("role-a", "env-a", "job-a"))),
+        TASK_A);
+
+    // Combination of individual field and jobKeys is allowed.
+    assertQueryResults(
+        Query.roleScoped("role-b")
+            .byJobKeys(ImmutableSet.of(
+                JobKeys.from("role-b", "env-b", "job-b"),
+                JobKeys.from("role-a", "env-a", "job-a"))),
+        TASK_B);
+  }
+
+  @Test
   public void testMutate() {
     store.saveTasks(ImmutableSet.of(TASK_A, TASK_B, TASK_C, TASK_D));
     assertQueryResults(Query.statusScoped(RUNNING));

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/2a335d18/src/test/resources/org/apache/aurora/gen/api.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/api.thrift.md5 b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
index 593d154..2308ba8 100644
--- a/src/test/resources/org/apache/aurora/gen/api.thrift.md5
+++ b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
@@ -1 +1 @@
-5132f760c254dd05a9e7bdd57a5684cc
+0025936691573a8638d5b1a0323c3924


Mime
View raw message