aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject aurora git commit: Add deprecated field storage backfill
Date Thu, 04 Feb 2016 23:18:51 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 42bff1961 -> 505f31005


Add deprecated field storage backfill

Bugs closed: AURORA-1603

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


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

Branch: refs/heads/master
Commit: 505f3100550386a8f8fc21cd9889f137415c6c56
Parents: 42bff19
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Thu Feb 4 15:18:38 2016 -0800
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Thu Feb 4 15:18:38 2016 -0800

----------------------------------------------------------------------
 .../scheduler/storage/db/TaskConfigManager.java |  6 +-
 .../scheduler/storage/log/LogStorage.java       |  9 +--
 .../storage/log/SnapshotStoreImpl.java          |  7 +-
 .../scheduler/storage/log/ThriftBackfill.java   | 70 ++++++++++++++++++++
 .../scheduler/storage/log/LogStorageTest.java   | 41 +++++++++---
 .../storage/log/SnapshotStoreImplTest.java      | 64 ++++++++++++++----
 .../storage/log/ThriftBackfillTest.java         | 65 ++++++++++++++++++
 7 files changed, 228 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
index 27f1f33..364026a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
@@ -40,8 +40,12 @@ class TaskConfigManager {
   }
 
   private Optional<Long> getConfigRow(ITaskConfig config) {
-    // We could optimize this slightly by first comparing the un-hydrated row and breaking
early.
+    // NOTE: The 'config' object passed in MUST have all version-relevant fields populated
in order
+    // to correctly compare with objects loaded from DB. This may not hold true if a 'config'
is
+    // passed from storage recovery routine during version downgrade and fields are not properly
+    // backfilled. See AURORA-1603 for more details.
 
+    // We could optimize this slightly by first comparing the un-hydrated row and breaking
early.
     Map<ITaskConfig, DbTaskConfig> rowsByConfig =
         Maps.uniqueIndex(
             configMapper.selectConfigsByJob(config.getJob()),

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
index 042f71d..de16f2e 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java
@@ -62,16 +62,13 @@ import org.apache.aurora.scheduler.storage.Storage.MutateWork.NoResult;
 import org.apache.aurora.scheduler.storage.Storage.NonVolatileStorage;
 import org.apache.aurora.scheduler.storage.TaskStore;
 import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
-import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.IJobInstanceUpdateEvent;
 import org.apache.aurora.scheduler.storage.entities.IJobKey;
-import org.apache.aurora.scheduler.storage.entities.IJobUpdate;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateEvent;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey;
 import org.apache.aurora.scheduler.storage.entities.ILock;
 import org.apache.aurora.scheduler.storage.entities.ILockKey;
 import org.apache.aurora.scheduler.storage.entities.IResourceAggregate;
-import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -330,7 +327,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
         .put(Op._Fields.SAVE_CRON_JOB, op -> {
           SaveCronJob cronJob = op.getSaveCronJob();
           writeBehindJobStore.saveAcceptedJob(
-              IJobConfiguration.build(cronJob.getJobConfig()));
+              ThriftBackfill.backFillJobConfiguration(cronJob.getJobConfig()));
         })
         .put(
             Op._Fields.REMOVE_JOB,
@@ -338,7 +335,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
         .put(
             Op._Fields.SAVE_TASKS,
             op -> writeBehindTaskStore.saveTasks(
-            IScheduledTask.setFromBuilders(op.getSaveTasks().getTasks())))
+                ThriftBackfill.backFillScheduledTasks(op.getSaveTasks().getTasks())))
         .put(Op._Fields.REWRITE_TASK, op -> {
           RewriteTask rewriteTask = op.getRewriteTask();
           writeBehindTaskStore.unsafeModifyInPlace(
@@ -377,7 +374,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
         .put(Op._Fields.SAVE_JOB_UPDATE, op -> {
           JobUpdate update = op.getSaveJobUpdate().getJobUpdate();
           writeBehindJobUpdateStore.saveJobUpdate(
-              IJobUpdate.build(update),
+              ThriftBackfill.backFillJobUpdate(update),
               Optional.fromNullable(op.getSaveJobUpdate().getLockToken()));
         })
         .put(Op._Fields.SAVE_JOB_UPDATE_EVENT, op -> {

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
b/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
index db90150..08ea04e 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java
@@ -45,7 +45,6 @@ import org.apache.aurora.scheduler.storage.Storage.Volatile;
 import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.IJobInstanceUpdateEvent;
-import org.apache.aurora.scheduler.storage.entities.IJobUpdate;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateEvent;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey;
 import org.apache.aurora.scheduler.storage.entities.ILock;
@@ -122,7 +121,7 @@ public class SnapshotStoreImpl implements SnapshotStore<Snapshot>
{
 
           if (snapshot.isSetTasks()) {
             store.getUnsafeTaskStore().saveTasks(
-                IScheduledTask.setFromBuilders(snapshot.getTasks()));
+                ThriftBackfill.backFillScheduledTasks(snapshot.getTasks()));
           }
         }
       },
@@ -144,7 +143,7 @@ public class SnapshotStoreImpl implements SnapshotStore<Snapshot>
{
           if (snapshot.isSetCronJobs()) {
             for (StoredCronJob job : snapshot.getCronJobs()) {
               store.getCronJobStore().saveAcceptedJob(
-                  IJobConfiguration.build(job.getJobConfiguration()));
+                  ThriftBackfill.backFillJobConfiguration(job.getJobConfiguration()));
             }
           }
         }
@@ -206,7 +205,7 @@ public class SnapshotStoreImpl implements SnapshotStore<Snapshot>
{
             for (StoredJobUpdateDetails storedDetails : snapshot.getJobUpdateDetails()) {
               JobUpdateDetails details = storedDetails.getDetails();
               updateStore.saveJobUpdate(
-                  IJobUpdate.build(details.getUpdate()),
+                  ThriftBackfill.backFillJobUpdate(details.getUpdate()),
                   Optional.fromNullable(storedDetails.getLockToken()));
 
               if (details.getUpdateEventsSize() > 0) {

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java b/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java
new file mode 100644
index 0000000..9c88f57
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java
@@ -0,0 +1,70 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.log;
+
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import org.apache.aurora.gen.InstanceTaskConfig;
+import org.apache.aurora.gen.JobConfiguration;
+import org.apache.aurora.gen.JobUpdate;
+import org.apache.aurora.gen.JobUpdateInstructions;
+import org.apache.aurora.gen.ScheduledTask;
+import org.apache.aurora.gen.TaskConfig;
+import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
+import org.apache.aurora.scheduler.storage.entities.IJobUpdate;
+import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
+
+/**
+ * Populates deprecated fields to ensure backwards compatibility between scheduler releases.
+ * See AURORA-1603 for more details.
+ */
+final class ThriftBackfill {
+
+  private ThriftBackfill() {
+    // Utility class.
+  }
+
+  static Set<IScheduledTask> backFillScheduledTasks(Set<ScheduledTask> tasks)
{
+    tasks.stream().forEach(t -> backFillTaskConfig(t.getAssignedTask().getTask()));
+    return tasks.stream().map(t -> IScheduledTask.build(t)).collect(Collectors.toSet());
+  }
+
+  static IJobConfiguration backFillJobConfiguration(JobConfiguration jobConfiguration) {
+    backFillTaskConfig(jobConfiguration.getTaskConfig());
+    return IJobConfiguration.build(jobConfiguration);
+  }
+
+  static IJobUpdate backFillJobUpdate(JobUpdate update) {
+    JobUpdateInstructions instructions = update.getInstructions();
+    if (instructions.isSetDesiredState()) {
+      backFillTaskConfig(instructions.getDesiredState().getTask());
+    }
+
+    for (InstanceTaskConfig instanceConfig : instructions.getInitialState()) {
+      backFillTaskConfig(instanceConfig.getTask());
+    }
+
+    return IJobUpdate.build(update);
+  }
+
+  private static TaskConfig backFillTaskConfig(TaskConfig task) {
+    task.setJobName(task.getJob().getName()).setEnvironment(task.getJob().getEnvironment());
+    if (task.isSetOwner()) {
+      task.getOwner().setRole(task.getJob().getRole());
+    }
+
+    return task;
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
index 7382eca..13fcf9f 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
@@ -24,6 +24,7 @@ import com.google.common.base.Functions;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.common.hash.HashFunction;
 import com.google.common.hash.Hashing;
@@ -37,6 +38,7 @@ import org.apache.aurora.common.testing.easymock.EasyMockTest;
 import org.apache.aurora.gen.AssignedTask;
 import org.apache.aurora.gen.Attribute;
 import org.apache.aurora.gen.HostAttributes;
+import org.apache.aurora.gen.Identity;
 import org.apache.aurora.gen.InstanceTaskConfig;
 import org.apache.aurora.gen.JobConfiguration;
 import org.apache.aurora.gen.JobInstanceUpdateEvent;
@@ -121,6 +123,7 @@ import static org.easymock.EasyMock.expectLastCall;
 import static org.easymock.EasyMock.notNull;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
 public class LogStorageTest extends EasyMockTest {
@@ -274,17 +277,27 @@ public class LogStorageTest extends EasyMockTest {
     builder.add(createTransaction(Op.saveFrameworkId(new SaveFrameworkId("bob"))));
     storageUtil.schedulerStore.saveFrameworkId("bob");
 
-    SaveCronJob cronJob = new SaveCronJob().setJobConfig(new JobConfiguration());
-    builder.add(createTransaction(Op.saveCronJob(cronJob)));
-    storageUtil.jobStore.saveAcceptedJob(IJobConfiguration.build(cronJob.getJobConfig()));
+    TaskConfig task = new TaskConfig()
+        .setJob(JOB_KEY.newBuilder())
+        .setOwner(new Identity(null, "user"));
+    TaskConfig backfilledTask = new TaskConfig(task)
+        .setJobName(JOB_KEY.getName())
+        .setEnvironment(JOB_KEY.getEnvironment());
+    backfilledTask.getOwner().setRole(JOB_KEY.getRole());
+
+    builder.add(createTransaction(Op.saveCronJob(new SaveCronJob()
+        .setJobConfig(new JobConfiguration().setTaskConfig(task)))));
+    storageUtil.jobStore.saveAcceptedJob(IJobConfiguration.build(new JobConfiguration()
+        .setTaskConfig(backfilledTask)));
 
     RemoveJob removeJob = new RemoveJob(JOB_KEY.newBuilder());
     builder.add(createTransaction(Op.removeJob(removeJob)));
     storageUtil.jobStore.removeJob(JOB_KEY);
 
-    SaveTasks saveTasks = new SaveTasks(ImmutableSet.of(new ScheduledTask()));
-    builder.add(createTransaction(Op.saveTasks(saveTasks)));
-    storageUtil.taskStore.saveTasks(IScheduledTask.setFromBuilders(saveTasks.getTasks()));
+    builder.add(createTransaction(Op.saveTasks(new SaveTasks(ImmutableSet.of(new ScheduledTask()
+        .setAssignedTask(new AssignedTask().setTask(task)))))));
+    storageUtil.taskStore.saveTasks(ImmutableSet.of(IScheduledTask.build(new ScheduledTask()
+        .setAssignedTask(new AssignedTask().setTask(backfilledTask)))));
 
     RewriteTask rewriteTask = new RewriteTask("id1", new TaskConfig());
     builder.add(createTransaction(Op.rewriteTask(rewriteTask)));
@@ -327,12 +340,22 @@ public class LogStorageTest extends EasyMockTest {
     builder.add(createTransaction(Op.removeLock(removeLock)));
     storageUtil.lockStore.removeLock(ILockKey.build(removeLock.getLockKey()));
 
-    JobUpdate update = new JobUpdate().setSummary(
-        new JobUpdateSummary().setKey(UPDATE_ID.newBuilder()));
+    JobUpdate update = new JobUpdate()
+        .setSummary(new JobUpdateSummary().setKey(UPDATE_ID.newBuilder()))
+        .setInstructions(new JobUpdateInstructions()
+            .setDesiredState(new InstanceTaskConfig().setTask(task))
+            .setInitialState(ImmutableSet.of(new InstanceTaskConfig().setTask(task))));
+    JobUpdate backfilledUpdate = new JobUpdate(update);
+    backfilledUpdate.getInstructions()
+        .setDesiredState(new InstanceTaskConfig().setTask(backfilledTask))
+        .setInitialState(ImmutableSet.of(new InstanceTaskConfig().setTask(backfilledTask)));
     SaveJobUpdate saveUpdate = new SaveJobUpdate(update, "token");
+
+    assertNull(Iterables.getOnlyElement(
+        saveUpdate.getJobUpdate().getInstructions().getInitialState()).getTask().getJobName());
     builder.add(createTransaction(Op.saveJobUpdate(saveUpdate)));
     storageUtil.jobUpdateStore.saveJobUpdate(
-        IJobUpdate.build(saveUpdate.getJobUpdate()),
+        IJobUpdate.build(backfilledUpdate),
         Optional.of(saveUpdate.getLockToken()));
 
     SaveJobUpdateEvent saveUpdateEvent =

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
b/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
index 806f50d..3b176db 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java
@@ -25,14 +25,18 @@ import com.google.common.collect.Maps;
 import org.apache.aurora.common.testing.easymock.EasyMockTest;
 import org.apache.aurora.common.util.testing.FakeBuildInfo;
 import org.apache.aurora.common.util.testing.FakeClock;
+import org.apache.aurora.gen.AssignedTask;
 import org.apache.aurora.gen.Attribute;
 import org.apache.aurora.gen.HostAttributes;
+import org.apache.aurora.gen.Identity;
+import org.apache.aurora.gen.InstanceTaskConfig;
 import org.apache.aurora.gen.JobConfiguration;
 import org.apache.aurora.gen.JobInstanceUpdateEvent;
 import org.apache.aurora.gen.JobKey;
 import org.apache.aurora.gen.JobUpdate;
 import org.apache.aurora.gen.JobUpdateDetails;
 import org.apache.aurora.gen.JobUpdateEvent;
+import org.apache.aurora.gen.JobUpdateInstructions;
 import org.apache.aurora.gen.JobUpdateKey;
 import org.apache.aurora.gen.JobUpdateStatus;
 import org.apache.aurora.gen.JobUpdateSummary;
@@ -40,6 +44,7 @@ import org.apache.aurora.gen.Lock;
 import org.apache.aurora.gen.LockKey;
 import org.apache.aurora.gen.ScheduleStatus;
 import org.apache.aurora.gen.ScheduledTask;
+import org.apache.aurora.gen.TaskConfig;
 import org.apache.aurora.gen.storage.QuotaConfiguration;
 import org.apache.aurora.gen.storage.SchedulerMetadata;
 import org.apache.aurora.gen.storage.Snapshot;
@@ -56,6 +61,7 @@ import org.apache.aurora.scheduler.storage.entities.IJobUpdateDetails;
 import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey;
 import org.apache.aurora.scheduler.storage.entities.ILock;
 import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
+import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
 import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
 import org.junit.Before;
 import org.junit.Test;
@@ -67,6 +73,7 @@ import static org.junit.Assert.assertEquals;
 public class SnapshotStoreImplTest extends EasyMockTest {
 
   private static final long NOW = 10335463456L;
+  private static final JobKey JOB_KEY = JobKeys.from("role", "env", "job").newBuilder();
 
   private StorageTestUtil storageUtil;
   private SnapshotStore<Snapshot> snapshotStore;
@@ -83,14 +90,29 @@ public class SnapshotStoreImplTest extends EasyMockTest {
   }
 
   private static IJobUpdateKey makeKey(String id) {
-    return IJobUpdateKey.build(
-        new JobUpdateKey(JobKeys.from("role", "env", "job").newBuilder(), id));
+    return IJobUpdateKey.build(new JobUpdateKey(JOB_KEY, id));
   }
 
   @Test
   public void testCreateAndRestoreNewSnapshot() {
-    ImmutableSet<IScheduledTask> tasks = ImmutableSet.of(
-        IScheduledTask.build(new ScheduledTask().setStatus(ScheduleStatus.PENDING)));
+    // Having immutable objects is important here to overcome m -> i -> m' problem
with our wrappers
+    // when some primitive fields are not set.
+    ITaskConfig task = ITaskConfig.build(
+        new TaskConfig().setJob(JOB_KEY).setOwner(new Identity(null, "user")));
+    TaskConfig taskBuilder = new TaskConfig(task.newBuilder())
+        .setJobName(JOB_KEY.getName())
+        .setEnvironment(JOB_KEY.getEnvironment());
+    taskBuilder.getOwner().setRole(JOB_KEY.getRole());
+    ITaskConfig backfilledTask = ITaskConfig.build(taskBuilder);
+
+    ImmutableSet<IScheduledTask> tasks = ImmutableSet.of(IScheduledTask.build(new ScheduledTask()
+        .setAssignedTask(new AssignedTask().setTask(task.newBuilder()))
+        .setStatus(ScheduleStatus.PENDING)));
+    ImmutableSet<IScheduledTask> backFilledTasks = ImmutableSet.of(IScheduledTask.build(
+        new ScheduledTask()
+            .setAssignedTask(new AssignedTask().setTask(backfilledTask.newBuilder()))
+            .setStatus(ScheduleStatus.PENDING)));
+
     Set<QuotaConfiguration> quotas =
         ImmutableSet.of(
             new QuotaConfiguration("steve", ResourceAggregates.EMPTY.newBuilder()));
@@ -101,8 +123,10 @@ public class SnapshotStoreImplTest extends EasyMockTest {
     // dropped.
     IHostAttributes legacyAttribute = IHostAttributes.build(
         new HostAttributes("host", ImmutableSet.of()));
-    StoredCronJob job = new StoredCronJob(
-        new JobConfiguration().setKey(new JobKey("owner", "env", "name")));
+    StoredCronJob job =
+        new StoredCronJob(new JobConfiguration().setKey(JOB_KEY).setTaskConfig(task.newBuilder()));
+    IJobConfiguration backFilledJob = IJobConfiguration.build(
+        new JobConfiguration(job.getJobConfiguration()).setTaskConfig(backfilledTask.newBuilder()));
     String frameworkId = "framework_id";
     ILock lock = ILock.build(new Lock()
         .setKey(LockKey.job(JobKeys.from("testRole", "testEnv", "testJob").newBuilder()))
@@ -117,14 +141,26 @@ public class SnapshotStoreImplTest extends EasyMockTest {
     IJobUpdateKey updateId1 =  makeKey("updateId1");
     IJobUpdateKey updateId2 = makeKey("updateId2");
     IJobUpdateDetails updateDetails1 = IJobUpdateDetails.build(new JobUpdateDetails()
-        .setUpdate(new JobUpdate().setSummary(
-            new JobUpdateSummary().setKey(updateId1.newBuilder())))
+        .setUpdate(new JobUpdate()
+            .setSummary(new JobUpdateSummary().setKey(updateId1.newBuilder()))
+            .setInstructions(new JobUpdateInstructions()
+                .setDesiredState(new InstanceTaskConfig().setTask(task.newBuilder()))
+                .setInitialState(ImmutableSet.of(new InstanceTaskConfig().setTask(task.newBuilder()
+                )))))
         .setUpdateEvents(ImmutableList.of(new JobUpdateEvent().setStatus(JobUpdateStatus.ERROR)))
         .setInstanceEvents(ImmutableList.of(new JobInstanceUpdateEvent().setTimestampMs(123L))));
 
+    // The saved object for update1 should be backfilled with TaskConfig fields.
+    JobUpdateDetails backFilledDetails1 = new JobUpdateDetails(updateDetails1.newBuilder());
+    backFilledDetails1.getUpdate().getInstructions().getDesiredState()
+        .setTask(backfilledTask.newBuilder());
+    Iterables.getOnlyElement(backFilledDetails1.getUpdate().getInstructions().getInitialState())
+        .setTask(backfilledTask.newBuilder());
+
     IJobUpdateDetails updateDetails2 = IJobUpdateDetails.build(new JobUpdateDetails()
-        .setUpdate(new JobUpdate().setSummary(
-            new JobUpdateSummary().setKey(updateId2.newBuilder()))));
+        .setUpdate(new JobUpdate()
+            .setSummary(new JobUpdateSummary().setKey(updateId2.newBuilder()))
+            .setInstructions(new JobUpdateInstructions().setInitialState(ImmutableSet.of()))));
 
     storageUtil.expectOperations();
     expect(storageUtil.taskStore.fetchTasks(Query.unscoped())).andReturn(tasks);
@@ -143,14 +179,14 @@ public class SnapshotStoreImplTest extends EasyMockTest {
             new StoredJobUpdateDetails(updateDetails2.newBuilder(), null)));
 
     expectDataWipe();
-    storageUtil.taskStore.saveTasks(tasks);
+    storageUtil.taskStore.saveTasks(backFilledTasks);
     storageUtil.quotaStore.saveQuota("steve", ResourceAggregates.EMPTY);
     expect(storageUtil.attributeStore.saveHostAttributes(attribute)).andReturn(true);
-    storageUtil.jobStore.saveAcceptedJob(IJobConfiguration.build(job.getJobConfiguration()));
+    storageUtil.jobStore.saveAcceptedJob(backFilledJob);
     storageUtil.schedulerStore.saveFrameworkId(frameworkId);
     storageUtil.lockStore.saveLock(lock);
     storageUtil.jobUpdateStore.saveJobUpdate(
-        updateDetails1.getUpdate(), Optional.fromNullable(lockToken));
+        IJobUpdate.build(backFilledDetails1.getUpdate()), Optional.fromNullable(lockToken));
     storageUtil.jobUpdateStore.saveJobUpdateEvent(
         updateId1,
         Iterables.getOnlyElement(updateDetails1.getUpdateEvents()));
@@ -158,7 +194,7 @@ public class SnapshotStoreImplTest extends EasyMockTest {
         updateId1,
         Iterables.getOnlyElement(updateDetails1.getInstanceEvents()));
 
-    // The saved object for update2 should be backfilled.
+    // The saved object for update2 should be backfilled with update key.
     JobUpdate update2Expected = updateDetails2.getUpdate().newBuilder();
     update2Expected.getSummary().setKey(updateId2.newBuilder());
     storageUtil.jobUpdateStore.saveJobUpdate(

http://git-wip-us.apache.org/repos/asf/aurora/blob/505f3100/src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java
b/src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java
new file mode 100644
index 0000000..506d591
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java
@@ -0,0 +1,65 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.log;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+
+import org.apache.aurora.gen.Identity;
+import org.apache.aurora.gen.InstanceTaskConfig;
+import org.apache.aurora.gen.JobKey;
+import org.apache.aurora.gen.JobUpdate;
+import org.apache.aurora.gen.JobUpdateInstructions;
+import org.apache.aurora.gen.TaskConfig;
+import org.apache.aurora.scheduler.storage.entities.IJobUpdate;
+import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class ThriftBackfillTest {
+  private static final ITaskConfig TASK = ITaskConfig.build(new TaskConfig()
+      .setJob(new JobKey("role", "env", "name"))
+      .setOwner(new Identity(null, "user")));
+
+  @Test
+  public void testUpdateBackfill() {
+    JobUpdate update = update();
+    JobUpdate expected = update();
+    populateTask(expected.getInstructions().getDesiredState().getTask());
+    populateTask(Iterables.getOnlyElement(expected.getInstructions().getInitialState()).getTask());
+    assertEquals(IJobUpdate.build(expected), ThriftBackfill.backFillJobUpdate(update));
+  }
+
+  @Test
+  public void testUpdateBackfillNoDesiredState() {
+    JobUpdate update = update();
+    update.getInstructions().setDesiredState(null);
+    JobUpdate expected = update();
+    expected.getInstructions().setDesiredState(null);
+    populateTask(Iterables.getOnlyElement(expected.getInstructions().getInitialState()).getTask());
+    assertEquals(IJobUpdate.build(expected), ThriftBackfill.backFillJobUpdate(update));
+  }
+
+  private static JobUpdate update() {
+    return new JobUpdate().setInstructions(new JobUpdateInstructions()
+        .setDesiredState(new InstanceTaskConfig().setTask(TASK.newBuilder()))
+        .setInitialState(ImmutableSet.of(new InstanceTaskConfig().setTask(TASK.newBuilder()))));
+  }
+
+  private static TaskConfig populateTask(TaskConfig task) {
+    task.setJobName("name").setEnvironment("env").getOwner().setRole("role");
+    return task;
+  }
+}


Mime
View raw message