bundle-uri: store fetch.bundleCreationToken

When a bundle list specifies the "creationToken" heuristic, the Git client downloads the list and then starts downloading bundles in descending creationToken order. This process stops as soon as all downloaded bundles can be applied to the repository (because all required commits are present in the repository or in the downloaded bundles). When checking the same bundle list twice, this strategy requires downloading the bundle with the maximum creationToken again, which is wasteful. The creationToken heuristic promises that the client will not have a use for that bundle if its creationToken value is at most the previous creationToken value. To prevent these wasteful downloads, create a fetch.bundleCreationToken config setting that the Git client sets after downloading bundles. This value allows skipping that maximum bundle download when this config value is the same value (or larger). To test that this works correctly, we can insert some "duplicate" fetches into existing tests and demonstrate that only the bundle list is downloaded. The previous logic for downloading bundles by creationToken worked even if the bundle list was empty, but now we have logic that depends on the first entry of the list. Terminate early in the (non-sensical) case of an empty bundle list. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Jan 31, 2023 at 13:29 UTC c429bed10223135a13f1f2f854e367762c37af7a
3 files changed +90 -3
Documentation/config/fetch.txt
+16
@@ -104,3 +104,19 @@ fetch.bundleURI::
104 linkgit:git-clone[1]. `git clone --bundle-uri` will set the
105 `fetch.bundleURI` value if the supplied bundle URI contains a bundle
106 list that is organized for incremental fetches.
107 ++
108 +If you modify this value and your repository has a `fetch.bundleCreationToken`
109 +value, then remove that `fetch.bundleCreationToken` value before fetching from
110 +the new bundle URI.
111 +
112 +fetch.bundleCreationToken::
113 + When using `fetch.bundleURI` to fetch incrementally from a bundle
114 + list that uses the "creationToken" heuristic, this config value
115 + stores the maximum `creationToken` value of the downloaded bundles.
116 + This value is used to prevent downloading bundles in the future
117 + if the advertised `creationToken` is not strictly larger than this
118 + value.
119 ++
120 +The creation token values are chosen by the provider serving the specific
121 +bundle URI. If you modify the URI at `fetch.bundleURI`, then be sure to
122 +remove the value for the `fetch.bundleCreationToken` value before fetching.
bundle-uri.c
+46 -2
@@ -481,6 +481,8 @@ static int fetch_bundles_by_token(struct repository *r,
481 {
482 int cur;
483 int move_direction = 0;
484 + const char *creationTokenStr;
485 + uint64_t maxCreationToken = 0, newMaxCreationToken = 0;
486 struct bundle_list_context ctx = {
487 .r = r,
488 .list = list,
@@ -494,8 +496,27 @@ static int fetch_bundles_by_token(struct repository *r,
496
497 for_all_bundles_in_list(list, append_bundle, &bundles);
498
499 + if (!bundles.nr) {
500 + free(bundles.items);
501 + return 0;
502 + }
503 +
504 QSORT(bundles.items, bundles.nr, compare_creation_token_decreasing);
505
506 + /*
507 + * If fetch.bundleCreationToken exists, parses to a uint64t, and
508 + * is not strictly smaller than the maximum creation token in the
509 + * bundle list, then do not download any bundles.
510 + */
511 + if (!repo_config_get_value(r,
512 + "fetch.bundlecreationtoken",
513 + &creationTokenStr) &&
514 + sscanf(creationTokenStr, "%"PRIu64, &maxCreationToken) == 1 &&
515 + bundles.items[0]->creationToken <= maxCreationToken) {
516 + free(bundles.items);
517 + return 0;
518 + }
519 +
520 /*
521 * Attempt to download and unbundle the minimum number of bundles by
522 * creationToken in decreasing order. If we fail to unbundle (after
@@ -516,6 +537,16 @@ static int fetch_bundles_by_token(struct repository *r,
537 cur = 0;
538 while (cur >= 0 && cur < bundles.nr) {
539 struct remote_bundle_info *bundle = bundles.items[cur];
540 +
541 + /*
542 + * If we need to dig into bundles below the previous
543 + * creation token value, then likely we are in an erroneous
544 + * state due to missing or invalid bundles. Halt the process
545 + * instead of continuing to download extra data.
546 + */
547 + if (bundle->creationToken <= maxCreationToken)
548 + break;
549 +
550 if (!bundle->file) {
551 /*
552 * Not downloaded yet. Try downloading.
@@ -555,6 +586,9 @@ static int fetch_bundles_by_token(struct repository *r,
586 */
587 move_direction = -1;
588 bundle->unbundled = 1;
589 +
590 + if (bundle->creationToken > newMaxCreationToken)
591 + newMaxCreationToken = bundle->creationToken;
592 }
593 }
594
@@ -569,14 +603,24 @@ move:
603 cur += move_direction;
604 }
605
572 - free(bundles.items);
573 -
606 /*
607 * We succeed if the loop terminates because 'cur' drops below
608 * zero. The other case is that we terminate because 'cur'
609 * reaches the end of the list, so we have a failure no matter
610 * which bundles we apply from the list.
611 */
612 + if (cur < 0) {
613 + struct strbuf value = STRBUF_INIT;
614 + strbuf_addf(&value, "%"PRIu64"", newMaxCreationToken);
615 + if (repo_config_set_multivar_gently(ctx.r,
616 + "fetch.bundleCreationToken",
617 + value.buf, NULL, 0))
618 + warning(_("failed to store maximum creation token"));
619 +
620 + strbuf_release(&value);
621 + }
622 +
623 + free(bundles.items);
624 return cur >= 0;
625 }
626
t/t5558-clone-bundle-uri.sh
+28 -1
@@ -433,6 +433,7 @@ test_expect_success 'clone incomplete bundle list (http, creationToken)' '
433 "$HTTPD_URL/smart/fetch.git" clone-token-http &&
434
435 test_cmp_config -C clone-token-http "$HTTPD_URL/bundle-list" fetch.bundleuri &&
436 + test_cmp_config -C clone-token-http 1 fetch.bundlecreationtoken &&
437
438 cat >expect <<-EOF &&
439 $HTTPD_URL/bundle-list
@@ -468,6 +469,7 @@ test_expect_success 'clone incomplete bundle list (http, creationToken)' '
469 GIT_TRACE2_EVENT="$(pwd)/trace1.txt" \
470 git -C clone-token-http fetch origin --no-tags \
471 refs/heads/merge:refs/heads/merge &&
472 + test_cmp_config -C clone-token-http 4 fetch.bundlecreationtoken &&
473
474 cat >expect <<-EOF &&
475 $HTTPD_URL/bundle-list
@@ -511,6 +513,7 @@ test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
513 "$HTTPD_URL/smart/fetch.git" fetch-http-4 &&
514
515 test_cmp_config -C fetch-http-4 "$HTTPD_URL/bundle-list" fetch.bundleuri &&
516 + test_cmp_config -C fetch-http-4 1 fetch.bundlecreationtoken &&
517
518 cat >expect <<-EOF &&
519 $HTTPD_URL/bundle-list
@@ -538,6 +541,7 @@ test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
541 git -C fetch-http-4 fetch origin --no-tags \
542 refs/heads/left:refs/heads/left \
543 refs/heads/right:refs/heads/right &&
544 + test_cmp_config -C fetch-http-4 2 fetch.bundlecreationtoken &&
545
546 cat >expect <<-EOF &&
547 $HTTPD_URL/bundle-list
@@ -555,6 +559,18 @@ test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
559 EOF
560 test_cmp expect refs &&
561
562 + # No-op fetch
563 + GIT_TRACE2_EVENT="$(pwd)/trace1b.txt" \
564 + git -C fetch-http-4 fetch origin --no-tags \
565 + refs/heads/left:refs/heads/left \
566 + refs/heads/right:refs/heads/right &&
567 +
568 + cat >expect <<-EOF &&
569 + $HTTPD_URL/bundle-list
570 + EOF
571 + test_remote_https_urls <trace1b.txt >actual &&
572 + test_cmp expect actual &&
573 +
574 cat >>"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
575 [bundle "bundle-3"]
576 uri = bundle-3.bundle
@@ -570,6 +586,7 @@ test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
586 GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
587 git -C fetch-http-4 fetch origin --no-tags \
588 refs/heads/merge:refs/heads/merge &&
589 + test_cmp_config -C fetch-http-4 4 fetch.bundlecreationtoken &&
590
591 cat >expect <<-EOF &&
592 $HTTPD_URL/bundle-list
@@ -588,7 +605,17 @@ test_expect_success 'http clone with bundle.heuristic creates fetch.bundleURI' '
605 refs/bundles/left
606 refs/bundles/merge
607 EOF
591 - test_cmp expect refs
608 + test_cmp expect refs &&
609 +
610 + # No-op fetch
611 + GIT_TRACE2_EVENT="$(pwd)/trace2b.txt" \
612 + git -C fetch-http-4 fetch origin &&
613 +
614 + cat >expect <<-EOF &&
615 + $HTTPD_URL/bundle-list
616 + EOF
617 + test_remote_https_urls <trace2b.txt >actual &&
618 + test_cmp expect actual
619 '
620
621 # Do not add tests here unless they use the HTTP server, as they will