Implement exponential backoff for throttling repeated AU downloads.
Today we retry the same payload over and over again every hour. Ideally,
we shouldn't require ever to re-download the same payload again. But
from experience we find that post-install or firmware updates may succeed
on a second attempt. So until we have code that can do such selective
retries of those steps, we currently re-download and re-apply the whole
payload. So instead of retrying over and over again, we backoff the
successive payload download attempts at 1 day, 2 days, 4 days, etc. with
an upper limit of 16 days.
Another subtle reason for which we depend on the payload retry mechanism
today is if we've failed downloading the payload via all the URLs that are
specified in the rule, we don't want to keep re-attempting the download.
This case is different from the case discussed above, because in this case
we haven't even downloaded a payload once completely. In this case also,
there's a need for throttling the amount of bytes we end up downloading
repeatedly for a particular operation that may fail. This is done by
treating the exhaustion of all URLs as equivalent to having downloaded
a full payload and subjecting it to the same backoff behavior.
We waive backoffs for dev/test images so as not to cause any delay in
our testing or development.
BUG=chromium-os:36806
TEST=Added new unit tests. Tested all scenarios on my ZGB.
Change-Id: I6bd0d3f296a3c0da0a8026fb71b24785d825e39c
Reviewed-on: https://gerrit.chromium.org/gerrit/40220
Commit-Queue: Jay Srinivasan <jaysri@chromium.org>
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Tested-by: Jay Srinivasan <jaysri@chromium.org>
diff --git a/action_processor.h b/action_processor.h
index ba0e512..7c425de 100644
--- a/action_processor.h
+++ b/action_processor.h
@@ -63,10 +63,11 @@
kActionCodeOmahaErrorInHTTPResponse = 37,
kActionCodeDownloadOperationHashMissingError = 38,
kActionCodeDownloadMetadataSignatureMissingError = 39,
+ kActionCodeOmahaUpdateDeferredForBackoff = 40,
// Note: When adding new error codes, please remember to add the
// error into one of the buckets in PayloadState::UpdateFailed method so
- // that the retries across URLs and the payload back-off mechanism work
+ // that the retries across URLs and the payload backoff mechanism work
// correctly for those new error codes.
// Any code above this is sent to both Omaha and UMA as-is, except
diff --git a/delta_performer.cc b/delta_performer.cc
index 27ee556..3dbfbc0 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -883,7 +883,7 @@
// the one whose size matches the size mentioned in Omaha response. If any
// errors happen after this, it's likely a problem with the payload itself or
// the state of the system and not a problem with the URL or network. So,
- // indicate that to the payload state so that AU can back-off appropriately.
+ // indicate that to the payload state so that AU can backoff appropriately.
system_state_->payload_state()->DownloadComplete();
return kActionCodeSuccess;
diff --git a/mock_payload_state.h b/mock_payload_state.h
index 9756ac9..049624b 100644
--- a/mock_payload_state.h
+++ b/mock_payload_state.h
@@ -17,14 +17,19 @@
return true;
}
+ // Significant methods.
MOCK_METHOD1(SetResponse, void(const OmahaResponse& response));
MOCK_METHOD0(DownloadComplete, void());
MOCK_METHOD1(DownloadProgress, void(size_t count));
MOCK_METHOD1(UpdateFailed, void(ActionExitCode error));
- MOCK_METHOD0(GetResponse, std::string());
+ MOCK_METHOD0(ShouldBackoffDownload, bool());
+
+ // Getters.
+ MOCK_METHOD0(GetResponseSignature, std::string());
MOCK_METHOD0(GetPayloadAttemptNumber, uint32_t());
MOCK_METHOD0(GetUrlIndex, uint32_t());
MOCK_METHOD0(GetUrlFailureCount, uint32_t());
+ MOCK_METHOD0(GetBackoffExpiryTime, base::Time());
};
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index c98357b..679d326 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -31,7 +31,9 @@
// List of custom pair tags that we interpret in the Omaha Response:
static const char* kTagDeadline = "deadline";
+static const char* kTagDisablePayloadBackoff = "DisablePayloadBackoff";
static const char* kTagDisplayVersion = "DisplayVersion";
+static const char* kTagIsDeltaPayload = "IsDelta";
static const char* kTagMaxFailureCountPerUrl = "MaxFailureCountPerUrl";
static const char* kTagMaxDaysToScatter = "MaxDaysToScatter";
// Deprecated: "ManifestSignatureRsa"
@@ -597,9 +599,15 @@
ParseInt(XmlGetProperty(pie_action_node, kTagMaxDaysToScatter));
string max = XmlGetProperty(pie_action_node, kTagMaxFailureCountPerUrl);
- if (!base::StringToInt(max, &output_object->max_failure_count_per_url))
+ if (!base::StringToUint(max, &output_object->max_failure_count_per_url))
output_object->max_failure_count_per_url = kDefaultMaxFailureCountPerUrl;
+ output_object->is_delta_payload =
+ XmlGetProperty(pie_action_node, kTagIsDeltaPayload) == "true";
+
+ output_object->disable_payload_backoff =
+ XmlGetProperty(pie_action_node, kTagDisablePayloadBackoff) == "true";
+
return true;
}
@@ -691,9 +699,20 @@
// Update the payload state with the current response. The payload state
// will automatically reset all stale state if this response is different
- // from what's stored already.
+ // from what's stored already. We are updating the payload state as late
+ // as possible in this method so that if a new release gets pushed and then
+ // got pulled back due to some issues, we don't want to clear our internal
+ // state unnecessarily.
PayloadStateInterface* payload_state = system_state_->payload_state();
payload_state->SetResponse(output_object);
+
+ if (payload_state->ShouldBackoffDownload()) {
+ output_object.update_exists = false;
+ LOG(INFO) << "Ignoring Omaha updates in order to backoff our retry "
+ "attempts";
+ completer.set_code(kActionCodeOmahaUpdateDeferredForBackoff);
+ return;
+ }
}
bool OmahaRequestAction::ShouldDeferDownload(OmahaResponse* output_object) {
diff --git a/omaha_request_action.h b/omaha_request_action.h
index ebd3ed0..b6e31af 100644
--- a/omaha_request_action.h
+++ b/omaha_request_action.h
@@ -18,6 +18,7 @@
#include "update_engine/action.h"
#include "update_engine/http_fetcher.h"
+#include "update_engine/omaha_response.h"
// The Omaha Request action makes a request to Omaha and can output
// the response on the output ActionPipe.
@@ -28,47 +29,6 @@
// UTF-8 formatted. Output will be UTF-8 formatted.
std::string XmlEncode(const std::string& input);
-// This struct encapsulates the data Omaha's response for the request.
-// These strings in this struct are not XML escaped.
-struct OmahaResponse {
- OmahaResponse()
- : update_exists(false),
- poll_interval(0),
- size(0),
- metadata_size(0),
- max_days_to_scatter(0),
- max_failure_count_per_url(0),
- needs_admin(false),
- prompt(false) {}
- // True iff there is an update to be downloaded.
- bool update_exists;
-
- // If non-zero, server-dictated poll frequency in seconds.
- int poll_interval;
-
- // These are only valid if update_exists is true:
- std::string display_version;
-
- // The ordered list of URLs in the Omaha response. Each item is a complete
- // URL (i.e. in terms of Omaha XML, each value is a urlBase + packageName)
- std::vector<std::string> payload_urls;
-
- std::string more_info_url;
- std::string hash;
- std::string metadata_signature;
- std::string deadline;
- off_t size;
- off_t metadata_size;
- int max_days_to_scatter;
- // The number of URL-related failures to tolerate before moving on to the
- // next URL in the current pass. This is a configurable value from the
- // Omaha Response attribute, if ever we need to fine tune the behavior.
- int max_failure_count_per_url;
- bool needs_admin;
- bool prompt;
-};
-COMPILE_ASSERT(sizeof(off_t) == 8, off_t_not_64bit);
-
// This struct encapsulates the Omaha event information. For a
// complete list of defined event types and results, see
// http://code.google.com/p/omaha/wiki/ServerProtocol#event
diff --git a/omaha_response.h b/omaha_response.h
new file mode 100644
index 0000000..ea4fab1
--- /dev/null
+++ b/omaha_response.h
@@ -0,0 +1,71 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_OMAHA_RESPONSE_H
+#define CHROMEOS_PLATFORM_UPDATE_ENGINE_OMAHA_RESPONSE_H
+
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include <string>
+#include <vector>
+
+namespace chromeos_update_engine {
+
+// This struct encapsulates the data Omaha's response for the request.
+// The strings in this struct are not XML escaped.
+struct OmahaResponse {
+ OmahaResponse()
+ : update_exists(false),
+ poll_interval(0),
+ size(0),
+ metadata_size(0),
+ max_days_to_scatter(0),
+ max_failure_count_per_url(0),
+ needs_admin(false),
+ prompt(false),
+ is_delta_payload(false),
+ disable_payload_backoff(false) {}
+
+ // True iff there is an update to be downloaded.
+ bool update_exists;
+
+ // If non-zero, server-dictated poll interval in seconds.
+ int poll_interval;
+
+ // These are only valid if update_exists is true:
+ std::string display_version;
+
+ // The ordered list of URLs in the Omaha response. Each item is a complete
+ // URL (i.e. in terms of Omaha XML, each value is a urlBase + packageName)
+ std::vector<std::string> payload_urls;
+
+ std::string more_info_url;
+ std::string hash;
+ std::string metadata_signature;
+ std::string deadline;
+ off_t size;
+ off_t metadata_size;
+ int max_days_to_scatter;
+ // The number of URL-related failures to tolerate before moving on to the
+ // next URL in the current pass. This is a configurable value from the
+ // Omaha Response attribute, if ever we need to fine tune the behavior.
+ uint32_t max_failure_count_per_url;
+ bool needs_admin;
+ bool prompt;
+
+ // True if the payload described in this response is a delta payload.
+ // False if it's a full payload.
+ bool is_delta_payload;
+
+ // True if the Omaha rule instructs us to disable the backoff logic
+ // on the client altogether. False otherwise.
+ bool disable_payload_backoff;
+};
+COMPILE_ASSERT(sizeof(off_t) == 8, off_t_not_64bit);
+
+} // namespace chromeos_update_engine
+
+#endif // CHROMEOS_PLATFORM_UPDATE_ENGINE_OMAHA_RESPONSE_H
diff --git a/payload_state.cc b/payload_state.cc
index a44fc92..26a3a3f 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -4,74 +4,65 @@
#include "update_engine/payload_state.h"
+#include <algorithm>
+
#include <base/logging.h>
#include <base/stringprintf.h>
-#include "update_engine/omaha_request_action.h"
#include "update_engine/prefs.h"
#include "update_engine/utils.h"
+using base::Time;
+using base::TimeDelta;
+using std::min;
using std::string;
namespace chromeos_update_engine {
-// Returns a string containing that subset of the fields from the OmahaResponse
-// which we're interested in persisting for the purpose of detecting whether
-// we should clear the rest of the payload state when we get a new
-// OmahaResponse.
-static string GetFilteredResponse(const OmahaResponse& response) {
- string mini_response = StringPrintf("NumURLs = %d\n",
- response.payload_urls.size());
+// We want to upperbound backoffs to 16 days
+static const uint32_t kMaxBackoffDays = 16;
- for (size_t i = 0; i < response.payload_urls.size(); i++)
- mini_response += StringPrintf("Url%d = %s\n",
- i, response.payload_urls[i].c_str());
-
- mini_response += StringPrintf("Payload Size = %llu\n"
- "Payload Sha256 Hash = %s\n"
- "Metadata Size = %llu\n"
- "Metadata Signature = %s\n",
- response.size,
- response.hash.c_str(),
- response.metadata_size,
- response.metadata_signature.c_str());
- return mini_response;
-}
+// We want to randomize retry attempts after the backoff by +/- 6 hours.
+static const uint32_t kMaxBackoffFuzzMinutes = 12 * 60;
bool PayloadState::Initialize(PrefsInterface* prefs) {
CHECK(prefs);
prefs_ = prefs;
- LoadResponse();
+ LoadResponseSignature();
LoadPayloadAttemptNumber();
LoadUrlIndex();
LoadUrlFailureCount();
- LogPayloadState();
+ LoadBackoffExpiryTime();
return true;
}
void PayloadState::SetResponse(const OmahaResponse& omaha_response) {
- CHECK(prefs_);
- num_urls_ = omaha_response.payload_urls.size();
- max_failure_count_per_url_ = omaha_response.max_failure_count_per_url;
- string new_response = GetFilteredResponse(omaha_response);
- bool has_response_changed = (response_ != new_response);
- response_ = new_response;
- LOG(INFO) << "Stored Response = \n" << response_;
- prefs_->SetString(kPrefsCurrentResponse, response_);
+ // Always store the latest response.
+ response_ = omaha_response;
- bool should_reset = false;
+ // Check if the "signature" of this response (i.e. the fields we care about)
+ // has changed.
+ string new_response_signature = CalculateResponseSignature();
+ bool has_response_changed = (response_signature_ != new_response_signature);
+
+ // If the response has changed, we should persist the new signature and
+ // clear away all the existing state.
if (has_response_changed) {
- LOG(INFO) << "Resetting all payload state as this is a new response";
- should_reset = true;
- } else if (url_index_ >= num_urls_) {
- LOG(INFO) << "Resetting all payload state as the persisted state "
- << "seems to have been tampered with";
- should_reset = true;
+ LOG(INFO) << "Resetting all persisted state as this is a new response";
+ SetResponseSignature(new_response_signature);
+ ResetPersistedState();
+ return;
}
- if (should_reset) {
- SetPayloadAttemptNumber(0);
- SetUrlIndex(0);
+ // This is the earliest point at which we can validate whether the URL index
+ // we loaded from the persisted state is a valid value. If the response
+ // hasn't changed but the URL index is invalid, it's indicative of some
+ // tampering of the persisted state.
+ if (url_index_ >= GetNumUrls()) {
+ LOG(INFO) << "Resetting all payload state as the url index seems to have "
+ "been tampered with";
+ ResetPersistedState();
+ return;
}
}
@@ -105,11 +96,9 @@
ActionExitCode base_error = utils::GetBaseErrorCode(error);
LOG(INFO) << "Updating payload state for error code: " << base_error;
- if (!num_urls_) {
- // Since we don't persist num_urls_, it's possible that we get an error in
- // our communication to Omaha before even OmahaRequestAction code had a
- // chance to call SetResponse (which sets num_urls_). So we should not
- // advance the url_index_ in such cases.
+ if (GetNumUrls() == 0) {
+ // This means we got this error even before we got a valid Omaha response.
+ // So we should not advance the url_index_ in such cases.
LOG(INFO) << "Ignoring failures until we get a valid Omaha response.";
return;
}
@@ -163,7 +152,7 @@
// regular retries at the next update check.
// 2. We have successfully downloaded the payload: In this case, the
// payload attempt number would have been incremented and would take care
- // of the back-off at the next update check.
+ // of the backoff at the next update check.
// In either case, there's no need to update URL index or failure count.
case kActionCodeOmahaRequestError:
case kActionCodeOmahaResponseHandlerError:
@@ -180,6 +169,7 @@
case kActionCodeOmahaResponseInvalid:
case kActionCodeOmahaUpdateIgnoredPerPolicy:
case kActionCodeOmahaUpdateDeferredPerPolicy:
+ case kActionCodeOmahaUpdateDeferredForBackoff:
LOG(INFO) << "Not incrementing URL index or failure count for this error";
break;
@@ -201,38 +191,80 @@
}
}
-void PayloadState::LogPayloadState() {
- LOG(INFO) << "Current Payload State:\n"
- << "Current Response = \n" << response_
- << "\nPayload Attempt Number = " << payload_attempt_number_
- << "\nCurrent URL Index = " << url_index_
- << "\nCurrent URL Failure Count = " << url_failure_count_;
+bool PayloadState::ShouldBackoffDownload() {
+ if (response_.disable_payload_backoff) {
+ LOG(INFO) << "Payload backoff logic is disabled. "
+ "Can proceed with the download";
+ return false;
+ }
+
+ if (response_.is_delta_payload) {
+ // If delta payloads fail, we want to fallback quickly to full payloads as
+ // they are more likely to succeed. Exponential backoffs would greatly
+ // slow down the fallback to full payloads. So we don't backoff for delta
+ // payloads.
+ LOG(INFO) << "No backoffs for delta payloads. "
+ << "Can proceed with the download";
+ return false;
+ }
+
+ if (!utils::IsOfficialBuild()) {
+ // Backoffs are needed only for official builds. We do not want any delays
+ // or update failures due to backoffs during testing or development.
+ LOG(INFO) << "No backoffs for test/dev images. "
+ << "Can proceed with the download";
+ return false;
+ }
+
+ if (backoff_expiry_time_.is_null()) {
+ LOG(INFO) << "No backoff expiry time has been set. "
+ << "Can proceed with the download";
+ return false;
+ }
+
+ if (backoff_expiry_time_ < Time::Now()) {
+ LOG(INFO) << "The backoff expiry time ("
+ << utils::ToString(backoff_expiry_time_)
+ << ") has elapsed. Can proceed with the download";
+ return false;
+ }
+
+ LOG(INFO) << "Cannot proceed with downloads as we need to backoff until "
+ << utils::ToString(backoff_expiry_time_);
+ return true;
}
void PayloadState::IncrementPayloadAttemptNumber() {
+ if (response_.is_delta_payload) {
+ LOG(INFO) << "Not incrementing payload attempt number for delta payloads";
+ return;
+ }
+
LOG(INFO) << "Incrementing the payload attempt number";
SetPayloadAttemptNumber(GetPayloadAttemptNumber() + 1);
-
- // TODO(jaysri): chromium-os:36806: Implement the payload back-off logic
- // that uses the payload attempt number.
+ UpdateBackoffExpiryTime();
}
void PayloadState::IncrementUrlIndex() {
uint32_t next_url_index = GetUrlIndex() + 1;
- if (next_url_index < num_urls_) {
+ if (next_url_index < GetNumUrls()) {
LOG(INFO) << "Incrementing the URL index for next attempt";
SetUrlIndex(next_url_index);
} else {
LOG(INFO) << "Resetting the current URL index (" << GetUrlIndex() << ") to "
- << "0 as we only have " << num_urls_ << " URL(s)";
+ << "0 as we only have " << GetNumUrls() << " URL(s)";
SetUrlIndex(0);
IncrementPayloadAttemptNumber();
}
+
+ // Whenever we update the URL index, we should also clear the URL failure
+ // count so we can start over fresh for the new URL.
+ SetUrlFailureCount(0);
}
void PayloadState::IncrementFailureCount() {
uint32_t next_url_failure_count = GetUrlFailureCount() + 1;
- if (next_url_failure_count < max_failure_count_per_url_) {
+ if (next_url_failure_count < response_.max_failure_count_per_url) {
LOG(INFO) << "Incrementing the URL failure count";
SetUrlFailureCount(next_url_failure_count);
} else {
@@ -242,15 +274,90 @@
}
}
-void PayloadState::LoadResponse() {
+void PayloadState::UpdateBackoffExpiryTime() {
+ if (response_.disable_payload_backoff) {
+ LOG(INFO) << "Resetting backoff expiry time as payload backoff is disabled";
+ SetBackoffExpiryTime(Time());
+ return;
+ }
+
+ if (GetPayloadAttemptNumber() == 0) {
+ SetBackoffExpiryTime(Time());
+ return;
+ }
+
+ // Since we're doing left-shift below, make sure we don't shift more
+ // than this. E.g. if uint32_t is 4-bytes, don't left-shift more than 30 bits,
+ // since we don't expect value of kMaxBackoffDays to be more than 100 anyway.
+ uint32_t num_days = 1; // the value to be shifted.
+ const uint32_t kMaxShifts = (sizeof(num_days) * 8) - 2;
+
+ // Normal backoff days is 2 raised to (payload_attempt_number - 1).
+ // E.g. if payload_attempt_number is over 30, limit power to 30.
+ uint32_t power = min(GetPayloadAttemptNumber() - 1, kMaxShifts);
+
+ // The number of days is the minimum of 2 raised to (payload_attempt_number
+ // - 1) or kMaxBackoffDays.
+ num_days = min(num_days << power, kMaxBackoffDays);
+
+ // We don't want all retries to happen exactly at the same time when
+ // retrying after backoff. So add some random minutes to fuzz.
+ int fuzz_minutes = utils::FuzzInt(0, kMaxBackoffFuzzMinutes);
+ TimeDelta next_backoff_interval = TimeDelta::FromDays(num_days) +
+ TimeDelta::FromMinutes(fuzz_minutes);
+ LOG(INFO) << "Incrementing the backoff expiry time by "
+ << utils::FormatTimeDelta(next_backoff_interval);
+ SetBackoffExpiryTime(Time::Now() + next_backoff_interval);
+}
+
+void PayloadState::ResetPersistedState() {
+ SetPayloadAttemptNumber(0);
+ SetUrlIndex(0);
+ SetUrlFailureCount(0);
+ UpdateBackoffExpiryTime(); // This will reset the backoff expiry time.
+}
+
+string PayloadState::CalculateResponseSignature() {
+ string response_sign = StringPrintf("NumURLs = %d\n",
+ response_.payload_urls.size());
+
+ for (size_t i = 0; i < response_.payload_urls.size(); i++)
+ response_sign += StringPrintf("Url%d = %s\n",
+ i, response_.payload_urls[i].c_str());
+
+ response_sign += StringPrintf("Payload Size = %llu\n"
+ "Payload Sha256 Hash = %s\n"
+ "Metadata Size = %llu\n"
+ "Metadata Signature = %s\n"
+ "Is Delta Payload = %d\n"
+ "Max Failure Count Per Url = %d\n"
+ "Disable Payload Backoff = %d\n",
+ response_.size,
+ response_.hash.c_str(),
+ response_.metadata_size,
+ response_.metadata_signature.c_str(),
+ response_.is_delta_payload,
+ response_.max_failure_count_per_url,
+ response_.disable_payload_backoff);
+ return response_sign;
+}
+
+void PayloadState::LoadResponseSignature() {
CHECK(prefs_);
string stored_value;
- if (prefs_->Exists(kPrefsCurrentResponse) &&
- prefs_->GetString(kPrefsCurrentResponse, &stored_value)) {
- response_ = stored_value;
+ if (prefs_->Exists(kPrefsCurrentResponseSignature) &&
+ prefs_->GetString(kPrefsCurrentResponseSignature, &stored_value)) {
+ SetResponseSignature(stored_value);
}
}
+void PayloadState::SetResponseSignature(string response_signature) {
+ CHECK(prefs_);
+ response_signature_ = response_signature;
+ LOG(INFO) << "Current Response Signature = \n" << response_signature_;
+ prefs_->SetString(kPrefsCurrentResponseSignature, response_signature_);
+}
+
void PayloadState::LoadPayloadAttemptNumber() {
CHECK(prefs_);
int64_t stored_value;
@@ -261,7 +368,7 @@
<< ") in persisted state. Defaulting to 0";
stored_value = 0;
}
- payload_attempt_number_ = stored_value;
+ SetPayloadAttemptNumber(stored_value);
}
}
@@ -277,12 +384,14 @@
int64_t stored_value;
if (prefs_->Exists(kPrefsCurrentUrlIndex) &&
prefs_->GetInt64(kPrefsCurrentUrlIndex, &stored_value)) {
+ // We only check for basic sanity value here. Detailed check will be
+ // done in SetResponse once the first response comes in.
if (stored_value < 0) {
LOG(ERROR) << "Invalid URL Index (" << stored_value
<< ") in persisted state. Defaulting to 0";
stored_value = 0;
}
- url_index_ = stored_value;
+ SetUrlIndex(stored_value);
}
}
@@ -291,11 +400,6 @@
url_index_ = url_index;
LOG(INFO) << "Current URL Index = " << url_index_;
prefs_->SetInt64(kPrefsCurrentUrlIndex, url_index_);
-
- // Everytime we set the URL index, we should also reset its failure count.
- // Otherwise, the URL will be tried only once, instead of
- // max_failure_count_per_url times in the next round.
- SetUrlFailureCount(0);
}
void PayloadState::LoadUrlFailureCount() {
@@ -308,7 +412,7 @@
<< ") in persisted state. Defaulting to 0";
stored_value = 0;
}
- url_failure_count_ = stored_value;
+ SetUrlFailureCount(stored_value);
}
}
@@ -320,4 +424,32 @@
prefs_->SetInt64(kPrefsCurrentUrlFailureCount, url_failure_count_);
}
+void PayloadState::LoadBackoffExpiryTime() {
+ CHECK(prefs_);
+ int64_t stored_value;
+ if (!prefs_->Exists(kPrefsBackoffExpiryTime))
+ return;
+
+ if (!prefs_->GetInt64(kPrefsBackoffExpiryTime, &stored_value))
+ return;
+
+ Time stored_time = Time::FromInternalValue(stored_value);
+ if (stored_time > Time::Now() + TimeDelta::FromDays(kMaxBackoffDays)) {
+ LOG(ERROR) << "Invalid backoff expiry time ("
+ << utils::ToString(stored_time)
+ << ") in persisted state. Resetting.";
+ stored_time = Time();
+ }
+ SetBackoffExpiryTime(stored_time);
+}
+
+void PayloadState::SetBackoffExpiryTime(const Time& new_time) {
+ CHECK(prefs_);
+ backoff_expiry_time_ = new_time;
+ LOG(INFO) << "Backoff Expiry Time = "
+ << utils::ToString(backoff_expiry_time_);
+ prefs_->SetInt64(kPrefsBackoffExpiryTime,
+ backoff_expiry_time_.ToInternalValue());
+}
+
} // namespace chromeos_update_engine
diff --git a/payload_state.h b/payload_state.h
index cc90cf4..eb9d655 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -5,27 +5,26 @@
#ifndef CHROMEOS_PLATFORM_UPDATE_ENGINE_PAYLOAD_STATE_H__
#define CHROMEOS_PLATFORM_UPDATE_ENGINE_PAYLOAD_STATE_H__
+#include <base/time.h>
+
#include "update_engine/payload_state_interface.h"
#include "update_engine/prefs_interface.h"
namespace chromeos_update_engine {
// Encapsulates all the payload state required for download. This includes the
-// state necessary for handling multiple URLs in Omaha response, the back-off
+// state necessary for handling multiple URLs in Omaha response, the backoff
// state, etc. All state is persisted so that we use the most recently saved
// value when resuming the update_engine process. All state is also cached in
// memory so that we ensure we always make progress based on last known good
// state even when there's any issue in reading/writing from the file system.
class PayloadState : public PayloadStateInterface {
public:
-
PayloadState()
: prefs_(NULL),
payload_attempt_number_(0),
- num_urls_(0),
url_index_(0),
- url_failure_count_(0),
- max_failure_count_per_url_(0) {}
+ url_failure_count_(0) {}
virtual ~PayloadState() {}
@@ -42,9 +41,10 @@
virtual void DownloadComplete();
virtual void DownloadProgress(size_t count);
virtual void UpdateFailed(ActionExitCode error);
+ virtual bool ShouldBackoffDownload();
- virtual inline std::string GetResponse() {
- return response_;
+ virtual inline std::string GetResponseSignature() {
+ return response_signature_;
}
virtual inline uint32_t GetPayloadAttemptNumber() {
@@ -59,11 +59,12 @@
return url_failure_count_;
}
- private:
- // Logs the current payload state.
- void LogPayloadState();
+ virtual inline base::Time GetBackoffExpiryTime() {
+ return backoff_expiry_time_;
+ }
- // Increments the payload attempt number which governs the back-off behavior
+ private:
+ // Increments the payload attempt number which governs the backoff behavior
// at the time of the next update check.
void IncrementPayloadAttemptNumber();
@@ -78,8 +79,26 @@
// to the next URL and resets the failure count for that URL.
void IncrementFailureCount();
- // Initializes the current response from the persisted state.
- void LoadResponse();
+ // Updates the backoff expiry time exponentially based on the current
+ // payload attempt number.
+ void UpdateBackoffExpiryTime();
+
+ // Resets all the persisted state values which are maintained relative to the
+ // current response signature. The response signature itself is not reset.
+ void ResetPersistedState();
+
+ // Calculates the response "signature", which is basically a string composed
+ // of the subset of the fields in the current response that affect the
+ // behavior of the PayloadState.
+ std::string CalculateResponseSignature();
+
+ // Initializes the current response signature from the persisted state.
+ void LoadResponseSignature();
+
+ // Sets the response signature to the given value. Also persists the value
+ // being set so that we resume from the save value in case of a process
+ // restart.
+ void SetResponseSignature(std::string response_signature);
// Initializes the payload attempt number from the persisted state.
void LoadPayloadAttemptNumber();
@@ -105,14 +124,26 @@
// restart.
void SetUrlFailureCount(uint32_t url_failure_count);
+ // Initializes the backoff expiry time from the persisted state.
+ void LoadBackoffExpiryTime();
+
+ // Sets the backoff expiry time to the given value. Also persists the value
+ // being set so that we resume from the same value in case of a process
+ // restart.
+ void SetBackoffExpiryTime(const base::Time& new_time);
+
// Interface object with which we read/write persisted state. This must
// be set by calling the Initialize method before calling any other method.
PrefsInterface* prefs_;
- // This stores a subset of the current response from Omaha. Each update to
+ // This is the current response object from Omaha.
+ OmahaResponse response_;
+
+ // This stores a "signature" of the current response. The signature here
+ // refers to a subset of the current response from Omaha. Each update to
// this value is persisted so we resume from the same value in case of a
// process restart.
- std::string response_;
+ std::string response_signature_;
// The number of times we've tried to download the payload in full. This is
// incremented each time we download the payload in full successsfully or
@@ -121,10 +152,6 @@
// we resume from the same value in case of a process restart.
uint32_t payload_attempt_number_;
- // The number of urls in the current response. This value is not persisted,
- // as we will always get a response from Omaha before we need this value.
- uint32_t num_urls_;
-
// The index of the current URL. This type is different from the one in the
// accessor methods because PrefsInterface supports only int64_t but we want
// to provide a stronger abstraction of uint32_t. Each update to this value
@@ -137,10 +164,16 @@
// persisted so we resume from the same value in case of a process restart.
int64_t url_failure_count_;
- // The max failure count per url configured in the current response. This
- // value is not persisted, as we will always get a response from Omaha before
- // we need this value.
- uint32_t max_failure_count_per_url_;
+ // The timestamp until which we've to wait before attempting to download the
+ // payload again, so as to backoff repeated downloads.
+ base::Time backoff_expiry_time_;
+
+ // Returns the number of URLs in the current response.
+ // Note: This value will be 0 if this method is called before we receive
+ // the first valid Omaha response in this process.
+ uint32_t GetNumUrls() {
+ return response_.payload_urls.size();
+ }
DISALLOW_COPY_AND_ASSIGN(PayloadState);
};
diff --git a/payload_state_interface.h b/payload_state_interface.h
index 031fbec..35bab04 100644
--- a/payload_state_interface.h
+++ b/payload_state_interface.h
@@ -8,13 +8,10 @@
#include <string>
#include "update_engine/action_processor.h"
+#include "update_engine/omaha_response.h"
namespace chromeos_update_engine {
-// Forward declaration here because we get a circular dependency if
-// we include omaha_request_action.h directly.
-struct OmahaResponse;
-
// Describes the methods that need to be implemented by the PayloadState class.
// This interface has been carved out to support mocking of the PayloadState
// object.
@@ -47,8 +44,13 @@
// depending on the type of the error that happened.
virtual void UpdateFailed(ActionExitCode error) = 0;
- // Returns the currently stored response.
- virtual std::string GetResponse() = 0;
+ // Returns true if we should backoff the current download attempt.
+ // False otherwise.
+ virtual bool ShouldBackoffDownload() = 0;
+
+ // Returns the currently stored response "signature". The signature is a
+ // subset of fields that are of interest to the PayloadState behavior.
+ virtual std::string GetResponseSignature() = 0;
// Returns the payload attempt number.
virtual uint32_t GetPayloadAttemptNumber() = 0;
@@ -58,6 +60,9 @@
// Returns the current URL's failure count.
virtual uint32_t GetUrlFailureCount() = 0;
+
+ // Returns the expiry time for the current backoff period.
+ virtual base::Time GetBackoffExpiryTime() = 0;
};
} // namespace chromeos_update_engine
diff --git a/payload_state_unittest.cc b/payload_state_unittest.cc
index f1d586a..97bebd7 100644
--- a/payload_state_unittest.cc
+++ b/payload_state_unittest.cc
@@ -14,6 +14,8 @@
#include "update_engine/test_utils.h"
#include "update_engine/utils.h"
+using base::Time;
+using base::TimeDelta;
using std::string;
using testing::_;
using testing::NiceMock;
@@ -34,23 +36,29 @@
response->metadata_signature = "metasign";
response->max_failure_count_per_url = 3;
payload_state->SetResponse(*response);
- string stored_response = payload_state->GetResponse();
- string expected_response = StringPrintf(
+ string stored_response_sign = payload_state->GetResponseSignature();
+ string expected_response_sign = StringPrintf(
"NumURLs = 2\n"
"Url0 = http://test\n"
"Url1 = https://test\n"
"Payload Size = 523456789\n"
"Payload Sha256 Hash = %s\n"
"Metadata Size = 558123\n"
- "Metadata Signature = metasign\n",
- hash.c_str());
- EXPECT_EQ(expected_response, stored_response);
+ "Metadata Signature = metasign\n"
+ "Is Delta Payload = %d\n"
+ "Max Failure Count Per Url = %d\n"
+ "Disable Payload Backoff = %d\n",
+ hash.c_str(),
+ response->is_delta_payload,
+ response->max_failure_count_per_url,
+ response->disable_payload_backoff);
+ EXPECT_EQ(expected_response_sign, stored_response_sign);
}
class PayloadStateTest : public ::testing::Test { };
TEST(PayloadStateTest, DidYouAddANewActionExitCode) {
- if (kActionCodeUmaReportedMax != 40) {
+ if (kActionCodeUmaReportedMax != 41) {
LOG(ERROR) << "The following failure is intentional. If you added a new "
<< "ActionExitCode enum value, make sure to add it to the "
<< "PayloadState::UpdateFailed method and then update this test "
@@ -64,18 +72,22 @@
OmahaResponse response;
NiceMock<PrefsMock> prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
PayloadState payload_state;
EXPECT_TRUE(payload_state.Initialize(&prefs));
payload_state.SetResponse(response);
- string stored_response = payload_state.GetResponse();
- string expected_response = "NumURLs = 0\n"
- "Payload Size = 0\n"
- "Payload Sha256 Hash = \n"
- "Metadata Size = 0\n"
- "Metadata Signature = \n";
- EXPECT_EQ(expected_response, stored_response);
+ string stored_response_sign = payload_state.GetResponseSignature();
+ string expected_response_sign = "NumURLs = 0\n"
+ "Payload Size = 0\n"
+ "Payload Sha256 Hash = \n"
+ "Metadata Size = 0\n"
+ "Metadata Signature = \n"
+ "Is Delta Payload = 0\n"
+ "Max Failure Count Per Url = 0\n"
+ "Disable Payload Backoff = 0\n";
+ EXPECT_EQ(expected_response_sign, stored_response_sign);
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
}
@@ -89,19 +101,23 @@
response.metadata_signature = "msign";
NiceMock<PrefsMock> prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
PayloadState payload_state;
EXPECT_TRUE(payload_state.Initialize(&prefs));
payload_state.SetResponse(response);
- string stored_response = payload_state.GetResponse();
- string expected_response = "NumURLs = 1\n"
- "Url0 = http://single.url.test\n"
- "Payload Size = 123456789\n"
- "Payload Sha256 Hash = hash\n"
- "Metadata Size = 58123\n"
- "Metadata Signature = msign\n";
- EXPECT_EQ(expected_response, stored_response);
+ string stored_response_sign = payload_state.GetResponseSignature();
+ string expected_response_sign = "NumURLs = 1\n"
+ "Url0 = http://single.url.test\n"
+ "Payload Size = 123456789\n"
+ "Payload Sha256 Hash = hash\n"
+ "Metadata Size = 58123\n"
+ "Metadata Signature = msign\n"
+ "Is Delta Payload = 0\n"
+ "Max Failure Count Per Url = 0\n"
+ "Disable Payload Backoff = 0\n";
+ EXPECT_EQ(expected_response_sign, stored_response_sign);
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
}
@@ -116,20 +132,24 @@
response.metadata_signature = "metasign";
NiceMock<PrefsMock> prefs;
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0));
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0));
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0));
PayloadState payload_state;
EXPECT_TRUE(payload_state.Initialize(&prefs));
payload_state.SetResponse(response);
- string stored_response = payload_state.GetResponse();
- string expected_response = "NumURLs = 2\n"
- "Url0 = http://multiple.url.test\n"
- "Url1 = https://multiple.url.test\n"
- "Payload Size = 523456789\n"
- "Payload Sha256 Hash = rhash\n"
- "Metadata Size = 558123\n"
- "Metadata Signature = metasign\n";
- EXPECT_EQ(expected_response, stored_response);
+ string stored_response_sign = payload_state.GetResponseSignature();
+ string expected_response_sign = "NumURLs = 2\n"
+ "Url0 = http://multiple.url.test\n"
+ "Url1 = https://multiple.url.test\n"
+ "Payload Size = 523456789\n"
+ "Payload Sha256 Hash = rhash\n"
+ "Metadata Size = 558123\n"
+ "Metadata Signature = metasign\n"
+ "Is Delta Payload = 0\n"
+ "Max Failure Count Per Url = 0\n"
+ "Disable Payload Backoff = 0\n";
+ EXPECT_EQ(expected_response_sign, stored_response_sign);
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
}
@@ -142,6 +162,7 @@
// Payload attempt should start with 0 and then advance to 1.
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0)).Times(1);
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(2);
// Url index should go from 0 to 1 twice.
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(2);
@@ -204,6 +225,8 @@
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 2)).Times(1);
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(4);
+
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(4);
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 1)).Times(2);
@@ -241,12 +264,14 @@
EXPECT_EQ(1, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_TRUE(payload_state.ShouldBackoffDownload());
// This should advance the URL index.
payload_state.UpdateFailed(kActionCodePayloadHashMismatchError);
EXPECT_EQ(1, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(1, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_TRUE(payload_state.ShouldBackoffDownload());
// This should advance the URL index and payload attempt number due to
// wrap-around of URL index.
@@ -254,6 +279,7 @@
EXPECT_EQ(2, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_TRUE(payload_state.ShouldBackoffDownload());
// This HTTP error code should only increase the failure count.
payload_state.UpdateFailed(static_cast<ActionExitCode>(
@@ -261,6 +287,7 @@
EXPECT_EQ(2, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(1, payload_state.GetUrlFailureCount());
+ EXPECT_TRUE(payload_state.ShouldBackoffDownload());
// And that failure count should be reset when we download some bytes
// afterwards.
@@ -268,6 +295,7 @@
EXPECT_EQ(2, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_TRUE(payload_state.ShouldBackoffDownload());
// Now, slightly change the response and set it again.
SetupPayloadStateWith2Urls("Hash8532", &payload_state, &response);
@@ -276,6 +304,7 @@
EXPECT_EQ(0, payload_state.GetPayloadAttemptNumber());
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
+ EXPECT_FALSE(payload_state.ShouldBackoffDownload());
}
TEST(PayloadStateTest, PayloadAttemptNumberIncreasesOnSuccessfulDownload) {
@@ -286,6 +315,8 @@
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 0)).Times(1);
EXPECT_CALL(prefs, SetInt64(kPrefsPayloadAttemptNumber, 1)).Times(1);
+ EXPECT_CALL(prefs, SetInt64(kPrefsBackoffExpiryTime, _)).Times(2);
+
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlIndex, 0)).Times(1);
EXPECT_CALL(prefs, SetInt64(kPrefsCurrentUrlFailureCount, 0)).Times(1);
@@ -324,6 +355,7 @@
NiceMock<PrefsMock> prefs2;
EXPECT_CALL(prefs2, Exists(_)).WillRepeatedly(Return(true));
EXPECT_CALL(prefs2, GetInt64(kPrefsPayloadAttemptNumber, _));
+ EXPECT_CALL(prefs2, GetInt64(kPrefsBackoffExpiryTime, _));
EXPECT_CALL(prefs2, GetInt64(kPrefsCurrentUrlIndex, _))
.WillOnce(DoAll(SetArgumentPointee<1>(2), Return(true)));
EXPECT_CALL(prefs2, GetInt64(kPrefsCurrentUrlFailureCount, _));
@@ -341,4 +373,92 @@
EXPECT_EQ(0, payload_state.GetUrlIndex());
EXPECT_EQ(0, payload_state.GetUrlFailureCount());
}
+
+TEST(PayloadStateTest, NoBackoffForDeltaPayloads) {
+ OmahaResponse response;
+ response.is_delta_payload = true;
+ PayloadState payload_state;
+ NiceMock<PrefsMock> prefs;
+
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ SetupPayloadStateWith2Urls("Hash6437", &payload_state, &response);
+
+ // Simulate a successful download and see that we're ready to download
+ // again without any backoff as this is a delta payload.
+ payload_state.DownloadComplete();
+ EXPECT_EQ(0, payload_state.GetPayloadAttemptNumber());
+ EXPECT_FALSE(payload_state.ShouldBackoffDownload());
+
+ // Simulate two failures (enough to cause payload backoff) and check
+ // again that we're ready to re-download without any backoff as this is
+ // a delta payload.
+ payload_state.UpdateFailed(kActionCodeDownloadMetadataSignatureMismatch);
+ payload_state.UpdateFailed(kActionCodeDownloadMetadataSignatureMismatch);
+ EXPECT_EQ(0, payload_state.GetUrlIndex());
+ EXPECT_EQ(0, payload_state.GetPayloadAttemptNumber());
+ EXPECT_FALSE(payload_state.ShouldBackoffDownload());
+}
+
+static void CheckPayloadBackoffState(PayloadState* payload_state,
+ int expected_attempt_number,
+ TimeDelta expected_days) {
+ payload_state->DownloadComplete();
+ EXPECT_EQ(expected_attempt_number, payload_state->GetPayloadAttemptNumber());
+ EXPECT_TRUE(payload_state->ShouldBackoffDownload());
+ Time backoff_expiry_time = payload_state->GetBackoffExpiryTime();
+ // Add 1 hour extra to the 6 hour fuzz check to tolerate edge cases.
+ TimeDelta max_fuzz_delta = TimeDelta::FromHours(7);
+ Time expected_min_time = Time::Now() + expected_days - max_fuzz_delta;
+ Time expected_max_time = Time::Now() + expected_days + max_fuzz_delta;
+ EXPECT_LT(expected_min_time.ToInternalValue(),
+ backoff_expiry_time.ToInternalValue());
+ EXPECT_GT(expected_max_time.ToInternalValue(),
+ backoff_expiry_time.ToInternalValue());
+}
+
+TEST(PayloadStateTest, BackoffPeriodsAreInCorrectRange) {
+ OmahaResponse response;
+ response.is_delta_payload = false;
+ PayloadState payload_state;
+ NiceMock<PrefsMock> prefs;
+
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ SetupPayloadStateWith2Urls("Hash8939", &payload_state, &response);
+
+ CheckPayloadBackoffState(&payload_state, 1, TimeDelta::FromDays(1));
+ CheckPayloadBackoffState(&payload_state, 2, TimeDelta::FromDays(2));
+ CheckPayloadBackoffState(&payload_state, 3, TimeDelta::FromDays(4));
+ CheckPayloadBackoffState(&payload_state, 4, TimeDelta::FromDays(8));
+ CheckPayloadBackoffState(&payload_state, 5, TimeDelta::FromDays(16));
+ CheckPayloadBackoffState(&payload_state, 6, TimeDelta::FromDays(16));
+ CheckPayloadBackoffState(&payload_state, 7, TimeDelta::FromDays(16));
+ CheckPayloadBackoffState(&payload_state, 8, TimeDelta::FromDays(16));
+ CheckPayloadBackoffState(&payload_state, 9, TimeDelta::FromDays(16));
+ CheckPayloadBackoffState(&payload_state, 10, TimeDelta::FromDays(16));
+}
+
+TEST(PayloadStateTest, BackoffLogicCanBeDisabled) {
+ OmahaResponse response;
+ response.disable_payload_backoff = true;
+ PayloadState payload_state;
+ NiceMock<PrefsMock> prefs;
+
+ EXPECT_TRUE(payload_state.Initialize(&prefs));
+ SetupPayloadStateWith2Urls("Hash8939", &payload_state, &response);
+
+ // Simulate a successful download and see that we are ready to download
+ // again without any backoff.
+ payload_state.DownloadComplete();
+ EXPECT_EQ(1, payload_state.GetPayloadAttemptNumber());
+ EXPECT_FALSE(payload_state.ShouldBackoffDownload());
+
+ // Test again, this time by simulating two errors that would cause
+ // the payload attempt number to increment due to wrap around. And
+ // check that we are still ready to re-download without any backoff.
+ payload_state.UpdateFailed(kActionCodeDownloadMetadataSignatureMismatch);
+ payload_state.UpdateFailed(kActionCodeDownloadMetadataSignatureMismatch);
+ EXPECT_EQ(2, payload_state.GetPayloadAttemptNumber());
+ EXPECT_FALSE(payload_state.ShouldBackoffDownload());
+}
+
}
diff --git a/prefs.cc b/prefs.cc
index 03a122a..3af3706 100644
--- a/prefs.cc
+++ b/prefs.cc
@@ -38,9 +38,10 @@
const char kPrefsUpdateFirstSeenAt[] = "update-first-seen-at";
const char kPrefsPayloadAttemptNumber[] = "payload-attempt-number";
-const char kPrefsCurrentResponse[] = "current-omaha-response";
+const char kPrefsCurrentResponseSignature[] = "current-response-signature";
const char kPrefsCurrentUrlIndex[] = "current-url-index";
const char kPrefsCurrentUrlFailureCount[] = "current-url-failure-count";
+const char kPrefsBackoffExpiryTime[] = "backoff-expiry-time";
bool Prefs::Init(const FilePath& prefs_dir) {
prefs_dir_ = prefs_dir;
diff --git a/prefs_interface.h b/prefs_interface.h
index 7bc56ca..14962dd 100644
--- a/prefs_interface.h
+++ b/prefs_interface.h
@@ -28,9 +28,10 @@
extern const char kPrefsWallClockWaitPeriod[];
extern const char kPrefsUpdateFirstSeenAt[];
extern const char kPrefsPayloadAttemptNumber[];
-extern const char kPrefsCurrentResponse[];
+extern const char kPrefsCurrentResponseSignature[];
extern const char kPrefsCurrentUrlIndex[];
extern const char kPrefsCurrentUrlFailureCount[];
+extern const char kPrefsBackoffExpiryTime[];
// The prefs interface allows access to a persistent preferences
// store. The two reasons for providing this as an interface are
diff --git a/system_state.h b/system_state.h
index 4dee1eb..a04c434 100644
--- a/system_state.h
+++ b/system_state.h
@@ -102,7 +102,7 @@
Prefs prefs_;
// All state pertaining to payload state such as
- // response, URL, back-off states.
+ // response, URL, backoff states.
PayloadState payload_state_;
};
diff --git a/update_check_scheduler.cc b/update_check_scheduler.cc
index 074cb4a..ff597c6 100644
--- a/update_check_scheduler.cc
+++ b/update_check_scheduler.cc
@@ -137,10 +137,10 @@
} else if ((http_response_code = update_attempter_->http_response_code()) ==
kHttpResponseInternalServerError ||
http_response_code == kHttpResponseServiceUnavailable) {
- // Implements exponential back off on 500 (Internal Server Error) and 503
+ // Implements exponential backoff on 500 (Internal Server Error) and 503
// (Service Unavailable) HTTP response codes.
interval = 2 * last_interval_;
- LOG(WARNING) << "Exponential back off due to HTTP response code ("
+ LOG(WARNING) << "Exponential backoff due to HTTP response code ("
<< http_response_code << ")";
}
@@ -149,7 +149,7 @@
interval = kTimeoutMaxBackoffInterval;
// Ensures that under normal conditions the regular update check interval
- // and fuzz are used. Also covers the case where back off is required based
+ // and fuzz are used. Also covers the case where backoff is required based
// on the initial update check.
if (interval < kTimeoutPeriodicInterval) {
interval = kTimeoutPeriodicInterval;