Throttle metric writing by 3 seconds
After aosp/2320000 , pref writes are atomic but slower. Since
update_engine frequently writes metrics such as bytes downloaded to
disk, update time is negatively impacted. Throttle metric writing to
mitigate.
Bug: 259174530
Test: th
Change-Id: I09be219fa08d5569a5cb5b808e07da3d71a8131c
diff --git a/aosp/update_attempter_android.cc b/aosp/update_attempter_android.cc
index 23d3ea7..eda82e2 100644
--- a/aosp/update_attempter_android.cc
+++ b/aosp/update_attempter_android.cc
@@ -144,7 +144,9 @@
hardware_(hardware),
apex_handler_android_(std::move(apex_handler)),
processor_(new ActionProcessor()),
- clock_(new Clock()) {
+ clock_(new Clock()),
+ metric_bytes_downloaded_(kPrefsCurrentBytesDownloaded, prefs_),
+ metric_total_bytes_downloaded_(kPrefsTotalBytesDownloaded, prefs_) {
metrics_reporter_ = metrics::CreateMetricsReporter(
boot_control_->GetDynamicPartitionControl(), &install_plan_);
network_selector_ = network::CreateNetworkSelector();
@@ -570,6 +572,8 @@
void UpdateAttempterAndroid::ProcessingDone(const ActionProcessor* processor,
ErrorCode code) {
LOG(INFO) << "Processing Done.";
+ metric_bytes_downloaded_.Flush(true);
+ metric_total_bytes_downloaded_.Flush(true);
last_error_ = code;
if (status_ == UpdateStatus::CLEANUP_PREVIOUS_UPDATE) {
TerminateUpdateAndNotify(code);
@@ -667,14 +671,8 @@
}
// Update the bytes downloaded in prefs.
- int64_t current_bytes_downloaded =
- metrics_utils::GetPersistedValue(kPrefsCurrentBytesDownloaded, prefs_);
- int64_t total_bytes_downloaded =
- metrics_utils::GetPersistedValue(kPrefsTotalBytesDownloaded, prefs_);
- prefs_->SetInt64(kPrefsCurrentBytesDownloaded,
- current_bytes_downloaded + bytes_progressed);
- prefs_->SetInt64(kPrefsTotalBytesDownloaded,
- total_bytes_downloaded + bytes_progressed);
+ metric_bytes_downloaded_ += bytes_progressed;
+ metric_total_bytes_downloaded_ += bytes_progressed;
}
bool UpdateAttempterAndroid::ShouldCancel(ErrorCode* cancel_reason) {
@@ -754,7 +752,7 @@
prefs_->Delete(kPrefsPayloadAttemptNumber);
metrics_utils::SetSystemUpdatedMarker(clock_.get(), prefs_);
// Clear the total bytes downloaded if and only if the update succeeds.
- prefs_->SetInt64(kPrefsTotalBytesDownloaded, 0);
+ metric_total_bytes_downloaded_.Delete();
}
}
@@ -882,8 +880,7 @@
attempt_result,
error_code);
- int64_t current_bytes_downloaded =
- metrics_utils::GetPersistedValue(kPrefsCurrentBytesDownloaded, prefs_);
+ int64_t current_bytes_downloaded = metric_bytes_downloaded_.get();
metrics_reporter_->ReportUpdateAttemptDownloadMetrics(
current_bytes_downloaded,
0,
@@ -900,8 +897,7 @@
// For android metrics, we only care about the total bytes downloaded
// for all sources; for now we assume the only download source is
// HttpsServer.
- int64_t total_bytes_downloaded =
- metrics_utils::GetPersistedValue(kPrefsTotalBytesDownloaded, prefs_);
+ int64_t total_bytes_downloaded = metric_total_bytes_downloaded_.get();
int64_t num_bytes_downloaded[kNumDownloadSources] = {};
num_bytes_downloaded[DownloadSource::kDownloadSourceHttpsServer] =
total_bytes_downloaded;
@@ -1064,7 +1060,7 @@
void UpdateAttempterAndroid::ClearMetricsPrefs() {
CHECK(prefs_);
- prefs_->Delete(kPrefsCurrentBytesDownloaded);
+ metric_bytes_downloaded_.Delete();
prefs_->Delete(kPrefsNumReboots);
prefs_->Delete(kPrefsSystemUpdatedMarker);
prefs_->Delete(kPrefsUpdateTimestampStart);
diff --git a/aosp/update_attempter_android.h b/aosp/update_attempter_android.h
index f3ed604..3174904 100644
--- a/aosp/update_attempter_android.h
+++ b/aosp/update_attempter_android.h
@@ -39,6 +39,7 @@
#include "update_engine/common/metrics_reporter_interface.h"
#include "update_engine/common/network_selector_interface.h"
#include "update_engine/common/prefs_interface.h"
+#include "update_engine/metrics_utils.h"
#include "update_engine/payload_consumer/filesystem_verifier_action.h"
#include "update_engine/payload_consumer/postinstall_runner_action.h"
@@ -279,6 +280,9 @@
std::string update_certificates_path_{constants::kUpdateCertificatesPath};
ErrorCode last_error_{ErrorCode::kSuccess};
+ metrics_utils::PersistedValue<int64_t> metric_bytes_downloaded_;
+ metrics_utils::PersistedValue<int64_t> metric_total_bytes_downloaded_;
+
DISALLOW_COPY_AND_ASSIGN(UpdateAttempterAndroid);
};
diff --git a/common/metrics_constants.h b/common/metrics_constants.h
index b7633b9..af40300 100644
--- a/common/metrics_constants.h
+++ b/common/metrics_constants.h
@@ -17,6 +17,7 @@
#ifndef UPDATE_ENGINE_COMMON_METRICS_CONSTANTS_H_
#define UPDATE_ENGINE_COMMON_METRICS_CONSTANTS_H_
+#include <chrono>
namespace chromeos_update_engine {
namespace metrics {
@@ -140,6 +141,8 @@
kNumConstants
};
+constexpr auto kMetricFlushInterval = std::chrono::seconds(3);
+
} // namespace metrics
} // namespace chromeos_update_engine
diff --git a/common/utils.cc b/common/utils.cc
index 6b8bc54..4c1365a 100644
--- a/common/utils.cc
+++ b/common/utils.cc
@@ -56,8 +56,8 @@
#include <brillo/data_encoding.h>
#include "update_engine/common/constants.h"
-#include "update_engine/common/platform_constants.h"
#include "update_engine/common/subprocess.h"
+#include "update_engine/common/platform_constants.h"
#include "update_engine/payload_consumer/file_descriptor.h"
using base::Time;
diff --git a/metrics_utils.h b/metrics_utils.h
index 16e9eec..2f79140 100644
--- a/metrics_utils.h
+++ b/metrics_utils.h
@@ -17,7 +17,11 @@
#ifndef UPDATE_ENGINE_METRICS_UTILS_H_
#define UPDATE_ENGINE_METRICS_UTILS_H_
+#include <chrono>
#include <string>
+#include <string_view>
+#include <type_traits>
+#include <utility>
#include <base/time/time.h>
@@ -81,6 +85,69 @@
PrefsInterface* prefs,
ClockInterface* clock);
+template <typename T>
+class PersistedValue {
+ public:
+ PersistedValue(std::string_view key, PrefsInterface* prefs)
+ : key_(key), prefs_(prefs) {
+ val_ = metrics_utils::GetPersistedValue(key, prefs);
+ }
+ ~PersistedValue() { Flush(true); }
+ void Delete() {
+ val_ = {};
+ prefs_->Delete(key_);
+ }
+ T get() const { return val_; }
+ using clock = std::chrono::system_clock;
+ using time_point = clock::time_point;
+ // prefix increment
+ PersistedValue<T>& operator++() {
+ ++val_;
+ Flush();
+ return *this;
+ }
+ PersistedValue<T>& operator--() {
+ --val_;
+ Flush();
+ return *this;
+ }
+ PersistedValue<T>& operator+=(T&& t) {
+ val_ += std::forward<T>(t);
+ Flush();
+ return *this;
+ }
+ PersistedValue<T>& operator-=(T&& t) {
+ val_ -= std::forward<T>(t);
+ Flush();
+ return *this;
+ }
+ PersistedValue<T>& operator=(T&& t) {
+ val_ = std::forward<T>(t);
+ Flush();
+ return *this;
+ }
+ void Flush(bool force = false) {
+ auto now = clock::now();
+ if (now - last_save_ > metrics::kMetricFlushInterval || force) {
+ last_save_ = now;
+ if (std::is_integral_v<T>) {
+ prefs_->SetInt64(key_, val_);
+ } else if (std::is_same_v<T, bool>) {
+ prefs_->SetBoolean(key_, val_);
+ } else {
+ auto value = std::to_string(val_);
+ prefs_->SetString(key_, value);
+ }
+ }
+ }
+
+ private:
+ const std::string_view key_;
+ PrefsInterface* prefs_;
+ T val_;
+ time_point last_save_{};
+};
+
} // namespace metrics_utils
} // namespace chromeos_update_engine