lib/pull: Don't scan commit objects we fetch via deltas
authorJonathan Lebon <jonathan@jlebon.com>
Fri, 8 Sep 2023 20:54:29 +0000 (16:54 -0400)
committerJonathan Lebon <jonathan@jlebon.com>
Fri, 8 Sep 2023 21:49:25 +0000 (17:49 -0400)
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
src/libostree/ostree-repo-pull.c
tests/test-delta.sh

index 982cf1b57b7c86752418d20d8bdb640419283631..2a0b7f44b18513f891f2c194198227e9939475ef 100644 (file)
@@ -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<checksum> 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 */
index d593df2523a886f19c7c669d65a3f5cf6c0280dc..6822028c270c745275ecd037d01d8de96d82626a 100644 (file)
@@ -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 @@ out:
   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);
index 2a6302670cd9ee974eb064929aec0204ca5bfbf3..4c9e2e2d89cda27b7e42cba7fd6d35a2764e68f9 100755 (executable)
@@ -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