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

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 @@ 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<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

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;
 }

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::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

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;