From c2c9521806929b587a6ebfd1470bfa8607cfd6d9 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Fri, 8 Sep 2023 16:54:29 -0400 Subject: [PATCH] lib/pull: Don't scan commit objects we fetch via deltas When we're fetching a commit via static delta, we already take care of fetching the full commit, so there's no need to also scan it using the regular object workflow. Closes: #2053 --- src/libostree/ostree-repo-pull-private.h | 2 ++ src/libostree/ostree-repo-pull.c | 9 +++++++-- tests/test-delta.sh | 20 +++++++++++++++++--- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-repo-pull-private.h b/src/libostree/ostree-repo-pull-private.h index 982cf1b57b..2a0b7f44b1 100644 --- a/src/libostree/ostree-repo-pull-private.h +++ b/src/libostree/ostree-repo-pull-private.h @@ -89,6 +89,8 @@ typedef struct signapi verified */ GHashTable *ref_keyring_map; /* Maps OstreeCollectionRef to keyring remote name */ GPtrArray *static_delta_superblocks; + GHashTable *static_delta_targets; /* Set of commits fetched via static delta */ + GHashTable *expected_commit_sizes; /* Maps commit checksum to known size */ GHashTable *commit_to_depth; /* Maps parent commit checksum maximum depth */ GHashTable *scanned_metadata; /* Maps object name to itself */ diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index d593df2523..6822028c27 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1611,9 +1611,10 @@ scan_commit_object (OtPullData *pull_data, const char *checksum, guint recursion /* We only recurse to looking whether we need dirtree/dirmeta * objects if the commit is partial, and we're not doing a - * commit-only fetch. + * commit-only fetch nor is it the target of a static delta. */ - if (is_partial && !pull_data->is_commit_only) + if (is_partial && !pull_data->is_commit_only + && !g_hash_table_contains (pull_data->static_delta_targets, checksum)) { g_autoptr (GVariant) tree_contents_csum = NULL; g_autoptr (GVariant) tree_meta_csum = NULL; @@ -2469,6 +2470,7 @@ on_superblock_fetched (GObject *src, GAsyncResult *res, gpointer data) (GVariantType *)OSTREE_STATIC_DELTA_SUPERBLOCK_FORMAT, delta_superblock_data, FALSE)); g_ptr_array_add (pull_data->static_delta_superblocks, g_variant_ref (delta_superblock)); + g_hash_table_add (pull_data->static_delta_targets, g_strdup (to_revision)); if (!process_one_static_delta (pull_data, from_revision, to_revision, delta_superblock, fetch_data->requested_ref, pull_data->cancellable, error)) goto out; @@ -4052,6 +4054,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, const char *remote_name_or_base pull_data->static_delta_superblocks = g_ptr_array_new_with_free_func ((GDestroyNotify)g_variant_unref); + pull_data->static_delta_targets + = g_hash_table_new_full (g_str_hash, g_str_equal, (GDestroyNotify)g_free, NULL); if (need_summary) { @@ -4907,6 +4911,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, const char *remote_name_or_base g_clear_pointer (&pull_data->summary_sig_etag, g_free); g_clear_pointer (&pull_data->summary, g_variant_unref); g_clear_pointer (&pull_data->static_delta_superblocks, g_ptr_array_unref); + g_clear_pointer (&pull_data->static_delta_targets, g_hash_table_unref); g_clear_pointer (&pull_data->commit_to_depth, g_hash_table_unref); g_clear_pointer (&pull_data->expected_commit_sizes, g_hash_table_unref); g_clear_pointer (&pull_data->scanned_metadata, g_hash_table_unref); diff --git a/tests/test-delta.sh b/tests/test-delta.sh index 2a6302670c..4c9e2e2d89 100755 --- a/tests/test-delta.sh +++ b/tests/test-delta.sh @@ -26,7 +26,7 @@ skip_without_user_xattrs bindatafiles="bash true ostree" morebindatafiles="false ls" -echo '1..13' +echo '1..14' mkdir repo ostree_repo_init repo --mode=archive @@ -183,13 +183,27 @@ echo 'ok heuristic endian detection' ${CMD_PREFIX} ostree --repo=repo summary -u mkdir repo2 && ostree_repo_init repo2 --mode=bare-user -${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${origrev} +${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${origrev} | tee pullstats.txt +# we should've only fetched the superblock, the index, and the delta part +assert_file_has_content pullstats.txt '1 delta parts, 2 loose fetched; .* transferred in .* seconds; 0 bytes content written' ${CMD_PREFIX} ostree --repo=repo2 fsck ${CMD_PREFIX} ostree --repo=repo2 ls ${origrev} >/dev/null echo 'ok pull delta' -rm repo2 -rf +# verify that having the commit partially doesn't degrade static delta fetching +rm repo2 pullstats.txt -rf +mkdir repo2 && ostree_repo_init repo2 --mode=bare-user +${CMD_PREFIX} ostree --repo=repo2 pull-local --disable-static-deltas repo ${origrev} --commit-metadata-only +${CMD_PREFIX} ostree --repo=repo2 pull-local --require-static-deltas repo ${origrev} | tee pullstats.txt +${CMD_PREFIX} ostree --repo=repo2 fsck +# we should've only fetched the superblock, the index, and the delta part +assert_file_has_content pullstats.txt '1 delta parts, 2 loose fetched; .* transferred in .* seconds; 0 bytes content written' +${CMD_PREFIX} ostree --repo=repo2 ls ${origrev} >/dev/null + +echo 'ok pull delta with commitpartial' + +rm repo2 pullstats.txt -rf mkdir repo2 && ostree_repo_init repo2 --mode=bare-user mkdir deltadir