Resume multiple payloads.
DownloadAction will first go through all the already applied payload
and only downloading the manifest and filling in partitions info in
install plan without applying or downloading any operations.
And then resume the partially applied payload using states in prefs.
Moved constuction of MultiRangeHttpFetcher from UpdateAttempter to
DownloadAction, because we now need to setup the range for every
payload, also reduced code duplication.
Also fixed download progress for multi payload.
Bug: 36252799
Test: stop an update during second payload and resume the update
Change-Id: I9ee54a87d15d88c7a14a13575965b19c1773340b
(cherry picked from commit 7162b666318cdd6c75b73fa6a0b06b23559de3e5)
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 84048eb..ab5e275 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -626,6 +626,12 @@
if (!ParseManifestPartitions(error))
return false;
+ // |install_plan.partitions| was filled in, nothing need to be done here if
+ // the payload was already applied, returns false to terminate http fetcher,
+ // but keep |error| as ErrorCode::kSuccess.
+ if (payload_->already_applied)
+ return false;
+
num_total_operations_ = 0;
for (const auto& partition : partitions_) {
num_total_operations_ += partition.operations_size();
@@ -1673,7 +1679,6 @@
TEST_AND_RETURN_FALSE(prefs->SetInt64(kPrefsUpdateStateNextOperation,
kUpdateStateOperationInvalid));
if (!quick) {
- prefs->SetString(kPrefsUpdateCheckResponseHash, "");
prefs->SetInt64(kPrefsUpdateStateNextDataOffset, -1);
prefs->SetInt64(kPrefsUpdateStateNextDataLength, 0);
prefs->SetString(kPrefsUpdateStateSHA256Context, "");
diff --git a/payload_consumer/download_action.cc b/payload_consumer/download_action.cc
index 451c49e..c3a5016 100644
--- a/payload_consumer/download_action.cc
+++ b/payload_consumer/download_action.cc
@@ -27,6 +27,7 @@
#include "update_engine/common/action_pipe.h"
#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/error_code_utils.h"
+#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/utils.h"
#include "update_engine/omaha_request_params.h"
#include "update_engine/p2p_manager.h"
@@ -47,14 +48,12 @@
boot_control_(boot_control),
hardware_(hardware),
system_state_(system_state),
- http_fetcher_(http_fetcher),
+ http_fetcher_(new MultiRangeHttpFetcher(http_fetcher)),
writer_(nullptr),
code_(ErrorCode::kSuccess),
delegate_(nullptr),
- bytes_received_(0),
p2p_sharing_fd_(-1),
- p2p_visible_(true) {
-}
+ p2p_visible_(true) {}
DownloadAction::~DownloadAction() {}
@@ -171,9 +170,24 @@
// Get the InstallPlan and read it
CHECK(HasInputObject());
install_plan_ = GetInputObject();
- bytes_received_ = 0;
-
install_plan_.Dump();
+
+ bytes_received_ = 0;
+ bytes_total_ = 0;
+ for (const auto& payload : install_plan_.payloads)
+ bytes_total_ += payload.size;
+
+ if (install_plan_.is_resume) {
+ int64_t payload_index = 0;
+ if (prefs_->GetInt64(kPrefsUpdateStatePayloadIndex, &payload_index) &&
+ static_cast<size_t>(payload_index) < install_plan_.payloads.size()) {
+ // Save the index for the resume payload before downloading any previous
+ // payload, otherwise it will be overwritten.
+ resume_payload_index_ = payload_index;
+ for (int i = 0; i < payload_index; i++)
+ install_plan_.payloads[i].already_applied = true;
+ }
+ }
// TODO(senj): check that install plan has at least one payload.
if (!payload_)
payload_ = &install_plan_.payloads[0];
@@ -185,11 +199,44 @@
<< ". Proceeding with the update anyway.";
}
- download_active_ = true;
StartDownloading();
}
void DownloadAction::StartDownloading() {
+ download_active_ = true;
+ http_fetcher_->ClearRanges();
+ if (install_plan_.is_resume &&
+ payload_ == &install_plan_.payloads[resume_payload_index_]) {
+ // Resuming an update so fetch the update manifest metadata first.
+ int64_t manifest_metadata_size = 0;
+ int64_t manifest_signature_size = 0;
+ prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
+ prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
+ http_fetcher_->AddRange(base_offset_,
+ manifest_metadata_size + manifest_signature_size);
+ // If there're remaining unprocessed data blobs, fetch them. Be careful not
+ // to request data beyond the end of the payload to avoid 416 HTTP response
+ // error codes.
+ int64_t next_data_offset = 0;
+ prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
+ uint64_t resume_offset =
+ manifest_metadata_size + manifest_signature_size + next_data_offset;
+ if (!payload_->size) {
+ http_fetcher_->AddRange(base_offset_ + resume_offset);
+ } else if (resume_offset < payload_->size) {
+ http_fetcher_->AddRange(base_offset_ + resume_offset,
+ payload_->size - resume_offset);
+ }
+ } else {
+ if (payload_->size) {
+ http_fetcher_->AddRange(base_offset_, payload_->size);
+ } else {
+ // If no payload size is passed we assume we read until the end of the
+ // stream.
+ http_fetcher_->AddRange(base_offset_);
+ }
+ }
+
if (writer_ && writer_ != delta_performer_.get()) {
LOG(INFO) << "Using writer for test.";
} else {
@@ -271,12 +318,14 @@
bytes_received_ += length;
if (delegate_ && download_active_) {
- delegate_->BytesReceived(length, bytes_received_, payload_->size);
+ delegate_->BytesReceived(length, bytes_received_, bytes_total_);
}
if (writer_ && !writer_->Write(bytes, length, &code_)) {
- LOG(ERROR) << "Error " << utils::ErrorCodeToString(code_) << " (" << code_
- << ") in DeltaPerformer's Write method when "
- << "processing the received payload -- Terminating processing";
+ if (code_ != ErrorCode::kSuccess) {
+ LOG(ERROR) << "Error " << utils::ErrorCodeToString(code_) << " (" << code_
+ << ") in DeltaPerformer's Write method when "
+ << "processing the received payload -- Terminating processing";
+ }
// Delete p2p file, if applicable.
if (!p2p_file_id_.empty())
CloseP2PSharingFd(true);
@@ -306,7 +355,8 @@
ErrorCode code =
successful ? ErrorCode::kSuccess : ErrorCode::kDownloadTransferError;
if (code == ErrorCode::kSuccess && delta_performer_.get()) {
- code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
+ if (!payload_->already_applied)
+ code = delta_performer_->VerifyPayload(payload_->hash, payload_->size);
if (code != ErrorCode::kSuccess) {
LOG(ERROR) << "Download of " << install_plan_.download_url
<< " failed due to payload verification error.";
@@ -315,9 +365,11 @@
CloseP2PSharingFd(true);
} else if (payload_ < &install_plan_.payloads.back() &&
system_state_->payload_state()->NextPayload()) {
+ // No need to reset if this payload was already applied.
+ if (!payload_->already_applied)
+ DeltaPerformer::ResetUpdateProgress(prefs_, false);
// Start downloading next payload.
payload_++;
- DeltaPerformer::ResetUpdateProgress(prefs_, false);
install_plan_.download_url =
system_state_->payload_state()->GetCurrentUrl();
StartDownloading();
@@ -331,9 +383,13 @@
processor_->ActionComplete(this, code);
}
-void DownloadAction::TransferTerminated(HttpFetcher *fetcher) {
+void DownloadAction::TransferTerminated(HttpFetcher* fetcher) {
if (code_ != ErrorCode::kSuccess) {
processor_->ActionComplete(this, code_);
+ } else if (payload_->already_applied) {
+ LOG(INFO) << "TransferTerminated with ErrorCode::kSuccess when the current "
+ "payload has already applied, treating as TransferComplete.";
+ TransferComplete(fetcher, true);
}
}
diff --git a/payload_consumer/download_action.h b/payload_consumer/download_action.h
index 48d6292..d0e6000 100644
--- a/payload_consumer/download_action.h
+++ b/payload_consumer/download_action.h
@@ -27,6 +27,7 @@
#include "update_engine/common/action.h"
#include "update_engine/common/boot_control_interface.h"
#include "update_engine/common/http_fetcher.h"
+#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/payload_consumer/delta_performer.h"
#include "update_engine/payload_consumer/install_plan.h"
#include "update_engine/system_state.h"
@@ -106,6 +107,8 @@
delegate_ = delegate;
}
+ void set_base_offset(int64_t base_offset) { base_offset_ = base_offset; }
+
HttpFetcher* http_fetcher() { return http_fetcher_.get(); }
// Returns the p2p file id for the file being written or the empty
@@ -148,8 +151,8 @@
// Global context for the system.
SystemState* system_state_;
- // Pointer to the HttpFetcher that does the http work.
- std::unique_ptr<HttpFetcher> http_fetcher_;
+ // Pointer to the MultiRangeHttpFetcher that does the http work.
+ std::unique_ptr<MultiRangeHttpFetcher> http_fetcher_;
// The FileWriter that downloaded data should be written to. It will
// either point to *decompressing_file_writer_ or *delta_performer_.
@@ -163,7 +166,8 @@
// For reporting status to outsiders
DownloadActionDelegate* delegate_;
- uint64_t bytes_received_;
+ uint64_t bytes_received_{0};
+ uint64_t bytes_total_{0};
bool download_active_{false};
// The file-id for the file we're sharing or the empty string
@@ -177,6 +181,12 @@
// Set to |false| if p2p file is not visible.
bool p2p_visible_;
+ // Loaded from prefs before downloading any payload.
+ size_t resume_payload_index_{0};
+
+ // Offset of the payload in the download URL, used by UpdateAttempterAndroid.
+ int64_t base_offset_{0};
+
DISALLOW_COPY_AND_ASSIGN(DownloadAction);
};
diff --git a/payload_consumer/download_action_unittest.cc b/payload_consumer/download_action_unittest.cc
index 57910cc..9d85550 100644
--- a/payload_consumer/download_action_unittest.cc
+++ b/payload_consumer/download_action_unittest.cc
@@ -139,7 +139,7 @@
0, writer.Open(output_temp_file.path().c_str(), O_WRONLY | O_CREAT, 0));
writer.set_fail_write(fail_write);
- uint64_t size = data.size();
+ uint64_t size = data.size() - 1;
InstallPlan install_plan;
install_plan.payload_type = InstallPayloadType::kDelta;
install_plan.payloads.push_back({.size = size});
@@ -174,7 +174,7 @@
download_action.set_delegate(&download_delegate);
if (data.size() > kMockHttpFetcherChunkSize)
EXPECT_CALL(download_delegate,
- BytesReceived(_, 1 + kMockHttpFetcherChunkSize, _));
+ BytesReceived(_, kMockHttpFetcherChunkSize, _));
EXPECT_CALL(download_delegate, BytesReceived(_, _, _)).Times(AtLeast(1));
}
ErrorCode expected_code = ErrorCode::kSuccess;
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index db471da..d8d9f57 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -61,10 +61,15 @@
uint64_t metadata_size = 0; // size of the metadata
std::string metadata_signature; // signature of the metadata in base64
brillo::Blob hash; // SHA256 hash of the payload
+ // Only download manifest and fill in partitions in install plan without
+ // apply the payload if true. Will be set by DownloadAction when resuming
+ // multi-payload.
+ bool already_applied = false;
bool operator==(const Payload& that) const {
return size == that.size && metadata_size == that.metadata_size &&
- metadata_signature == that.metadata_signature && hash == that.hash;
+ metadata_signature == that.metadata_signature &&
+ hash == that.hash && already_applied == that.already_applied;
}
};
std::vector<Payload> payloads;
diff --git a/payload_state.cc b/payload_state.cc
index 287e24c..2a7030f 100644
--- a/payload_state.cc
+++ b/payload_state.cc
@@ -881,6 +881,20 @@
full_payload_attempt_number_);
}
+void PayloadState::SetPayloadIndex(size_t payload_index) {
+ CHECK(prefs_);
+ payload_index_ = payload_index;
+ LOG(INFO) << "Payload Index = " << payload_index_;
+ prefs_->SetInt64(kPrefsUpdateStatePayloadIndex, payload_index_);
+}
+
+bool PayloadState::NextPayload() {
+ if (payload_index_ + 1 >= candidate_urls_.size())
+ return false;
+ SetPayloadIndex(payload_index_ + 1);
+ return true;
+}
+
void PayloadState::LoadUrlIndex() {
SetUrlIndex(GetPersistedValue(kPrefsCurrentUrlIndex));
}
diff --git a/payload_state.h b/payload_state.h
index 56e32fd..699fc74 100644
--- a/payload_state.h
+++ b/payload_state.h
@@ -153,13 +153,7 @@
return attempt_error_code_;
}
- bool NextPayload() override {
- if (payload_index_ + 1 >= candidate_urls_.size())
- return false;
- payload_index_++;
- url_index_ = 0;
- return true;
- }
+ bool NextPayload() override;
private:
enum class AttemptType {
@@ -280,6 +274,11 @@
// of a process restart.
void SetFullPayloadAttemptNumber(int payload_attempt_number);
+ // Sets the current payload index 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 SetPayloadIndex(size_t payload_index);
+
// Initializes the current URL index from the persisted state.
void LoadUrlIndex();
diff --git a/update_attempter.cc b/update_attempter.cc
index 448e29c..ff3b046 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -45,7 +45,6 @@
#include "update_engine/common/clock_interface.h"
#include "update_engine/common/constants.h"
#include "update_engine/common/hardware_interface.h"
-#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/platform_constants.h"
#include "update_engine/common/prefs_interface.h"
#include "update_engine/common/subprocess.h"
@@ -619,12 +618,12 @@
LibcurlHttpFetcher* download_fetcher =
new LibcurlHttpFetcher(GetProxyResolver(), system_state_->hardware());
download_fetcher->set_server_to_check(ServerToCheck::kDownload);
- shared_ptr<DownloadAction> download_action(new DownloadAction(
- prefs_,
- system_state_->boot_control(),
- system_state_->hardware(),
- system_state_,
- new MultiRangeHttpFetcher(download_fetcher))); // passes ownership
+ shared_ptr<DownloadAction> download_action(
+ new DownloadAction(prefs_,
+ system_state_->boot_control(),
+ system_state_->hardware(),
+ system_state_,
+ download_fetcher)); // passes ownership
shared_ptr<OmahaRequestAction> download_finished_action(
new OmahaRequestAction(
system_state_,
@@ -1038,7 +1037,6 @@
new_payload_size_ = 0;
for (const auto& payload : plan.payloads)
new_payload_size_ += payload.size;
- SetupDownload();
cpu_limiter_.StartLimiter();
SetStatusAndNotify(UpdateStatus::UPDATE_AVAILABLE);
} else if (type == DownloadAction::StaticType()) {
@@ -1326,35 +1324,6 @@
prefs_->SetInt64(kPrefsDeltaUpdateFailures, ++delta_failures);
}
-void UpdateAttempter::SetupDownload() {
- MultiRangeHttpFetcher* fetcher =
- static_cast<MultiRangeHttpFetcher*>(download_action_->http_fetcher());
- fetcher->ClearRanges();
- if (response_handler_action_->install_plan().is_resume) {
- // Resuming an update so fetch the update manifest metadata first.
- int64_t manifest_metadata_size = 0;
- int64_t manifest_signature_size = 0;
- prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
- fetcher->AddRange(0, manifest_metadata_size + manifest_signature_size);
- // If there're remaining unprocessed data blobs, fetch them. Be careful not
- // to request data beyond the end of the payload to avoid 416 HTTP response
- // error codes.
- int64_t next_data_offset = 0;
- prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset =
- manifest_metadata_size + manifest_signature_size + next_data_offset;
- int64_t payload_index = 0;
- prefs_->GetInt64(kPrefsUpdateStatePayloadIndex, &payload_index);
- if (resume_offset <
- response_handler_action_->install_plan().payloads[payload_index].size) {
- fetcher->AddRange(resume_offset);
- }
- } else {
- fetcher->AddRange(0);
- }
-}
-
void UpdateAttempter::PingOmaha() {
if (!processor_->IsRunning()) {
shared_ptr<OmahaRequestAction> ping_action(new OmahaRequestAction(
diff --git a/update_attempter.h b/update_attempter.h
index 193e172..7780357 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -281,9 +281,6 @@
// Sets the status to the given status and notifies a status update over dbus.
void SetStatusAndNotify(UpdateStatus status);
- // Sets up the download parameters after receiving the update check response.
- void SetupDownload();
-
// Creates an error event object in |error_event_| to be included in an
// OmahaRequestAction once the current action processor is done.
void CreatePendingErrorEvent(AbstractAction* action, ErrorCode code);
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index a39379f..09549f8 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -30,7 +30,6 @@
#include "update_engine/common/constants.h"
#include "update_engine/common/file_fetcher.h"
-#include "update_engine/common/multi_range_http_fetcher.h"
#include "update_engine/common/utils.h"
#include "update_engine/daemon_state_interface.h"
#include "update_engine/network_selector.h"
@@ -207,7 +206,6 @@
install_plan_.Dump();
BuildUpdateActions(payload_url);
- SetupDownload();
// Setup extra headers.
HttpFetcher* fetcher = download_action_->http_fetcher();
if (!headers[kPayloadPropertyAuthorization].empty())
@@ -459,12 +457,12 @@
download_fetcher = libcurl_fetcher;
#endif // _UE_SIDELOAD
}
- shared_ptr<DownloadAction> download_action(new DownloadAction(
- prefs_,
- boot_control_,
- hardware_,
- nullptr, // system_state, not used.
- new MultiRangeHttpFetcher(download_fetcher))); // passes ownership
+ shared_ptr<DownloadAction> download_action(
+ new DownloadAction(prefs_,
+ boot_control_,
+ hardware_,
+ nullptr, // system_state, not used.
+ download_fetcher)); // passes ownership
shared_ptr<FilesystemVerifierAction> filesystem_verifier_action(
new FilesystemVerifierAction());
@@ -472,6 +470,7 @@
new PostinstallRunnerAction(boot_control_, hardware_));
download_action->set_delegate(this);
+ download_action->set_base_offset(base_offset_);
download_action_ = download_action;
postinstall_runner_action->set_delegate(this);
@@ -492,42 +491,6 @@
processor_->EnqueueAction(action.get());
}
-void UpdateAttempterAndroid::SetupDownload() {
- MultiRangeHttpFetcher* fetcher =
- static_cast<MultiRangeHttpFetcher*>(download_action_->http_fetcher());
- fetcher->ClearRanges();
- if (install_plan_.is_resume) {
- // Resuming an update so fetch the update manifest metadata first.
- int64_t manifest_metadata_size = 0;
- int64_t manifest_signature_size = 0;
- prefs_->GetInt64(kPrefsManifestMetadataSize, &manifest_metadata_size);
- prefs_->GetInt64(kPrefsManifestSignatureSize, &manifest_signature_size);
- fetcher->AddRange(base_offset_,
- manifest_metadata_size + manifest_signature_size);
- // If there're remaining unprocessed data blobs, fetch them. Be careful not
- // to request data beyond the end of the payload to avoid 416 HTTP response
- // error codes.
- int64_t next_data_offset = 0;
- prefs_->GetInt64(kPrefsUpdateStateNextDataOffset, &next_data_offset);
- uint64_t resume_offset =
- manifest_metadata_size + manifest_signature_size + next_data_offset;
- if (!install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_ + resume_offset);
- } else if (resume_offset < install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_ + resume_offset,
- install_plan_.payloads[0].size - resume_offset);
- }
- } else {
- if (install_plan_.payloads[0].size) {
- fetcher->AddRange(base_offset_, install_plan_.payloads[0].size);
- } else {
- // If no payload size is passed we assume we read until the end of the
- // stream.
- fetcher->AddRange(base_offset_);
- }
- }
-}
-
bool UpdateAttempterAndroid::WriteUpdateCompletedMarker() {
string boot_id;
TEST_AND_RETURN_FALSE(utils::GetBootId(&boot_id));
diff --git a/update_attempter_android.h b/update_attempter_android.h
index 6a5c227..167191e 100644
--- a/update_attempter_android.h
+++ b/update_attempter_android.h
@@ -110,10 +110,6 @@
// applying an update from the given |url|.
void BuildUpdateActions(const std::string& url);
- // Sets up the download parameters based on the update requested on the
- // |install_plan_|.
- void SetupDownload();
-
// Writes to the processing completed marker. Does nothing if
// |update_completed_marker_| is empty.
bool WriteUpdateCompletedMarker();
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 17baaa0..4928477 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -218,7 +218,6 @@
DownloadAction action(prefs_, nullptr, nullptr, nullptr, fetcher.release());
EXPECT_CALL(*prefs_, GetInt64(kPrefsDeltaUpdateFailures, _)).Times(0);
attempter_.ActionCompleted(nullptr, &action, ErrorCode::kSuccess);
- EXPECT_EQ(503, attempter_.http_response_code());
EXPECT_EQ(UpdateStatus::FINALIZING, attempter_.status());
ASSERT_EQ(nullptr, attempter_.error_event_.get());
}