aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject incubator-aurora git commit: Suppressing duplicate update instance events.
Date Wed, 11 Mar 2015 01:22:39 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 7a6feee3f -> 21b41806e


Suppressing duplicate update instance events.

Bugs closed: AURORA-1097

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


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

Branch: refs/heads/master
Commit: 21b41806ee77a577e5315f9aae19a48653d2a6d4
Parents: 7a6feee
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Tue Mar 10 18:22:17 2015 -0700
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Tue Mar 10 18:22:17 2015 -0700

----------------------------------------------------------------------
 .../updater/JobUpdateControllerImpl.java        | 52 +++++++++++++-------
 .../aurora/scheduler/updater/JobUpdaterIT.java  | 18 +------
 2 files changed, 37 insertions(+), 33 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/21b41806/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
index 5882488..3d9ca41 100644
--- a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
@@ -25,6 +25,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Throwables;
+import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -571,19 +572,19 @@ class JobUpdateControllerImpl implements JobUpdateController {
     for (Map.Entry<Integer, SideEffect> entry : result.getSideEffects().entrySet())
{
       Iterable<InstanceUpdateStatus> statusChanges;
 
+      int instanceId = entry.getKey();
+      List<IJobInstanceUpdateEvent> savedEvents = updateStore.fetchInstanceEvents(key,
instanceId);
+
+      Set<JobUpdateAction> savedActions =
+          FluentIterable.from(savedEvents).transform(EVENT_TO_ACTION).toSet();
+
       // Don't bother persisting a sequence of status changes that represents an instance
that
       // was immediately recognized as being healthy and in the desired state.
-      if (entry.getValue().getStatusChanges().equals(NOOP_INSTANCE_UPDATE)) {
-        List<IJobInstanceUpdateEvent> savedEvents =
-            updateStore.fetchInstanceEvents(key, entry.getKey());
-        if (savedEvents.isEmpty()) {
-          LOG.info("Suppressing no-op update for instance " + entry.getKey());
-          statusChanges = ImmutableSet.of();
-        } else {
-          // We choose to risk redundant events in corner cases here (after failing over
while
-          // updates are in-flight) to simplify the implementation.
-          statusChanges = entry.getValue().getStatusChanges();
-        }
+      if (entry.getValue().getStatusChanges().equals(NOOP_INSTANCE_UPDATE)
+          && savedEvents.isEmpty()) {
+
+        LOG.info("Suppressing no-op update for instance " + instanceId);
+        statusChanges = ImmutableSet.of();
       } else {
         statusChanges = entry.getValue().getStatusChanges();
       }
@@ -592,12 +593,20 @@ class JobUpdateControllerImpl implements JobUpdateController {
         JobUpdateAction action = STATE_MAP.get(Pair.of(statusChange, updaterStatus));
         requireNonNull(action);
 
-        IJobInstanceUpdateEvent event = IJobInstanceUpdateEvent.build(
-            new JobInstanceUpdateEvent()
-                .setInstanceId(entry.getKey())
-                .setTimestampMs(clock.nowMillis())
-                .setAction(action));
-        updateStore.saveJobInstanceUpdateEvent(summary.getKey(), event);
+        // A given instance update action may only be issued once during the update lifecycle.
+        // Suppress duplicate events due to pause/resume operations.
+        if (savedActions.contains(action)) {
+          LOG.info(String.format("Suppressing duplicate update %s for instance %s.",
+              action,
+              instanceId));
+        } else {
+          IJobInstanceUpdateEvent event = IJobInstanceUpdateEvent.build(
+              new JobInstanceUpdateEvent()
+                  .setInstanceId(instanceId)
+                  .setTimestampMs(clock.nowMillis())
+                  .setAction(action));
+          updateStore.saveJobInstanceUpdateEvent(summary.getKey(), event);
+        }
       }
     }
 
@@ -652,6 +661,15 @@ class JobUpdateControllerImpl implements JobUpdateController {
   }
 
   @VisibleForTesting
+  static final Function<IJobInstanceUpdateEvent, JobUpdateAction> EVENT_TO_ACTION =
+      new Function<IJobInstanceUpdateEvent, JobUpdateAction>() {
+        @Override
+        public JobUpdateAction apply(IJobInstanceUpdateEvent event) {
+          return event.getAction();
+        }
+      };
+
+  @VisibleForTesting
   static String failureMessage(int instanceId, Failure failure) {
     return String.format("Latest failure: instance %d %s", instanceId, failure.getReason());
   }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/21b41806/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
index e119c49..1f1e02e 100644
--- a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
@@ -266,13 +266,6 @@ public class JobUpdaterIT extends EasyMockTest {
           return event.getInstanceId();
         }
       };
-  private static final Function<IJobInstanceUpdateEvent, JobUpdateAction> EVENT_TO_ACTION
=
-      new Function<IJobInstanceUpdateEvent, JobUpdateAction>() {
-        @Override
-        public JobUpdateAction apply(IJobInstanceUpdateEvent event) {
-          return event.getAction();
-        }
-      };
 
   private IJobUpdateDetails getDetails() {
     return storage.read(new Work.Quiet<IJobUpdateDetails>() {
@@ -298,7 +291,7 @@ public class JobUpdaterIT extends EasyMockTest {
     Multimap<Integer, IJobInstanceUpdateEvent> eventsByInstance =
         Multimaps.index(orderedEvents, EVENT_TO_INSTANCE);
     Multimap<Integer, JobUpdateAction> actionsByInstance =
-        Multimaps.transformValues(eventsByInstance, EVENT_TO_ACTION);
+        Multimaps.transformValues(eventsByInstance, JobUpdateControllerImpl.EVENT_TO_ACTION);
     assertEquals(expectedActions, actionsByInstance);
     assertEquals(expected, details.getUpdate().getSummary().getState().getStatus());
   }
@@ -375,8 +368,7 @@ public class JobUpdaterIT extends EasyMockTest {
     clock.advance(WATCH_TIMEOUT);
     updater.resume(UPDATE_ID, USER);
 
-    actions.putAll(1, INSTANCE_UPDATING, INSTANCE_UPDATED)
-        .put(2, INSTANCE_UPDATING);
+    actions.putAll(1, INSTANCE_UPDATED).put(2, INSTANCE_UPDATING);
     assertState(ROLLING_FORWARD, actions.build());
 
     // A task outside the scope of the update should be ignored by the updater.
@@ -443,8 +435,6 @@ public class JobUpdaterIT extends EasyMockTest {
 
     // Pulse arrives and instance 2 is updated.
     assertEquals(JobUpdatePulseStatus.OK, updater.pulse(UPDATE_ID));
-    actions.putAll(1, INSTANCE_UPDATING, INSTANCE_UPDATED);
-    actions.putAll(2, INSTANCE_UPDATING);
     changeState(JOB, 2, KILLED, ASSIGNED, STARTING, RUNNING);
     clock.advance(WATCH_TIMEOUT);
     actions.put(2, INSTANCE_UPDATED);
@@ -589,8 +579,6 @@ public class JobUpdaterIT extends EasyMockTest {
 
     // Update is resumed
     updater.resume(UPDATE_ID, USER);
-    actions.putAll(1, INSTANCE_UPDATING, INSTANCE_UPDATED);
-    actions.put(2, INSTANCE_UPDATING);
     assertState(ROLLING_FORWARD, actions.build());
 
     // Instance 2 is updated.
@@ -780,8 +768,6 @@ public class JobUpdaterIT extends EasyMockTest {
     assertState(ROLL_BACK_PAUSED, actions.build());
     clock.advance(ONE_DAY);
     updater.resume(UPDATE_ID, USER);
-    actions.putAll(1, INSTANCE_ROLLING_BACK)
-        .putAll(2, INSTANCE_ROLLING_BACK, INSTANCE_ROLLED_BACK);
     assertState(ROLLING_BACK, actions.build());
 
     // Instance 1 is removed.


Mime
View raw message