aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject git commit: Drop HostAttributes with no slaveId when recovering from persistent storage.
Date Fri, 08 Aug 2014 01:00:27 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master b0fef4b20 -> 5154f61e7


Drop HostAttributes with no slaveId when recovering from persistent storage.

Bugs closed: AURORA-641

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


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

Branch: refs/heads/master
Commit: 5154f61e7534f7f9c22d4760a6e12302b89c7dc5
Parents: b0fef4b
Author: Bill Farner <wfarner@apache.org>
Authored: Thu Aug 7 17:57:59 2014 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Thu Aug 7 17:57:59 2014 -0700

----------------------------------------------------------------------
 .../scheduler/storage/log/LogStorage.java       | 12 +++++++++--
 .../storage/log/SnapshotStoreImpl.java          |  9 ++++++++-
 .../scheduler/storage/log/LogStorageTest.java   | 21 ++++++++++++++++++--
 .../storage/log/SnapshotStoreImplTest.java      | 14 +++++++++----
 4 files changed, 47 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5154f61e/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 3454420..342bab0 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
@@ -36,6 +36,7 @@ import com.twitter.common.quantity.Time;
 import com.twitter.common.util.concurrent.ExecutorServiceShutdown;
 
 import org.apache.aurora.codec.ThriftBinaryCodec.CodingException;
+import org.apache.aurora.gen.HostAttributes;
 import org.apache.aurora.gen.storage.LogEntry;
 import org.apache.aurora.gen.storage.Op;
 import org.apache.aurora.gen.storage.RewriteTask;
@@ -439,8 +440,15 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
         break;
 
       case SAVE_HOST_ATTRIBUTES:
-        writeBehindAttributeStore.saveHostAttributes(
-            IHostAttributes.build(op.getSaveHostAttributes().hostAttributes));
+        HostAttributes attributes = op.getSaveHostAttributes().hostAttributes;
+        // Prior to commit 5cf760b, the store would persist maintenance mode changes for
+        // unknown hosts.  5cf760b began rejecting these, but the replicated log may still
+        // contain entries with a null slave ID.
+        if (attributes.isSetSlaveId()) {
+          writeBehindAttributeStore.saveHostAttributes(IHostAttributes.build(attributes));
+        } else {
+          LOG.info("Dropping host attributes with no slave ID: " + attributes);
+        }
         break;
 
       case SAVE_LOCK:

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5154f61e/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 5d3ec36..e921e71 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
@@ -80,7 +80,14 @@ public class SnapshotStoreImpl implements SnapshotStore<Snapshot>
{
 
           if (snapshot.isSetHostAttributes()) {
             for (HostAttributes attributes : snapshot.getHostAttributes()) {
-              store.getAttributeStore().saveHostAttributes(IHostAttributes.build(attributes));
+              // Prior to commit 5cf760b, the store would persist maintenance mode changes
for
+              // unknown hosts.  5cf760b began rejecting these, but the replicated log may
still
+              // contain entries with a null slave ID.
+              if (attributes.isSetSlaveId()) {
+                store.getAttributeStore().saveHostAttributes(IHostAttributes.build(attributes));
+              } else {
+                LOG.info("Dropping host attributes with no slave ID: " + attributes);
+              }
             }
           }
         }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5154f61e/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 a9545e1..8f2be53 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
@@ -45,6 +45,7 @@ import org.apache.aurora.gen.JobUpdateStatus;
 import org.apache.aurora.gen.JobUpdateSummary;
 import org.apache.aurora.gen.Lock;
 import org.apache.aurora.gen.LockKey;
+import org.apache.aurora.gen.MaintenanceMode;
 import org.apache.aurora.gen.Range;
 import org.apache.aurora.gen.ResourceAggregate;
 import org.apache.aurora.gen.ScheduleStatus;
@@ -186,15 +187,30 @@ public class LogStorageTest extends EasyMockTest {
 
     Entry entry1 = createMock(Entry.class);
     Entry entry2 = createMock(Entry.class);
+    Entry entry3 = createMock(Entry.class);
+    Entry entry4 = createMock(Entry.class);
     String frameworkId1 = "bob";
     LogEntry recoveredEntry1 =
         createTransaction(Op.saveFrameworkId(new SaveFrameworkId(frameworkId1)));
     String frameworkId2 = "jim";
     LogEntry recoveredEntry2 =
         createTransaction(Op.saveFrameworkId(new SaveFrameworkId(frameworkId2)));
+    // This entry lacks a slave ID, and should therefore be discarded.
+    LogEntry recoveredEntry3 =
+        createTransaction(Op.saveHostAttributes(new SaveHostAttributes(new HostAttributes()
+            .setHost("host1")
+            .setMode(MaintenanceMode.DRAINED))));
+    IHostAttributes attributes = IHostAttributes.build(new HostAttributes()
+        .setHost("host2")
+        .setSlaveId("slave2")
+        .setMode(MaintenanceMode.DRAINED));
+    LogEntry recoveredEntry4 =
+        createTransaction(Op.saveHostAttributes(new SaveHostAttributes(attributes.newBuilder())));
     expect(entry1.contents()).andReturn(ThriftBinaryCodec.encodeNonNull(recoveredEntry1));
     expect(entry2.contents()).andReturn(ThriftBinaryCodec.encodeNonNull(recoveredEntry2));
-    expect(stream.readAll()).andReturn(Iterators.forArray(entry1, entry2));
+    expect(entry3.contents()).andReturn(ThriftBinaryCodec.encodeNonNull(recoveredEntry3));
+    expect(entry4.contents()).andReturn(ThriftBinaryCodec.encodeNonNull(recoveredEntry4));
+    expect(stream.readAll()).andReturn(Iterators.forArray(entry1, entry2, entry3, entry4));
 
     final Capture<MutateWork<Void, RuntimeException>> recoveryWork = createCapture();
     expect(storageUtil.storage.write(capture(recoveryWork))).andAnswer(
@@ -207,6 +223,7 @@ public class LogStorageTest extends EasyMockTest {
         });
     storageUtil.schedulerStore.saveFrameworkId(frameworkId1);
     storageUtil.schedulerStore.saveFrameworkId(frameworkId2);
+    storageUtil.attributeStore.saveHostAttributes(attributes);
 
     final Capture<MutateWork<Void, RuntimeException>> initializationWork = createCapture();
     expect(storageUtil.storage.write(capture(initializationWork))).andAnswer(
@@ -238,7 +255,7 @@ public class LogStorageTest extends EasyMockTest {
             snapshotWork.getValue().apply(storageUtil.mutableStoreProvider);
             return null;
           }
-        }).times(2);
+        }).times(4);
 
     control.replay();
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5154f61e/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 48600b5..9b5ba5b 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
@@ -80,8 +80,13 @@ public class SnapshotStoreImplTest extends EasyMockTest {
     Set<QuotaConfiguration> quotas =
         ImmutableSet.of(
             new QuotaConfiguration("steve", ResourceAggregates.none().newBuilder()));
-    IHostAttributes attribute = IHostAttributes.build(new HostAttributes("host",
-        ImmutableSet.of(new Attribute("attr", ImmutableSet.of("value")))));
+    IHostAttributes attribute = IHostAttributes.build(
+        new HostAttributes("host", ImmutableSet.of(new Attribute("attr", ImmutableSet.of("value"))))
+            .setSlaveId("slave id"));
+    // A legacy attribute that has a maintenance mode set, but nothing else.  These should
be
+    // dropped.
+    IHostAttributes legacyAttribute = IHostAttributes.build(
+        new HostAttributes("host", ImmutableSet.<Attribute>of()));
     StoredJob job = new StoredJob(
         "jobManager",
         new JobConfiguration().setKey(new JobKey("owner", "env", "name")));
@@ -104,7 +109,8 @@ public class SnapshotStoreImplTest extends EasyMockTest {
     expect(storageUtil.taskStore.fetchTasks(Query.unscoped())).andReturn(tasks);
     expect(storageUtil.quotaStore.fetchQuotas())
         .andReturn(ImmutableMap.of("steve", ResourceAggregates.none()));
-    expect(storageUtil.attributeStore.getHostAttributes()).andReturn(ImmutableSet.of(attribute));
+    expect(storageUtil.attributeStore.getHostAttributes())
+        .andReturn(ImmutableSet.of(attribute, legacyAttribute));
     expect(storageUtil.jobStore.fetchManagerIds()).andReturn(ImmutableSet.of("jobManager"));
     expect(storageUtil.jobStore.fetchJobs("jobManager"))
         .andReturn(ImmutableSet.of(IJobConfiguration.build(job.getJobConfiguration())));
@@ -136,7 +142,7 @@ public class SnapshotStoreImplTest extends EasyMockTest {
         .setTimestamp(NOW)
         .setTasks(IScheduledTask.toBuildersSet(tasks))
         .setQuotaConfigurations(quotas)
-        .setHostAttributes(ImmutableSet.of(attribute.newBuilder()))
+        .setHostAttributes(ImmutableSet.of(attribute.newBuilder(), legacyAttribute.newBuilder()))
         .setJobs(ImmutableSet.of(job))
         .setSchedulerMetadata(metadata)
         .setLocks(ImmutableSet.of(lock.newBuilder()))


Mime
View raw message