UpdateManager: Obey server-dictated poll interval when scheduling checks.
This abides by the current logic, as found in
UpdateCheckScheduler::ComputeNextIntervalAndFuzz(). New unit tests
added to verify this behavior, as well as the addition of a new
UpdaterProvider variable to pull this value from the UpdateAttempter.
BUG=chromium:358269
TEST=Unit tests.
Change-Id: I0ac67dea5a622823a9c4713ec7165a55bc0a5c92
Reviewed-on: https://chromium-review.googlesource.com/207471
Tested-by: Gilad Arnold <garnold@chromium.org>
Reviewed-by: Alex Vakulenko <avakulenko@chromium.org>
Reviewed-by: Alex Deymo <deymo@chromium.org>
Commit-Queue: Gilad Arnold <garnold@chromium.org>
diff --git a/update_manager/chromeos_policy.cc b/update_manager/chromeos_policy.cc
index ba3241e..98095bf 100644
--- a/update_manager/chromeos_policy.cc
+++ b/update_manager/chromeos_policy.cc
@@ -412,6 +412,8 @@
EvalStatus ChromeOSPolicy::NextUpdateCheckTime(EvaluationContext* ec,
State* state, string* error,
Time* next_update_check) const {
+ UpdaterProvider* const updater_provider = state->updater_provider();
+
// Don't check for updates too often. We limit the update checks to once every
// some interval. The interval is kTimeoutInitialInterval the first time and
// kTimeoutPeriodicInterval for the subsequent update checks. If the update
@@ -420,11 +422,11 @@
// many chromebooks running update checks at the exact same time, we add some
// fuzz to the interval.
const Time* updater_started_time =
- ec->GetValue(state->updater_provider()->var_updater_started_time());
+ ec->GetValue(updater_provider->var_updater_started_time());
POLICY_CHECK_VALUE_AND_FAIL(updater_started_time, error);
const base::Time* last_checked_time =
- ec->GetValue(state->updater_provider()->var_last_checked_time());
+ ec->GetValue(updater_provider->var_last_checked_time());
const uint64_t* seed = ec->GetValue(state->random_provider()->var_seed());
POLICY_CHECK_VALUE_AND_FAIL(seed, error);
@@ -438,23 +440,43 @@
return EvalStatus::kSucceeded;
}
- // Check for previous failed attempts to implement an exponential backoff.
- const unsigned int* consecutive_failed_update_checks = ec->GetValue(
- state->updater_provider()->var_consecutive_failed_update_checks());
- POLICY_CHECK_VALUE_AND_FAIL(consecutive_failed_update_checks, error);
+ // Check whether the server is enforcing a poll interval; if not, this value
+ // will be zero.
+ const unsigned int* server_dictated_poll_interval = ec->GetValue(
+ updater_provider->var_server_dictated_poll_interval());
+ POLICY_CHECK_VALUE_AND_FAIL(server_dictated_poll_interval, error);
- int interval = kTimeoutPeriodicInterval;
- int fuzz = kTimeoutRegularFuzz;
- for (unsigned int i = 0; i < *consecutive_failed_update_checks; ++i) {
- interval *= 2;
- fuzz = 0; // In case of backoff, fuzz is different (see below).
- if (interval > kTimeoutMaxBackoffInterval) {
- interval = kTimeoutMaxBackoffInterval;
- break;
+ int interval = *server_dictated_poll_interval;
+ int fuzz = 0;
+
+ // If no poll interval was dictated by server compute a backoff period,
+ // starting from a predetermined base periodic interval and increasing
+ // exponentially by the number of consecutive failed attempts.
+ if (interval == 0) {
+ const unsigned int* consecutive_failed_update_checks = ec->GetValue(
+ updater_provider->var_consecutive_failed_update_checks());
+ POLICY_CHECK_VALUE_AND_FAIL(consecutive_failed_update_checks, error);
+
+ interval = kTimeoutPeriodicInterval;
+ unsigned int num_failures = *consecutive_failed_update_checks;
+ while (interval < kTimeoutMaxBackoffInterval && num_failures) {
+ interval *= 2;
+ num_failures--;
}
}
- // Defer to a fuzz of +/-(interval / 2) in case of backoff.
+ // We cannot backoff longer than the predetermined maximum interval.
+ if (interval > kTimeoutMaxBackoffInterval)
+ interval = kTimeoutMaxBackoffInterval;
+
+ // We cannot backoff shorter than the predetermined periodic interval. Also,
+ // in this case set the fuzz to a predetermined regular value.
+ if (interval <= kTimeoutPeriodicInterval) {
+ interval = kTimeoutPeriodicInterval;
+ fuzz = kTimeoutRegularFuzz;
+ }
+
+ // If not otherwise determined, defer to a fuzz of +/-(interval / 2).
if (fuzz == 0)
fuzz = interval;
diff --git a/update_manager/chromeos_policy.h b/update_manager/chromeos_policy.h
index 75e31cc..9c9fe9d 100644
--- a/update_manager/chromeos_policy.h
+++ b/update_manager/chromeos_policy.h
@@ -65,6 +65,7 @@
FirstCheckIsAtMostInitialIntervalAfterStart);
FRIEND_TEST(UmChromeOSPolicyTest, RecurringCheckBaseIntervalAndFuzz);
FRIEND_TEST(UmChromeOSPolicyTest, RecurringCheckBackoffIntervalAndFuzz);
+ FRIEND_TEST(UmChromeOSPolicyTest, RecurringCheckServerDictatedPollInterval);
FRIEND_TEST(UmChromeOSPolicyTest, ExponentialBackoffIsCapped);
FRIEND_TEST(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForTheTimeout);
FRIEND_TEST(UmChromeOSPolicyTest, UpdateCheckAllowedWaitsForOOBE);
diff --git a/update_manager/chromeos_policy_unittest.cc b/update_manager/chromeos_policy_unittest.cc
index d0a2114..9ea2d6a 100644
--- a/update_manager/chromeos_policy_unittest.cc
+++ b/update_manager/chromeos_policy_unittest.cc
@@ -48,6 +48,8 @@
new Time(fake_clock_.GetWallclockTime()));
fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
reset(new unsigned int(0)); // NOLINT(readability/casting)
+ fake_state_.updater_provider()->var_server_dictated_poll_interval()->
+ reset(new unsigned int(0)); // NOLINT(readability/casting)
fake_state_.random_provider()->var_seed()->reset(
new uint64_t(4)); // chosen by fair dice roll.
@@ -211,6 +213,30 @@
next_update_check);
}
+TEST_F(UmChromeOSPolicyTest, RecurringCheckServerDictatedPollInterval) {
+ // Policy honors the server provided check poll interval.
+ Time next_update_check;
+
+ const unsigned int kInterval = ChromeOSPolicy::kTimeoutPeriodicInterval * 4;
+ fake_state_.updater_provider()->var_server_dictated_poll_interval()->
+ reset(new unsigned int(kInterval)); // NOLINT(readability/casting)
+ // We should not be backing off in this case.
+ fake_state_.updater_provider()->var_consecutive_failed_update_checks()->
+ reset(new unsigned int(2)); // NOLINT(readability/casting)
+
+ ExpectPolicyStatus(EvalStatus::kSucceeded,
+ &ChromeOSPolicy::NextUpdateCheckTime, &next_update_check);
+
+ EXPECT_LE(
+ fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+ kInterval - kInterval / 2),
+ next_update_check);
+ EXPECT_GE(
+ fake_clock_.GetWallclockTime() + TimeDelta::FromSeconds(
+ kInterval + kInterval / 2),
+ next_update_check);
+}
+
TEST_F(UmChromeOSPolicyTest, ExponentialBackoffIsCapped) {
Time next_update_check;
diff --git a/update_manager/fake_updater_provider.h b/update_manager/fake_updater_provider.h
index 52b64a0..e4c1866 100644
--- a/update_manager/fake_updater_provider.h
+++ b/update_manager/fake_updater_provider.h
@@ -66,6 +66,11 @@
return &var_consecutive_failed_update_checks_;
}
+ virtual FakeVariable<unsigned int>*
+ var_server_dictated_poll_interval() override {
+ return &var_server_dictated_poll_interval_;
+ }
+
private:
FakeVariable<base::Time>
var_updater_started_time_{ // NOLINT(whitespace/braces)
@@ -94,6 +99,9 @@
FakeVariable<unsigned int>
var_consecutive_failed_update_checks_{ // NOLINT(whitespace/braces)
"consecutive_failed_update_checks", kVariableModePoll};
+ FakeVariable<unsigned int>
+ var_server_dictated_poll_interval_{ // NOLINT(whitespace/braces)
+ "server_dictated_poll_interval", kVariableModePoll};
DISALLOW_COPY_AND_ASSIGN(FakeUpdaterProvider);
};
diff --git a/update_manager/real_updater_provider.cc b/update_manager/real_updater_provider.cc
index 6445972..47d5dfa 100644
--- a/update_manager/real_updater_provider.cc
+++ b/update_manager/real_updater_provider.cc
@@ -327,6 +327,22 @@
DISALLOW_COPY_AND_ASSIGN(ConsecutiveFailedUpdateChecksVariable);
};
+// A variable returning the server-dictated poll interval.
+class ServerDictatedPollIntervalVariable
+ : public UpdaterVariableBase<unsigned int> {
+ public:
+ using UpdaterVariableBase<unsigned int>::UpdaterVariableBase;
+
+ private:
+ virtual const unsigned int* GetValue(TimeDelta /* timeout */,
+ string* /* errmsg */) override {
+ return new unsigned int(
+ system_state()->update_attempter()->server_dictated_poll_interval());
+ }
+
+ DISALLOW_COPY_AND_ASSIGN(ServerDictatedPollIntervalVariable);
+};
+
// RealUpdaterProvider methods.
RealUpdaterProvider::RealUpdaterProvider(SystemState* system_state)
@@ -355,6 +371,9 @@
false)),
var_consecutive_failed_update_checks_(
new ConsecutiveFailedUpdateChecksVariable(
- "consecutive_failed_update_checks", system_state_)) {}
+ "consecutive_failed_update_checks", system_state_)),
+ var_server_dictated_poll_interval_(
+ new ServerDictatedPollIntervalVariable(
+ "server_dictated_poll_interval", system_state_)) {}
} // namespace chromeos_update_manager
diff --git a/update_manager/real_updater_provider.h b/update_manager/real_updater_provider.h
index 9462ab7..2bc30ae 100644
--- a/update_manager/real_updater_provider.h
+++ b/update_manager/real_updater_provider.h
@@ -78,6 +78,11 @@
return var_consecutive_failed_update_checks_.get();
}
+ virtual Variable<unsigned int>*
+ var_server_dictated_poll_interval() override {
+ return var_server_dictated_poll_interval_.get();
+ }
+
private:
// A pointer to the update engine's system state aggregator.
chromeos_update_engine::SystemState* system_state_;
@@ -95,6 +100,7 @@
scoped_ptr<Variable<bool>> var_p2p_enabled_;
scoped_ptr<Variable<bool>> var_cellular_enabled_;
scoped_ptr<Variable<unsigned int>> var_consecutive_failed_update_checks_;
+ scoped_ptr<Variable<unsigned int>> var_server_dictated_poll_interval_;
DISALLOW_COPY_AND_ASSIGN(RealUpdaterProvider);
};
diff --git a/update_manager/real_updater_provider_unittest.cc b/update_manager/real_updater_provider_unittest.cc
index 7bea777..6c4c435 100644
--- a/update_manager/real_updater_provider_unittest.cc
+++ b/update_manager/real_updater_provider_unittest.cc
@@ -454,4 +454,13 @@
kNumFailedChecks, provider_->var_consecutive_failed_update_checks());
}
+TEST_F(UmRealUpdaterProviderTest, GetServerDictatedPollInterval) {
+ const unsigned int kPollInterval = 2 * 60 * 60; // Two hours.
+ EXPECT_CALL(*fake_sys_state_.mock_update_attempter(),
+ server_dictated_poll_interval())
+ .WillRepeatedly(Return(kPollInterval));
+ UmTestUtils::ExpectVariableHasValue(
+ kPollInterval, provider_->var_server_dictated_poll_interval());
+}
+
} // namespace chromeos_update_manager
diff --git a/update_manager/updater_provider.h b/update_manager/updater_provider.h
index 2ec4166..f7a7648 100644
--- a/update_manager/updater_provider.h
+++ b/update_manager/updater_provider.h
@@ -79,6 +79,9 @@
// A variable returning the number of consecutive failed update checks.
virtual Variable<unsigned int>* var_consecutive_failed_update_checks() = 0;
+ // A server-dictated update check interval in seconds, if one was given.
+ virtual Variable<unsigned int>* var_server_dictated_poll_interval() = 0;
+
protected:
UpdaterProvider() {}