Fix OperationScheduler moratorium calculation for clock rollback case.
Make the unit test exercise some clock-rollback scenarios, using
a properly injected artificial clock.
Bug: 2579585
Change-Id: I6f81c32318ba27429bd30ff53b48449218e4ac64
diff --git a/common/java/com/android/common/OperationScheduler.java b/common/java/com/android/common/OperationScheduler.java
index 0a48fe7..08cc25b 100644
--- a/common/java/com/android/common/OperationScheduler.java
+++ b/common/java/com/android/common/OperationScheduler.java
@@ -124,7 +124,8 @@
}
/**
- * Compute the time of the next operation. Does not modify any state.
+ * Compute the time of the next operation. Does not modify any state
+ * (unless the clock rolls backwards, in which case timers are reset).
*
* @param options to use for this computation.
* @return the wall clock time ({@link System#currentTimeMillis()}) when the
@@ -143,11 +144,11 @@
// clipped to the current time so we don't languish forever.
int errorCount = mStorage.getInt(PREFIX + "errorCount", 0);
- long now = System.currentTimeMillis();
+ long now = currentTimeMillis();
long lastSuccessTimeMillis = getTimeBefore(PREFIX + "lastSuccessTimeMillis", now);
long lastErrorTimeMillis = getTimeBefore(PREFIX + "lastErrorTimeMillis", now);
long triggerTimeMillis = mStorage.getLong(PREFIX + "triggerTimeMillis", Long.MAX_VALUE);
- long moratoriumSetMillis = mStorage.getLong(PREFIX + "moratoriumSetTimeMillis", 0);
+ long moratoriumSetMillis = getTimeBefore(PREFIX + "moratoriumSetTimeMillis", now);
long moratoriumTimeMillis = getTimeBefore(PREFIX + "moratoriumTimeMillis",
moratoriumSetMillis + options.maxMoratoriumMillis);
@@ -155,9 +156,8 @@
if (options.periodicIntervalMillis > 0) {
time = Math.min(time, lastSuccessTimeMillis + options.periodicIntervalMillis);
}
- if (time >= moratoriumTimeMillis - options.maxMoratoriumMillis) {
- time = Math.max(time, moratoriumTimeMillis);
- }
+
+ time = Math.max(time, moratoriumTimeMillis);
time = Math.max(time, lastSuccessTimeMillis + options.minTriggerMillis);
if (errorCount > 0) {
time = Math.max(time, lastErrorTimeMillis + options.backoffFixedMillis +
@@ -205,7 +205,7 @@
/**
* Request an operation to be performed at a certain time. The actual
* scheduled time may be affected by error backoff logic and defined
- * minimum intervals.
+ * minimum intervals. Use {@link Long#MAX_VALUE} to disable triggering.
*
* @param millis wall clock time ({@link System#currentTimeMillis()}) to
* trigger another operation; 0 to trigger immediately
@@ -218,13 +218,13 @@
* Forbid any operations until after a certain (absolute) time.
* Limited by {@link #Options.maxMoratoriumMillis}.
*
- * @param millis wall clock time ({@link System#currentTimeMillis()}) to
- * wait before attempting any more operations; 0 to remove moratorium
+ * @param millis wall clock time ({@link System#currentTimeMillis()})
+ * when operations should be allowed again; 0 to remove moratorium
*/
public void setMoratoriumTimeMillis(long millis) {
mStorage.edit()
.putLong(PREFIX + "moratoriumTimeMillis", millis)
- .putLong(PREFIX + "moratoriumSetTimeMillis", System.currentTimeMillis())
+ .putLong(PREFIX + "moratoriumSetTimeMillis", currentTimeMillis())
.commit();
}
@@ -239,7 +239,7 @@
public boolean setMoratoriumTimeHttp(String retryAfter) {
try {
long ms = Long.valueOf(retryAfter) * 1000;
- setMoratoriumTimeMillis(ms + System.currentTimeMillis());
+ setMoratoriumTimeMillis(ms + currentTimeMillis());
return true;
} catch (NumberFormatException nfe) {
try {
@@ -269,13 +269,12 @@
public void onSuccess() {
resetTransientError();
resetPermanentError();
- long now = System.currentTimeMillis();
mStorage.edit()
.remove(PREFIX + "errorCount")
.remove(PREFIX + "lastErrorTimeMillis")
.remove(PREFIX + "permanentError")
.remove(PREFIX + "triggerTimeMillis")
- .putLong(PREFIX + "lastSuccessTimeMillis", now).commit();
+ .putLong(PREFIX + "lastSuccessTimeMillis", currentTimeMillis()).commit();
}
/**
@@ -284,8 +283,7 @@
* purposes.
*/
public void onTransientError() {
- long now = System.currentTimeMillis();
- mStorage.edit().putLong(PREFIX + "lastErrorTimeMillis", now).commit();
+ mStorage.edit().putLong(PREFIX + "lastErrorTimeMillis", currentTimeMillis()).commit();
mStorage.edit().putInt(PREFIX + "errorCount",
mStorage.getInt(PREFIX + "errorCount", 0) + 1).commit();
}
@@ -338,4 +336,13 @@
}
return out.append("]").toString();
}
+
+ /**
+ * Gets the current time. Can be overridden for unit testing.
+ *
+ * @return {@link System#currentTimeMillis()}
+ */
+ protected long currentTimeMillis() {
+ return System.currentTimeMillis();
+ }
}
diff --git a/common/tests/src/com/android/common/OperationSchedulerTest.java b/common/tests/src/com/android/common/OperationSchedulerTest.java
index 866d1a8..955508f 100644
--- a/common/tests/src/com/android/common/OperationSchedulerTest.java
+++ b/common/tests/src/com/android/common/OperationSchedulerTest.java
@@ -22,19 +22,34 @@
import android.test.suitebuilder.annotation.SmallTest;
public class OperationSchedulerTest extends AndroidTestCase {
+ /**
+ * OperationScheduler subclass which uses an artificial time.
+ * Set {@link #timeMillis} to whatever value you like.
+ */
+ private class TimeTravelScheduler extends OperationScheduler {
+ static final long DEFAULT_TIME = 1250146800000L; // 13-Aug-2009, 12:00:00 am
+ public long timeMillis = DEFAULT_TIME;
+
+ @Override
+ protected long currentTimeMillis() { return timeMillis; }
+ public TimeTravelScheduler() { super(getFreshStorage()); }
+ }
+
+ private SharedPreferences getFreshStorage() {
+ SharedPreferences sp = getContext().getSharedPreferences("OperationSchedulerTest", 0);
+ sp.edit().clear().commit();
+ return sp;
+ }
+
@MediumTest
public void testScheduler() throws Exception {
- String name = "OperationSchedulerTest.testScheduler";
- SharedPreferences storage = getContext().getSharedPreferences(name, 0);
- storage.edit().clear().commit();
-
- OperationScheduler scheduler = new OperationScheduler(storage);
+ TimeTravelScheduler scheduler = new TimeTravelScheduler();
OperationScheduler.Options options = new OperationScheduler.Options();
assertEquals(Long.MAX_VALUE, scheduler.getNextTimeMillis(options));
assertEquals(0, scheduler.getLastSuccessTimeMillis());
assertEquals(0, scheduler.getLastAttemptTimeMillis());
- long beforeTrigger = System.currentTimeMillis();
+ long beforeTrigger = scheduler.timeMillis;
scheduler.setTriggerTimeMillis(beforeTrigger + 1000000);
assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
@@ -51,33 +66,26 @@
assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
// Backoff interval after an error
- long beforeError = System.currentTimeMillis();
+ long beforeError = (scheduler.timeMillis += 100);
scheduler.onTransientError();
- long afterError = System.currentTimeMillis();
assertEquals(0, scheduler.getLastSuccessTimeMillis());
- assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
- assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
options.backoffFixedMillis = 1000000;
options.backoffIncrementalMillis = 500000;
- assertTrue(beforeError + 1500000 <= scheduler.getNextTimeMillis(options));
- assertTrue(afterError + 1500000 >= scheduler.getNextTimeMillis(options));
+ assertEquals(beforeError + 1500000, scheduler.getNextTimeMillis(options));
// Two errors: backoff interval increases
- beforeError = System.currentTimeMillis();
+ beforeError = (scheduler.timeMillis += 100);
scheduler.onTransientError();
- afterError = System.currentTimeMillis();
- assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
- assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
- assertTrue(beforeError + 2000000 <= scheduler.getNextTimeMillis(options));
- assertTrue(afterError + 2000000 >= scheduler.getNextTimeMillis(options));
+ assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeError + 2000000, scheduler.getNextTimeMillis(options));
// Reset transient error: no backoff interval
scheduler.resetTransientError();
assertEquals(0, scheduler.getLastSuccessTimeMillis());
assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
- assertTrue(beforeError <= scheduler.getLastAttemptTimeMillis());
- assertTrue(afterError >= scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
// Permanent error holds true even if transient errors are reset
// However, we remember that the transient error was reset...
@@ -89,30 +97,26 @@
assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
// Success resets the trigger
- long beforeSuccess = System.currentTimeMillis();
+ long beforeSuccess = (scheduler.timeMillis += 100);
scheduler.onSuccess();
- long afterSuccess = System.currentTimeMillis();
- assertTrue(beforeSuccess <= scheduler.getLastAttemptTimeMillis());
- assertTrue(afterSuccess >= scheduler.getLastAttemptTimeMillis());
- assertTrue(beforeSuccess <= scheduler.getLastSuccessTimeMillis());
- assertTrue(afterSuccess >= scheduler.getLastSuccessTimeMillis());
+ assertEquals(beforeSuccess, scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeSuccess, scheduler.getLastSuccessTimeMillis());
assertEquals(Long.MAX_VALUE, scheduler.getNextTimeMillis(options));
// The moratorium is not reset by success!
- scheduler.setTriggerTimeMillis(beforeSuccess + 500000);
+ scheduler.setTriggerTimeMillis(0);
assertEquals(beforeTrigger + 1500000, scheduler.getNextTimeMillis(options));
scheduler.setMoratoriumTimeMillis(0);
- assertEquals(beforeSuccess + 500000, scheduler.getNextTimeMillis(options));
+ assertEquals(beforeSuccess, scheduler.getNextTimeMillis(options));
// Periodic interval after success
options.periodicIntervalMillis = 250000;
- assertTrue(beforeSuccess + 250000 <= scheduler.getNextTimeMillis(options));
- assertTrue(afterSuccess + 250000 >= scheduler.getNextTimeMillis(options));
+ scheduler.setTriggerTimeMillis(Long.MAX_VALUE);
+ assertEquals(beforeSuccess + 250000, scheduler.getNextTimeMillis(options));
// Trigger minimum is also since the last success
options.minTriggerMillis = 1000000;
- assertTrue(beforeSuccess + 1000000 <= scheduler.getNextTimeMillis(options));
- assertTrue(afterSuccess + 1000000 >= scheduler.getNextTimeMillis(options));
+ assertEquals(beforeSuccess + 1000000, scheduler.getNextTimeMillis(options));
}
@SmallTest
@@ -138,23 +142,19 @@
@SmallTest
public void testMoratoriumWithHttpDate() throws Exception {
- String name = "OperationSchedulerTest.testMoratoriumWithHttpDate";
- SharedPreferences storage = getContext().getSharedPreferences(name, 0);
- storage.edit().clear().commit();
-
- OperationScheduler scheduler = new OperationScheduler(storage);
+ TimeTravelScheduler scheduler = new TimeTravelScheduler();
OperationScheduler.Options options = new OperationScheduler.Options();
- long beforeTrigger = System.currentTimeMillis();
+ long beforeTrigger = scheduler.timeMillis;
scheduler.setTriggerTimeMillis(beforeTrigger + 1000000);
assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
scheduler.setMoratoriumTimeMillis(beforeTrigger + 2000000);
assertEquals(beforeTrigger + 2000000, scheduler.getNextTimeMillis(options));
- long beforeMoratorium = System.currentTimeMillis();
+ long beforeMoratorium = scheduler.timeMillis;
assertTrue(scheduler.setMoratoriumTimeHttp("3000"));
- long afterMoratorium = System.currentTimeMillis();
+ long afterMoratorium = scheduler.timeMillis;
assertTrue(beforeMoratorium + 3000000 <= scheduler.getNextTimeMillis(options));
assertTrue(afterMoratorium + 3000000 >= scheduler.getNextTimeMillis(options));
@@ -164,4 +164,56 @@
assertFalse(scheduler.setMoratoriumTimeHttp("not actually a date"));
}
+
+ @SmallTest
+ public void testClockRollbackScenario() throws Exception {
+ TimeTravelScheduler scheduler = new TimeTravelScheduler();
+ OperationScheduler.Options options = new OperationScheduler.Options();
+ options.minTriggerMillis = 2000;
+
+ // First, set up a scheduler with reasons to wait: a transient
+ // error with backoff and a moratorium for a few minutes.
+
+ long beforeTrigger = scheduler.timeMillis;
+ long triggerTime = beforeTrigger - 10000000;
+ scheduler.setTriggerTimeMillis(triggerTime);
+ assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+ assertEquals(0, scheduler.getLastAttemptTimeMillis());
+
+ long beforeSuccess = (scheduler.timeMillis += 100);
+ scheduler.onSuccess();
+ scheduler.setTriggerTimeMillis(triggerTime);
+ assertEquals(beforeSuccess, scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeSuccess + 2000, scheduler.getNextTimeMillis(options));
+
+ long beforeError = (scheduler.timeMillis += 100);
+ scheduler.onTransientError();
+ assertEquals(beforeError, scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeError + 5000, scheduler.getNextTimeMillis(options));
+
+ long beforeMoratorium = (scheduler.timeMillis += 100);
+ scheduler.setMoratoriumTimeMillis(beforeTrigger + 1000000);
+ assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
+
+ // Now set the time back a few seconds.
+ // The moratorium time should still be honored.
+ long beforeRollback = (scheduler.timeMillis = beforeTrigger - 10000);
+ assertEquals(beforeTrigger + 1000000, scheduler.getNextTimeMillis(options));
+
+ // The rollback also moved the last-attempt clock back to the rollback time.
+ assertEquals(scheduler.timeMillis, scheduler.getLastAttemptTimeMillis());
+
+ // But if we set the time back more than a day, the moratorium
+ // resets to the maximum moratorium (a day, by default), exposing
+ // the original trigger time.
+ beforeRollback = (scheduler.timeMillis = beforeTrigger - 100000000);
+ assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+ assertEquals(beforeRollback, scheduler.getLastAttemptTimeMillis());
+
+ // If we roll forward until after the re-set moratorium, then it expires.
+ scheduler.timeMillis = triggerTime + 5000000;
+ assertEquals(triggerTime, scheduler.getNextTimeMillis(options));
+ assertEquals(beforeRollback, scheduler.getLastAttemptTimeMillis());
+ assertEquals(beforeRollback, scheduler.getLastSuccessTimeMillis());
+ }
}
