[pkg-resolver] Cache TUF metadata with non-auto repos
Previously, we would try to update TUF every time before we try to
resolve a package. This can generate a significant amount of load during
an OTA, since we would update for each package. This patch fixes this by
allowing the TUF metadata to be cached for 5 minutes, which should
significantly reduce the load on the server.
Note: This differs from the original CL since fuchsia-pkg-testing's
serve::handler was renamed to serve::responder, and that we needed to
backport serve::handler::Record for the tests to pass.
Fixed: 77442
Change-Id: I70b5a5682a56dd706f26b6536bed13565f6418bd
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/538013
Commit-Queue: Erick Tryzelaar <etryzelaar@google.com>
Fuchsia-Auto-Submit: Erick Tryzelaar <etryzelaar@google.com>
Reviewed-by: Dan Johnson <computerdruid@google.com>
(cherry picked from commit c8a2a422f7684f810af88217fa74ff2d374fdd64)
diff --git a/src/sys/pkg/bin/pkg-resolver/src/repository.rs b/src/sys/pkg/bin/pkg-resolver/src/repository.rs
index 84afc8f..bd71114 100644
--- a/src/sys/pkg/bin/pkg-resolver/src/repository.rs
+++ b/src/sys/pkg/bin/pkg-resolver/src/repository.rs
@@ -300,7 +300,7 @@
http_sse::Event,
matches::assert_matches,
std::sync::Arc,
- updating_tuf_client::SUBSCRIBE_CACHE_STALE_TIMEOUT,
+ updating_tuf_client::METADATA_CACHE_STALE_TIMEOUT,
};
const TEST_REPO_URL: &str = "fuchsia-pkg://test";
@@ -558,7 +558,7 @@
.unwrap();
// cache will now be stale, should trigger an update
- clock::mock::set(initial_time + SUBSCRIBE_CACHE_STALE_TIMEOUT);
+ clock::mock::set(initial_time + METADATA_CACHE_STALE_TIMEOUT);
repo.get_merkle_at_path(&TargetPath::new("just-meta-far/0".to_string()).unwrap())
.await
.unwrap();
@@ -613,6 +613,60 @@
assert!(dir.join("1.root.json").exists());
}
+
+ #[fasync::run_singlethreaded(test)]
+ async fn resolve_caches_metadata() {
+ clock::mock::set(zx::Time::from_nanos(0));
+
+ let pkg = PackageBuilder::new("just-meta-far").build().await.expect("created pkg");
+ let env = TestEnv::builder().add_package(&pkg).build().await;
+
+ let target_path =
+ TargetPath::new("just-meta-far/0".to_string()).expect("created target path");
+
+ let (responder, history) = handler::Record::new();
+ let (_served_repository, repo_config) =
+ env.serve_repo()
+ .uri_path_override_handler(responder)
+ .start(TEST_REPO_URL);
+ let mut repo = env.repo(&repo_config).await.expect("created opened repo");
+
+ // Resolve the package, which should succeed.
+ assert_matches!(repo.get_merkle_at_path(&target_path).await, Ok(_));
+
+ let entries = history.take();
+ let uri_paths = entries.iter().map(|e| e.uri_path().to_str().unwrap()).collect::<Vec<_>>();
+ assert_eq!(
+ uri_paths,
+ vec![
+ "/1.root.json",
+ "/2.root.json",
+ "/timestamp.json",
+ "/2.snapshot.json",
+ "/2.targets.json",
+ ],
+ );
+
+ // Advance time right before the timeout, and make sure we don't access the server.
+ clock::mock::set(
+ zx::Time::from_nanos(0) + METADATA_CACHE_STALE_TIMEOUT - zx::Duration::from_seconds(1),
+ );
+ assert_matches!(repo.get_merkle_at_path(&target_path).await, Ok(_));
+
+ let entries = history.take();
+ assert!(entries.is_empty(), "{:#?}", entries);
+
+ // Advance time right after the timeout, and make sure we access the server.
+ clock::mock::set(
+ zx::Time::from_nanos(0) + METADATA_CACHE_STALE_TIMEOUT + zx::Duration::from_seconds(1),
+ );
+ assert_matches!(repo.get_merkle_at_path(&target_path).await, Ok(_));
+
+ let entries = history.take();
+ assert_eq!(entries.len(), 2, "{:#?}", entries);
+ assert_eq!(entries[0].uri_path(), Path::new("/2.root.json"), "{:#?}", entries);
+ assert_eq!(entries[1].uri_path(), Path::new("/timestamp.json"), "{:#?}", entries);
+ }
}
#[cfg(test)]
diff --git a/src/sys/pkg/bin/pkg-resolver/src/repository/updating_tuf_client.rs b/src/sys/pkg/bin/pkg-resolver/src/repository/updating_tuf_client.rs
index 64683af..4846d5e 100644
--- a/src/sys/pkg/bin/pkg-resolver/src/repository/updating_tuf_client.rs
+++ b/src/sys/pkg/bin/pkg-resolver/src/repository/updating_tuf_client.rs
@@ -44,7 +44,7 @@
last_update_successfully_checked_time: InspectableDebugString<Option<zx::Time>>,
/// `Some` if there is an AutoClient task, dropping it stops the task.
- auto_client_aborter: Option<AbortHandleOnDrop>,
+ _auto_client_aborter: Option<AbortHandleOnDrop>,
tuf_metadata_timeout: Duration,
@@ -144,7 +144,7 @@
&node,
"last_update_successfully_checked_time",
),
- auto_client_aborter,
+ _auto_client_aborter: auto_client_aborter,
tuf_metadata_timeout,
inspect: UpdatingTufClientInspectState {
update_check_failure_count: inspect_util::Counter::new(
@@ -222,11 +222,8 @@
}
fn is_stale(&self) -> bool {
- if self.auto_client_aborter.is_none() {
- return true;
- }
if let Some(last_update_time) = *self.last_update_successfully_checked_time {
- last_update_time + SUBSCRIBE_CACHE_STALE_TIMEOUT <= clock::now()
+ last_update_time + METADATA_CACHE_STALE_TIMEOUT <= clock::now()
} else {
true
}
@@ -273,7 +270,7 @@
}
}
-pub const SUBSCRIBE_CACHE_STALE_TIMEOUT: zx::Duration = zx::Duration::from_minutes(5);
+pub const METADATA_CACHE_STALE_TIMEOUT: zx::Duration = zx::Duration::from_minutes(5);
struct AutoClient {
updating_client: Weak<AsyncMutex<UpdatingTufClient>>,
diff --git a/src/sys/pkg/lib/fuchsia-pkg-testing/src/serve/handler.rs b/src/sys/pkg/lib/fuchsia-pkg-testing/src/serve/handler.rs
index 96a6aa61..50b45b6 100644
--- a/src/sys/pkg/lib/fuchsia-pkg-testing/src/serve/handler.rs
+++ b/src/sys/pkg/lib/fuchsia-pkg-testing/src/serve/handler.rs
@@ -760,3 +760,52 @@
ready(response).boxed()
}
}
+
+/// Information saved by Record for each request it handles.
+#[derive(Debug)]
+pub struct HistoryEntry {
+ uri_path: PathBuf,
+}
+
+impl HistoryEntry {
+ /// The uri_path of the request.
+ pub fn uri_path(&self) -> &Path {
+ &self.uri_path
+ }
+}
+
+/// The request history recorded by Record.
+pub struct History(Arc<Mutex<Vec<HistoryEntry>>>);
+
+impl History {
+ /// Take the recorded history, clearing it from the Record.
+ pub fn take(&self) -> Vec<HistoryEntry> {
+ std::mem::replace(&mut self.0.lock(), vec![])
+ }
+}
+
+/// Responder that records the requests.
+pub struct Record {
+ history: History,
+}
+
+impl Record {
+ /// Creates a responder that records all the requests.
+ pub fn new() -> (Self, History) {
+ let history = Arc::new(Mutex::new(vec![]));
+ (Self { history: History(Arc::clone(&history)) }, History(history))
+ }
+}
+
+impl UriPathHandler for Record {
+ fn handle<'a>(
+ &'a self,
+ uri_path: &Path,
+ response: Response<Body>,
+ ) -> BoxFuture<'_, Response<Body>> {
+ self.history.0.lock().push(HistoryEntry {
+ uri_path: uri_path.to_owned(),
+ });
+ ready(response).boxed()
+ }
+}
diff --git a/src/sys/pkg/tests/pkg-resolver/src/metrics.rs b/src/sys/pkg/tests/pkg-resolver/src/metrics.rs
index 3a89bd5..35b4c7a 100644
--- a/src/sys/pkg/tests/pkg-resolver/src/metrics.rs
+++ b/src/sys/pkg/tests/pkg-resolver/src/metrics.rs
@@ -791,7 +791,6 @@
.uri_path_override_handler(handler::ForPath::new("/2.root.json", handler))
.start()
.unwrap();
- env.register_repo(&served_repository).await;
struct StatusTest {
min_code: u16,
@@ -856,6 +855,12 @@
for ent in test_table.iter() {
for code in ent.min_code..=ent.max_code {
+ // After the package resolver successfully updates TUF, it will automatically cache the
+ // fetched metadata for a period of time in order to reduce load on the repository
+ // server. Since we want each of these resolves to talk to our test server, we'll
+ // re-register the repository before each request in order to reset the timer.
+ env.register_repo(&served_repository).await;
+
response_code.set(code);
let _ = env.resolve_package(&pkg_url).await;
statuses.push(ent.status);
