diff --git a/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h b/fs_mgr/libsnapshot/include/libsnapshot/snapshot_stats.h index 60109a4d1..91dd34f80 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 +#include #include #include @@ -24,21 +25,34 @@ namespace snapshot { 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 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 init_time_; - std::chrono::time_point end_time_; + // Time of the last successful Start() / Resume() call. + std::chrono::time_point start_time_; + bool running_{false}; }; } // namespace snapshot diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index e83fe5a7a..7f9c7f1ff 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -2504,7 +2504,7 @@ UpdateState SnapshotManager::InitiateMergeAndWait(SnapshotMergeReport* stats_rep } } - SnapshotMergeStats merge_stats(*this); + auto merge_stats = SnapshotMergeStats::GetInstance(*this); unsigned int last_progress = 0; auto callback = [&]() -> bool { @@ -2519,9 +2519,9 @@ UpdateState SnapshotManager::InitiateMergeAndWait(SnapshotMergeReport* stats_rep 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; @@ -2532,10 +2532,6 @@ UpdateState SnapshotManager::InitiateMergeAndWait(SnapshotMergeReport* stats_rep 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; @@ -2544,12 +2540,17 @@ UpdateState SnapshotManager::InitiateMergeAndWait(SnapshotMergeReport* stats_rep 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 f4ebae81e..5da7b9873 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 @@ bool SnapshotMergeStats::WriteState() { 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::Finish() { + if (!running_) { + LOG(ERROR) << "SnapshotMergeStats running_ == " << running_; + return nullptr; + } + running_ = false; + + auto result = std::make_unique( + 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 4670eeea0..aa5e9c1fa 100644 --- a/fs_mgr/libsnapshot/snapshotctl.cpp +++ b/fs_mgr/libsnapshot/snapshotctl.cpp @@ -26,9 +26,9 @@ #include #include #include +#include #include -#include "snapshot_stats.h" #include "utility.h" using namespace std::string_literals;