You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ke...@apache.org on 2013/12/05 20:58:14 UTC

[2/9] git commit: Remove the deprecated instance ID field.

Remove the deprecated instance ID field.


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

Branch: refs/heads/master
Commit: 87afc2c5e9c79225fd524287b2a1466ec0c27231
Parents: 13ddf83
Author: Bill Farner <bi...@twitter.com>
Authored: Wed Dec 4 10:16:42 2013 -0800
Committer: Bill Farner <bi...@twitter.com>
Committed: Wed Dec 4 10:16:42 2013 -0800

----------------------------------------------------------------------
 .../aurora/scheduler/async/TaskGroups.java      | 11 +++---
 .../twitter/aurora/scheduler/base/Tasks.java    |  7 ----
 .../configuration/ConfigurationManager.java     | 16 --------
 .../scheduler/state/StateManagerImpl.java       |  2 +-
 .../scheduler/storage/StorageBackfill.java      | 28 --------------
 .../python/twitter/aurora/client/api/updater.py | 15 --------
 .../thrift/com/twitter/aurora/gen/api.thrift    |  6 ---
 .../state/BaseSchedulerCoreImplTest.java        |  8 +---
 .../scheduler/state/StateManagerImplTest.java   |  5 +--
 .../scheduler/storage/StorageBackfillTest.java  | 39 ++------------------
 .../twitter/aurora/client/api/test_updater.py   | 13 -------
 11 files changed, 11 insertions(+), 139 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java
----------------------------------------------------------------------
diff --git a/src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java b/src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java
index ff51e93..9ea0229 100644
--- a/src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java
+++ b/src/main/java/com/twitter/aurora/scheduler/async/TaskGroups.java
@@ -45,7 +45,6 @@ import com.twitter.aurora.gen.ScheduleStatus;
 import com.twitter.aurora.scheduler.base.JobKeys;
 import com.twitter.aurora.scheduler.base.Query;
 import com.twitter.aurora.scheduler.base.Tasks;
-import com.twitter.aurora.scheduler.configuration.ConfigurationManager;
 import com.twitter.aurora.scheduler.events.PubsubEvent.EventSubscriber;
 import com.twitter.aurora.scheduler.events.PubsubEvent.StorageStarted;
 import com.twitter.aurora.scheduler.events.PubsubEvent.TaskStateChange;
@@ -340,15 +339,15 @@ public class TaskGroups implements EventSubscriber {
   }
 
   static class GroupKey {
-    private final ITaskConfig scrubbedCanonicalTask;
+    private final ITaskConfig canonicalTask;
 
     GroupKey(ITaskConfig task) {
-      this.scrubbedCanonicalTask = ConfigurationManager.scrubNonUniqueTaskFields(task);
+      this.canonicalTask = task;
     }
 
     @Override
     public int hashCode() {
-      return Objects.hashCode(scrubbedCanonicalTask);
+      return Objects.hashCode(canonicalTask);
     }
 
     @Override
@@ -357,12 +356,12 @@ public class TaskGroups implements EventSubscriber {
         return false;
       }
       GroupKey other = (GroupKey) o;
-      return Objects.equal(scrubbedCanonicalTask, other.scrubbedCanonicalTask);
+      return Objects.equal(canonicalTask, other.canonicalTask);
     }
 
     @Override
     public String toString() {
-      return JobKeys.toPath(Tasks.INFO_TO_JOB_KEY.apply(scrubbedCanonicalTask));
+      return JobKeys.toPath(Tasks.INFO_TO_JOB_KEY.apply(canonicalTask));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/java/com/twitter/aurora/scheduler/base/Tasks.java
----------------------------------------------------------------------
diff --git a/src/main/java/com/twitter/aurora/scheduler/base/Tasks.java b/src/main/java/com/twitter/aurora/scheduler/base/Tasks.java
index f560fc8..557ac3a 100644
--- a/src/main/java/com/twitter/aurora/scheduler/base/Tasks.java
+++ b/src/main/java/com/twitter/aurora/scheduler/base/Tasks.java
@@ -70,13 +70,6 @@ public final class Tasks {
   public static final Function<IScheduledTask, String> SCHEDULED_TO_ID =
       Functions.compose(ASSIGNED_TO_ID, SCHEDULED_TO_ASSIGNED);
 
-  public static final Function<ITaskConfig, Integer> INFO_TO_INSTANCE_ID_DEPRECATED =
-      new Function<ITaskConfig, Integer>() {
-        @Override public Integer apply(ITaskConfig task) {
-          return task.getInstanceIdDEPRECATED();
-        }
-      };
-
   public static final Function<IAssignedTask, Integer> ASSIGNED_TO_INSTANCE_ID =
       new Function<IAssignedTask, Integer>() {
         @Override public Integer apply(IAssignedTask task) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/java/com/twitter/aurora/scheduler/configuration/ConfigurationManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/com/twitter/aurora/scheduler/configuration/ConfigurationManager.java b/src/main/java/com/twitter/aurora/scheduler/configuration/ConfigurationManager.java
index 8f0e055..6d40d11 100644
--- a/src/main/java/com/twitter/aurora/scheduler/configuration/ConfigurationManager.java
+++ b/src/main/java/com/twitter/aurora/scheduler/configuration/ConfigurationManager.java
@@ -207,22 +207,6 @@ public final class ConfigurationManager {
   }
 
   /**
-   * Resets fields within a task configuration that would make it unique from originally-equal
-   * configurations.
-   *
-   * @param task Task to scrub.
-   * @return Scrubbed task.
-   */
-  public static ITaskConfig scrubNonUniqueTaskFields(ITaskConfig task) {
-    TaskConfig copy = task.newBuilder();
-    // Unsetting only changes the isset bit vector.  For equals() comparison, the value must also be
-    // canonical.
-    copy.setInstanceIdDEPRECATED(0);
-    copy.unsetInstanceIdDEPRECATED();
-    return ITaskConfig.build(copy);
-  }
-
-  /**
    * Check validity of and populates defaults in a job configuration.  This will return a deep copy
    * of the provided job configuration with default configuration values applied, and configuration
    * map values sanitized and applied to their respective struct fields.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/java/com/twitter/aurora/scheduler/state/StateManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/com/twitter/aurora/scheduler/state/StateManagerImpl.java b/src/main/java/com/twitter/aurora/scheduler/state/StateManagerImpl.java
index f2b8c88..37d13f4 100644
--- a/src/main/java/com/twitter/aurora/scheduler/state/StateManagerImpl.java
+++ b/src/main/java/com/twitter/aurora/scheduler/state/StateManagerImpl.java
@@ -122,7 +122,7 @@ public class StateManagerImpl implements StateManager {
           AssignedTask assigned = new AssignedTask()
               .setTaskId(taskIdGenerator.generate(task, entry.getKey()))
               .setInstanceId(entry.getKey())
-              .setTask(task.newBuilder().setInstanceIdDEPRECATED(entry.getKey()));
+              .setTask(task.newBuilder());
           return IScheduledTask.build(new ScheduledTask()
               .setStatus(INIT)
               .setAssignedTask(assigned));

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/java/com/twitter/aurora/scheduler/storage/StorageBackfill.java
----------------------------------------------------------------------
diff --git a/src/main/java/com/twitter/aurora/scheduler/storage/StorageBackfill.java b/src/main/java/com/twitter/aurora/scheduler/storage/StorageBackfill.java
index 343cc7e..df5b603 100644
--- a/src/main/java/com/twitter/aurora/scheduler/storage/StorageBackfill.java
+++ b/src/main/java/com/twitter/aurora/scheduler/storage/StorageBackfill.java
@@ -23,7 +23,6 @@ import com.google.common.collect.FluentIterable;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 
-import com.twitter.aurora.gen.AssignedTask;
 import com.twitter.aurora.gen.JobConfiguration;
 import com.twitter.aurora.gen.ScheduleStatus;
 import com.twitter.aurora.gen.ScheduledTask;
@@ -109,32 +108,6 @@ public final class StorageBackfill {
       Stats.exportLong("instance_ids_inconsistent");
 
   /**
-   * Ensures backwards-compatible data is present for both the new and deprecated instance ID
-   * fields.
-   *
-   * @param task Task to possibly modify when ensuring backwards compatibility.
-   */
-  private static void dualWriteInstanceId(AssignedTask task) {
-    boolean oldFieldSet = task.getTask().isSetInstanceIdDEPRECATED();
-    boolean newFieldSet = task.isSetInstanceId();
-    if (oldFieldSet && newFieldSet) {
-      BOTH_FIELDS_SET.incrementAndGet();
-      if (task.getInstanceId() != task.getTask().getInstanceIdDEPRECATED()) {
-        FIELDS_INCONSISTENT.incrementAndGet();
-      }
-    } else if (oldFieldSet) {
-      OLD_FIELD_SET.incrementAndGet();
-      task.setInstanceId(task.getTask().getInstanceIdDEPRECATED());
-    } else if (newFieldSet) {
-      NEW_FIELD_SET.incrementAndGet();
-      task.getTask().setInstanceIdDEPRECATED(task.getInstanceId());
-    } else {
-      throw new IllegalStateException(
-          "Task " + task.getTaskId() + " does not have an instance id.");
-    }
-  }
-
-  /**
    * Ensures backwards-compatibility of the throttled state, which exists in this version but is
    * not handled.
    *
@@ -164,7 +137,6 @@ public final class StorageBackfill {
         // TODO(ksweeney): Guarantee tasks pass current validation code here and quarantine if they
         // don't.
         guaranteeShardUniqueness(builder, storeProvider.getUnsafeTaskStore(), clock);
-        dualWriteInstanceId(builder.getAssignedTask());
         rewriteThrottledState(builder);
         return IScheduledTask.build(builder);
       }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/python/twitter/aurora/client/api/updater.py
----------------------------------------------------------------------
diff --git a/src/main/python/twitter/aurora/client/api/updater.py b/src/main/python/twitter/aurora/client/api/updater.py
index 9d930be..3e6e642 100644
--- a/src/main/python/twitter/aurora/client/api/updater.py
+++ b/src/main/python/twitter/aurora/client/api/updater.py
@@ -193,7 +193,6 @@ invoking cancel_update.
     for instance_id in instance_ids:
       from_config = operation_configs.from_config.get(instance_id)
       to_config = operation_configs.to_config.get(instance_id)
-      self._unset_deprecated_fields(from_config, to_config)
 
       if from_config and to_config:
         # Sort internal dicts before comparing to rule out differences due to hashing.
@@ -213,20 +212,6 @@ invoking cancel_update.
 
     return to_kill, to_add
 
-  def _unset_deprecated_fields(self, from_config, to_config):
-    """Unsets deprecated fields in task configs to provide common base for diffing.
-
-       Arguments:
-       from_config - task config to update from.
-       to_config - task config to update to.
-    """
-    if from_config:
-      from_config.instanceIdDEPRECATED = None
-
-    if to_config:
-      to_config.instanceIdDEPRECATED = None
-
-
   def _update_instances(self, instance_ids, operation_configs):
     """Applies kill/add actions for the specified batch instances.
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/main/thrift/com/twitter/aurora/gen/api.thrift
----------------------------------------------------------------------
diff --git a/src/main/thrift/com/twitter/aurora/gen/api.thrift b/src/main/thrift/com/twitter/aurora/gen/api.thrift
index 3f8d61d..1ce4041 100644
--- a/src/main/thrift/com/twitter/aurora/gen/api.thrift
+++ b/src/main/thrift/com/twitter/aurora/gen/api.thrift
@@ -156,12 +156,6 @@ struct TaskConfig {
  10: i64 diskMb
  11: i32 priority
  13: i32 maxTaskFailures
- 14: i32 instanceIdDEPRECATED                // TODO(William Farner): Deprecated. Use
-                                             // AssignedTask.instanceId instead.
-                                             // The instance ID for this task.
-                                             // Instance IDs must be unique and contiguous within a
-                                             // job, and will be in the range [0, N-1] (inclusive)
-                                             // for a job that has N instances.
  18: optional bool production                // Whether this is a production task, which can preempt
                                              // non-production tasks.
  20: set<Constraint> constraints

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/test/java/com/twitter/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/com/twitter/aurora/scheduler/state/BaseSchedulerCoreImplTest.java b/src/test/java/com/twitter/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
index 0bfa3ab..3e4a502 100644
--- a/src/test/java/com/twitter/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
+++ b/src/test/java/com/twitter/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
@@ -224,19 +224,13 @@ public abstract class BaseSchedulerCoreImplTest extends EasyMockTest {
       assertFalse(state.getAssignedTask().isSetSlaveId());
       assertEquals(
           validateAndPopulate(job.getJobConfig()).getTaskConfig(),
-          ConfigurationManager.scrubNonUniqueTaskFields(state.getAssignedTask().getTask()));
+          state.getAssignedTask().getTask());
     }
     Set<Integer> expectedInstanceIds =
         ContiguousSet.create(Range.closedOpen(0, numTasks), DiscreteDomain.integers());
     assertEquals(
         expectedInstanceIds,
         FluentIterable.from(tasks).transform(Tasks.SCHEDULED_TO_INSTANCE_ID).toSet());
-    assertEquals(
-        expectedInstanceIds,
-        FluentIterable.from(tasks)
-            .transform(Tasks.SCHEDULED_TO_INFO)
-            .transform(Tasks.INFO_TO_INSTANCE_ID_DEPRECATED)
-            .toSet());
   }
 
   private static Constraint dedicatedConstraint(Set<String> values) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/test/java/com/twitter/aurora/scheduler/state/StateManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/com/twitter/aurora/scheduler/state/StateManagerImplTest.java b/src/test/java/com/twitter/aurora/scheduler/state/StateManagerImplTest.java
index 2e45de8..664ec64 100644
--- a/src/test/java/com/twitter/aurora/scheduler/state/StateManagerImplTest.java
+++ b/src/test/java/com/twitter/aurora/scheduler/state/StateManagerImplTest.java
@@ -42,14 +42,12 @@ import com.twitter.aurora.gen.TaskConfig;
 import com.twitter.aurora.gen.TaskEvent;
 import com.twitter.aurora.scheduler.Driver;
 import com.twitter.aurora.scheduler.TaskIdGenerator;
-import com.twitter.aurora.scheduler.base.JobKeys;
 import com.twitter.aurora.scheduler.base.Query;
 import com.twitter.aurora.scheduler.base.Tasks;
 import com.twitter.aurora.scheduler.events.PubsubEvent;
 import com.twitter.aurora.scheduler.events.PubsubEvent.TaskStateChange;
 import com.twitter.aurora.scheduler.events.PubsubEvent.TasksDeleted;
 import com.twitter.aurora.scheduler.storage.Storage;
-import com.twitter.aurora.scheduler.storage.entities.IJobKey;
 import com.twitter.aurora.scheduler.storage.entities.IScheduledTask;
 import com.twitter.aurora.scheduler.storage.entities.ITaskConfig;
 import com.twitter.aurora.scheduler.storage.mem.MemStorage;
@@ -75,7 +73,6 @@ public class StateManagerImplTest extends EasyMockTest {
   private static final String HOST_A = "host_a";
   private static final Identity JIM = new Identity("jim", "jim-user");
   private static final String MY_JOB = "myJob";
-  private static final IJobKey JOB_KEY = JobKeys.from(JIM.getRole(), DEFAULT_ENVIRONMENT, MY_JOB);
 
   private Driver driver;
   private TaskIdGenerator taskIdGenerator;
@@ -180,7 +177,7 @@ public class StateManagerImplTest extends EasyMockTest {
         .setAssignedTask(new AssignedTask()
             .setInstanceId(3)
             .setTaskId(taskId)
-            .setTask(task.newBuilder().setInstanceIdDEPRECATED(3)));
+            .setTask(task.newBuilder()));
     assertEquals(ImmutableSet.of(IScheduledTask.build(expected)),
         Storage.Util.consistentFetchTasks(storage, Query.taskScoped(taskId)));
   }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/test/java/com/twitter/aurora/scheduler/storage/StorageBackfillTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/com/twitter/aurora/scheduler/storage/StorageBackfillTest.java b/src/test/java/com/twitter/aurora/scheduler/storage/StorageBackfillTest.java
index 647f89e..56f96b5 100644
--- a/src/test/java/com/twitter/aurora/scheduler/storage/StorageBackfillTest.java
+++ b/src/test/java/com/twitter/aurora/scheduler/storage/StorageBackfillTest.java
@@ -1,6 +1,5 @@
 package com.twitter.aurora.scheduler.storage;
 
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -36,10 +35,7 @@ public class StorageBackfillTest {
     clock = new FakeClock();
   }
 
-  private static IScheduledTask makeTask(
-      String id,
-      Optional<Integer> oldInstanceId,
-      Optional<Integer> newInstanceId) {
+  private static IScheduledTask makeTask(String id, int instanceId) {
 
     TaskConfig config = new TaskConfig()
         .setOwner(new Identity("user", "role"))
@@ -54,43 +50,14 @@ public class StorageBackfillTest {
     ScheduledTask task = new ScheduledTask().setAssignedTask(
         new AssignedTask().setTask(config));
     task.getAssignedTask().setTaskId(id);
-
-    if (oldInstanceId.isPresent()) {
-      task.getAssignedTask().getTask().setInstanceIdDEPRECATED(oldInstanceId.get());
-    }
-    if (newInstanceId.isPresent()) {
-      task.getAssignedTask().setInstanceId(newInstanceId.get());
-    }
+    task.getAssignedTask().setInstanceId(instanceId);
     return IScheduledTask.build(task);
   }
 
   @Test
-  public void testInstanceIdBackfill() {
-    final String bothId = "both";
-    final String oldId = "old";
-    final String newId = "new";
-    final IScheduledTask bothFields = makeTask(bothId, Optional.of(5), Optional.of(5));
-    final IScheduledTask oldField = makeTask(oldId, Optional.of(6), Optional.<Integer>absent());
-    final IScheduledTask newField = makeTask(newId, Optional.<Integer>absent(), Optional.of(7));
-
-    storage.write(new MutateWork.NoResult.Quiet() {
-      @Override protected void execute(MutableStoreProvider storeProvider) {
-        storeProvider.getUnsafeTaskStore().saveTasks(
-            ImmutableSet.of(bothFields, oldField, newField));
-        StorageBackfill.backfill(storeProvider, clock);
-      }
-    });
-
-    assertEquals(makeTask(bothId, Optional.of(5), Optional.of(5)), getTask(bothId));
-    assertEquals(makeTask(oldId, Optional.of(6), Optional.of(6)), getTask(oldId));
-    assertEquals(makeTask(newId, Optional.of(7), Optional.of(7)), getTask(newId));
-  }
-
-  @Test
   public void testRewriteThrottledState() {
     final IScheduledTask savedTask =
-        IScheduledTask.build(makeTask("id", Optional.<Integer>absent(), Optional.of(0)).newBuilder()
-            .setStatus(ScheduleStatus.THROTTLED));
+        IScheduledTask.build(makeTask("id", 0).newBuilder().setStatus(ScheduleStatus.THROTTLED));
 
     storage.write(new MutateWork.NoResult.Quiet() {
       @Override protected void execute(MutableStoreProvider storeProvider) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/87afc2c5/src/test/python/twitter/aurora/client/api/test_updater.py
----------------------------------------------------------------------
diff --git a/src/test/python/twitter/aurora/client/api/test_updater.py b/src/test/python/twitter/aurora/client/api/test_updater.py
index f98dc0c..3c148d3 100644
--- a/src/test/python/twitter/aurora/client/api/test_updater.py
+++ b/src/test/python/twitter/aurora/client/api/test_updater.py
@@ -403,19 +403,6 @@ class UpdaterTest(TestCase):
     self.expect_finish()
     self.replay_mocks()
 
-  def test_noop_update_with_deprecated_fields(self):
-    """Deprecated fields in task config do not affect diffing."""
-    old_configs = self.make_task_configs(5)
-    new_config = old_configs[0]
-    old_configs[1].instanceIdDEPRECATED = 7
-    job_config = self.make_job_config(new_config, 5)
-    self._config.job_config = job_config
-    self.expect_start()
-    self.expect_get_tasks(old_configs)
-    self.expect_populate(job_config)
-    self.expect_finish()
-    self.replay_mocks()
-
   def test_update_rollback(self):
     """Update process failures exceed total allowable count and update is rolled back."""
     update_config = self.UPDATE_CONFIG.copy()