libsnapshot: SnapshotMergeStats::Start/Finish
Change API for SnapshotMergeStats so that it is easier
for update_engine to use.
Test: apply OTA and look at stats
Bug: 147696014
Bug: 138817833
Change-Id: Ie4036ac7382102c00f0761f443d78e00b9e585d5
Merged-In: Ie4036ac7382102c00f0761f443d78e00b9e585d5
diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h
index 60109a4..91dd34f 100644
--- a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h
+++ b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h
@@ -15,6 +15,7 @@
#pragma once
#include <chrono>
+#include <memory>
#include <android/snapshot/snapshot.pb.h>
#include <libsnapshot/snapshot.h>
@@ -24,21 +25,34 @@
class SnapshotMergeStats {
public:
- SnapshotMergeStats(SnapshotManager& parent);
- ~SnapshotMergeStats();
- void Start();
- void Resume();
+ // Not thread safe.
+ static SnapshotMergeStats* GetInstance(SnapshotManager& manager);
+
+ // Called when merge starts or resumes.
+ bool Start();
void set_state(android::snapshot::UpdateState state);
- SnapshotMergeReport GetReport();
+
+ // Called when merge ends. Properly clean up permanent storage.
+ class Result {
+ public:
+ virtual ~Result() {}
+ virtual const SnapshotMergeReport& report() const = 0;
+ // Time between successful Start() / Resume() to Finish().
+ virtual std::chrono::steady_clock::duration merge_time() const = 0;
+ };
+ std::unique_ptr<Result> Finish();
private:
bool ReadState();
bool WriteState();
+ bool DeleteState();
+ SnapshotMergeStats(const std::string& path);
- const SnapshotManager& parent_;
+ std::string path_;
SnapshotMergeReport report_;
- std::chrono::time_point<std::chrono::steady_clock> init_time_;
- std::chrono::time_point<std::chrono::steady_clock> end_time_;
+ // Time of the last successful Start() / Resume() call.
+ std::chrono::time_point<std::chrono::steady_clock> start_time_;
+ bool running_{false};
};
} // namespace snapshot
diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 896857f..61fc2df 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -2501,7 +2501,7 @@
}
}
- SnapshotMergeStats merge_stats(*this);
+ auto merge_stats = SnapshotMergeStats::GetInstance(*this);
unsigned int last_progress = 0;
auto callback = [&]() -> bool {
@@ -2516,9 +2516,9 @@
LOG(INFO) << "Waiting for any previous merge request to complete. "
<< "This can take up to several minutes.";
- merge_stats.Resume();
+ merge_stats->Start();
auto state = ProcessUpdateState(callback, before_cancel);
- merge_stats.set_state(state);
+ merge_stats->set_state(state);
if (state == UpdateState::None) {
LOG(INFO) << "Can't find any snapshot to merge.";
return state;
@@ -2529,10 +2529,6 @@
return state;
}
- // This is the first snapshot merge that is requested after OTA. We can
- // initialize the merge duration statistics.
- merge_stats.Start();
-
if (!InitiateMerge()) {
LOG(ERROR) << "Failed to initiate merge.";
return state;
@@ -2541,12 +2537,17 @@
LOG(INFO) << "Waiting for merge to complete. This can take up to several minutes.";
last_progress = 0;
state = ProcessUpdateState(callback, before_cancel);
- merge_stats.set_state(state);
+ merge_stats->set_state(state);
}
LOG(INFO) << "Merge finished with state \"" << state << "\".";
if (stats_report) {
- *stats_report = merge_stats.GetReport();
+ auto result = merge_stats->Finish();
+ if (result) {
+ *stats_report = result->report();
+ } else {
+ LOG(WARNING) << "SnapshotMergeStatus::Finish failed.";
+ }
}
return state;
}
diff --git a/fs_mgr/libsnapshot/snapshot_stats.cpp b/fs_mgr/libsnapshot/snapshot_stats.cpp
index f4ebae8..5da7b98 100644
--- a/fs_mgr/libsnapshot/snapshot_stats.cpp
+++ b/fs_mgr/libsnapshot/snapshot_stats.cpp
@@ -23,22 +23,17 @@
namespace android {
namespace snapshot {
-SnapshotMergeStats::SnapshotMergeStats(SnapshotManager& parent) : parent_(parent) {
- init_time_ = std::chrono::steady_clock::now();
+SnapshotMergeStats* SnapshotMergeStats::GetInstance(SnapshotManager& parent) {
+ static SnapshotMergeStats g_instance(parent.GetMergeStateFilePath());
+ CHECK(g_instance.path_ == parent.GetMergeStateFilePath());
+ return &g_instance;
}
-SnapshotMergeStats::~SnapshotMergeStats() {
- std::string error;
- auto file_path = parent_.GetMergeStateFilePath();
- if (!android::base::RemoveFileIfExists(file_path, &error)) {
- LOG(ERROR) << "Failed to remove merge statistics file " << file_path << ": " << error;
- return;
- }
-}
+SnapshotMergeStats::SnapshotMergeStats(const std::string& path) : path_(path), running_(false) {}
bool SnapshotMergeStats::ReadState() {
std::string contents;
- if (!android::base::ReadFileToString(parent_.GetMergeStateFilePath(), &contents)) {
+ if (!android::base::ReadFileToString(path_, &contents)) {
PLOG(INFO) << "Read merge statistics file failed";
return false;
}
@@ -55,34 +50,73 @@
LOG(ERROR) << "Unable to serialize SnapshotMergeStats.";
return false;
}
- auto file_path = parent_.GetMergeStateFilePath();
- if (!WriteStringToFileAtomic(contents, file_path)) {
+ if (!WriteStringToFileAtomic(contents, path_)) {
PLOG(ERROR) << "Could not write to merge statistics file";
return false;
}
return true;
}
-void SnapshotMergeStats::Start() {
- report_.set_resume_count(0);
- report_.set_state(UpdateState::None);
- WriteState();
+bool SnapshotMergeStats::DeleteState() {
+ std::string error;
+ if (!android::base::RemoveFileIfExists(path_, &error)) {
+ LOG(ERROR) << "Failed to remove merge statistics file " << path_ << ": " << error;
+ return false;
+ }
+ return true;
}
-void SnapshotMergeStats::Resume() {
- if (!ReadState()) {
- return;
+bool SnapshotMergeStats::Start() {
+ if (running_) {
+ LOG(ERROR) << "SnapshotMergeStats running_ == " << running_;
+ return false;
}
- report_.set_resume_count(report_.resume_count() + 1);
- WriteState();
+ running_ = true;
+
+ start_time_ = std::chrono::steady_clock::now();
+ if (ReadState()) {
+ report_.set_resume_count(report_.resume_count() + 1);
+ } else {
+ report_.set_resume_count(0);
+ report_.set_state(UpdateState::None);
+ }
+
+ return WriteState();
}
void SnapshotMergeStats::set_state(android::snapshot::UpdateState state) {
report_.set_state(state);
}
-SnapshotMergeReport SnapshotMergeStats::GetReport() {
- return report_;
+class SnapshotMergeStatsResultImpl : public SnapshotMergeStats::Result {
+ public:
+ SnapshotMergeStatsResultImpl(const SnapshotMergeReport& report,
+ std::chrono::steady_clock::duration merge_time)
+ : report_(report), merge_time_(merge_time) {}
+ const SnapshotMergeReport& report() const override { return report_; }
+ std::chrono::steady_clock::duration merge_time() const override { return merge_time_; }
+
+ private:
+ SnapshotMergeReport report_;
+ std::chrono::steady_clock::duration merge_time_;
+};
+
+std::unique_ptr<SnapshotMergeStats::Result> SnapshotMergeStats::Finish() {
+ if (!running_) {
+ LOG(ERROR) << "SnapshotMergeStats running_ == " << running_;
+ return nullptr;
+ }
+ running_ = false;
+
+ auto result = std::make_unique<SnapshotMergeStatsResultImpl>(
+ report_, std::chrono::steady_clock::now() - start_time_);
+
+ // We still want to report result if state is not deleted. Just leave
+ // it there and move on. A side effect is that it may be reported over and
+ // over again in the future, but there is nothing we can do.
+ (void)DeleteState();
+
+ return result;
}
} // namespace snapshot
diff --git a/fs_mgr/libsnapshot/snapshotctl.cpp b/fs_mgr/libsnapshot/snapshotctl.cpp
index 4670eee..aa5e9c1 100644
--- a/fs_mgr/libsnapshot/snapshotctl.cpp
+++ b/fs_mgr/libsnapshot/snapshotctl.cpp
@@ -26,9 +26,9 @@
#include <android-base/unique_fd.h>
#include <android/snapshot/snapshot.pb.h>
#include <libsnapshot/snapshot.h>
+#include <libsnapshot/snapshot_stats.h>
#include <statslog.h>
-#include "snapshot_stats.h"
#include "utility.h"
using namespace std::string_literals;