Continue with an older update if update progress is 30% or more
This feature is desirable in cases where a device is used infrequently
on a network with little or no bandwidth to the update servers. In a
nutshell, the problem is that the device will start downloading an
update but it's so slow that it will see a new update before it's
done. This new update will cause the device to throw away the old
update (and thereby the resources it has already expended downloading
it) and then start over again. Which means downloading an update and
then get interrupted again. Rinse and repeat. With this change, we'll
at least converge against an update (albeit, perhaps an old one)
instead of starting over again and again.
To implement this feature, a new "update-state-overall-progress"
persistent variable (so-called "preference" according to the codebase)
is introduced so we can easily evaluate the progress without having to
parse the manifest again.
This feature is difficult to test with devserver since it's always
using the same URL, e.g.
http://your-workstation.corp.company.net:8080/static/update.gz
meaning that it's not possible to actually continue an older update
server from deverser. What happens is that update_engine will happily
continue downloading and apply the update, but it will fail with
something like kErrorCodePayloadSizeMismatchError because it's getting
the new payload while expecting the old. This is not a problem per se,
it will just cause us to abandon the update attempt.
Fortunately, to verify that the patch works, it's sufficient to just
check the log files for these messages
[0612/163553:INFO:delta_performer.cc(1144)] Abandoning current update (at 12% completion) because there is a newer update available.
[0612/164451:INFO:delta_performer.cc(1139)] Resuming current update (at 51% completion) despite the fact that there's a newer update.
BUG=chromium:244538
TEST=Unit tests pass + Manually tested (see above)
Change-Id: Ibfb2fe10fe1883bfadf132f22b802ca3482c699e
Reviewed-on: https://gerrit.chromium.org/gerrit/58470
Reviewed-by: Chris Sosa <sosa@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
diff --git a/constants.cc b/constants.cc
index 3609a02..4dcb83b 100644
--- a/constants.cc
+++ b/constants.cc
@@ -41,6 +41,8 @@
const char kPrefsUpdateServerCertificate[] = "update-server-cert";
const char kPrefsUpdateStateNextDataOffset[] = "update-state-next-data-offset";
const char kPrefsUpdateStateNextOperation[] = "update-state-next-operation";
+const char kPrefsUpdateStateOverallProgress[] =
+ "update-state-overall-progress";
const char kPrefsUpdateStateSHA256Context[] = "update-state-sha-256-context";
const char kPrefsUpdateStateSignatureBlob[] = "update-state-signature-blob";
const char kPrefsUpdateStateSignedSHA256Context[] =
diff --git a/constants.h b/constants.h
index e97ec90..a188a51 100644
--- a/constants.h
+++ b/constants.h
@@ -44,6 +44,7 @@
extern const char kPrefsUpdateServerCertificate[];
extern const char kPrefsUpdateStateNextDataOffset[];
extern const char kPrefsUpdateStateNextOperation[];
+extern const char kPrefsUpdateStateOverallProgress[];
extern const char kPrefsUpdateStateSHA256Context[];
extern const char kPrefsUpdateStateSignatureBlob[];
extern const char kPrefsUpdateStateSignedSHA256Context[];
@@ -65,6 +66,10 @@
// The default number of UMA buckets for metrics.
const int kNumDefaultUmaBuckets = 50;
+// If an update has completed at least this many percent, complete it
+// instead of letting a newer update clobber it.
+const int kUpdateClobberPercentage = 30;
+
// General constants
const int kNumBytesInOneMiB = 1024 * 1024;
diff --git a/delta_performer.cc b/delta_performer.cc
index dd9a4c7..64342ad 100644
--- a/delta_performer.cc
+++ b/delta_performer.cc
@@ -1110,8 +1110,30 @@
string interrupted_hash;
TEST_AND_RETURN_FALSE(prefs->GetString(kPrefsUpdateCheckResponseHash,
&interrupted_hash) &&
- !interrupted_hash.empty() &&
- interrupted_hash == update_check_response_hash);
+ !interrupted_hash.empty());
+ if (interrupted_hash != update_check_response_hash) {
+ /* If there is a new update but we're already way into the current
+ * update (say, 30%), just continue the current update. Why?
+ * Because otherwise infrequently used machines with low-bandwidth
+ * connections will never finish updating. This is detailed in bug
+ * 244538, see
+ * https://code.google.com/p/chromium/issues/detail?id=244538
+ */
+ int64_t overall_progress = 0;
+ prefs->GetInt64(kPrefsUpdateStateOverallProgress, &overall_progress);
+ if (overall_progress > kUpdateClobberPercentage) {
+ LOG(INFO) << "Resuming current update (at "
+ << overall_progress
+ << "% completion) "
+ << "despite the fact that there's a newer update.";
+ } else {
+ LOG(INFO) << "Abandoning current update (at "
+ << overall_progress
+ << "% completion) "
+ << "because there is a newer update available.";
+ return false;
+ }
+ }
int64_t resumed_update_failures;
TEST_AND_RETURN_FALSE(!prefs->GetInt64(kPrefsResumedUpdateFailures,
@@ -1148,6 +1170,7 @@
prefs->SetString(kPrefsUpdateStateSignatureBlob, "");
prefs->SetInt64(kPrefsManifestMetadataSize, -1);
prefs->SetInt64(kPrefsResumedUpdateFailures, 0);
+ prefs->SetInt64(kPrefsUpdateStateOverallProgress, 0);
}
return true;
}
@@ -1166,6 +1189,8 @@
}
TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateNextOperation,
next_operation_num_));
+ TEST_AND_RETURN_FALSE(prefs_->SetInt64(kPrefsUpdateStateOverallProgress,
+ overall_progress_));
return true;
}
diff --git a/delta_performer_unittest.cc b/delta_performer_unittest.cc
index 410fceb..aa6880a 100644
--- a/delta_performer_unittest.cc
+++ b/delta_performer_unittest.cc
@@ -575,6 +575,8 @@
state->metadata_size)).WillOnce(Return(true));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextOperation, _))
.WillRepeatedly(Return(true));
+ EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateOverallProgress, _))
+ .WillRepeatedly(Return(true));
EXPECT_CALL(prefs, GetInt64(kPrefsUpdateStateNextOperation, _))
.WillOnce(Return(false));
EXPECT_CALL(prefs, SetInt64(kPrefsUpdateStateNextDataOffset, _))