[PATCH v5 2/3] fetch: advertise fetched refs when backfilling tags
flat view
WARM3d
SUPERSEDED
From: Karthik Nayak <hidden>
Date: 2026-10-28 09:48:09
Subsystem:
the rest · Maintainer:
Linus Torvalds
Status: Superseded by v6 (posted 2026-10-29). [go to v6]
Revision v5 (2 revisions archived).
Revisions (2)
- v5 current
- v6 [diff against previous]
In 0e358de64a (fetch: use batched reference updates, 2025-05-19), the fetch code was modified to use batched updates to provide a good performance improvement. Batched updates are used both for the fetched references and to backfill tags. When using batched updates, the references aren't yet committed to disk when we start backfilling tags. For shallow fetches, the negotiation phase while backfilling tags does not advertise the newly fetched refs. This can cause the server to send objects the client already has. The issue always existed when the '--atomic' flag was used, as both the regular ref updates and the tags need to be part of the same transaction. Fix this by advertising the already fetched refs as 'haves' during the negotiation. Do this by simply adding the tips to the transport's `negotiation_include_tips` field. This also fixes the issue for `--atomic` fetches since it follows the same route. This does mean that all references fetched before the backfill will be used for the negotiation. While duplicate OIDs are trimmed before negotiation, we could still end up with a lot of distinct OIDs used in the 'haves' section. However, this is still better than the server transferring already existing objects to the client, which can take a lot more time. Add two tests, which validate that the 'haves' are filled when backfilling tags for atomic and non-atomic flows. Reported-by: Mitja Bezenšek <redacted> Signed-off-by: Karthik Nayak <redacted> --- builtin/fetch.c | 33 +++++++++++++++++++++++++++++++-- t/t5510-fetch.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b2decc6cfd..d3974faf6b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c@@ -1676,7 +1676,8 @@ static int backfill_tags(struct display_state *display_state, struct fetch_head *fetch_head, const struct fetch_config *config, struct ref_update_display_info_array *display_array, - struct list_objects_filter_options *filter_options) + struct list_objects_filter_options *filter_options, + const struct oid_array *fetched_tips) { int retcode, cannot_reuse;
@@ -1697,6 +1698,21 @@ static int backfill_tags(struct display_state *display_state, transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); + + /* + * References fetched before the tag backfill aren't yet committed + * to disk, so negotiation cannot discover them. Advertise them to + * the server, so that it does not re-send objects we already have. + */ + if (fetched_tips && fetched_tips->nr && transport->smart_options) { + struct oid_array **include_tips = + &transport->smart_options->negotiation_include_tips; + if (!*include_tips) + *include_tips = xcalloc(1, sizeof(**include_tips)); + for (size_t i = 0; i < fetched_tips->nr; i++) + oid_array_append(*include_tips, &fetched_tips->oid[i]); + } + retcode = fetch_and_consume_refs(display_state, transport, transaction, ref_map, fetch_head, config, display_array);
@@ -2077,6 +2093,16 @@ static int do_fetch(struct transport *transport, find_non_local_tags(remote_refs, transaction, &tags_ref_map, &tail); if (tags_ref_map) { + struct oid_array fetched_tips = OID_ARRAY_INIT; + + /* + * Collect the tips we just fetched so the tag backfill + * can advertise them to the server as 'haves'. + */ + for (struct ref *rm = ref_map; rm; rm = rm->next) + if (!is_null_oid(&rm->old_oid)) + oid_array_append(&fetched_tips, &rm->old_oid); + /* * If backfilling of tags fails then we want to tell * the user so, but we have to continue regardless to
@@ -2086,8 +2112,11 @@ static int do_fetch(struct transport *transport, * the transaction and don't commit anything. */ if (backfill_tags(&display_state, transport, transaction, tags_ref_map, - &fetch_head, config, &display_array, filter_options)) + &fetch_head, config, &display_array, filter_options, + &fetched_tips)) retcode = 1; + + oid_array_clear(&fetched_tips); } free_refs(tags_ref_map);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 300bd5396d..7c1f8405df 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh@@ -1942,6 +1942,40 @@ test_expect_success "backfill tags when providing a refspec" ' test_cmp expect actual ' +test_expect_success 'backfilling tags advertises fetched refs as haves' ' + test_when_finished rm -rf source target trace && + + git init source && + test_commit_bulk -C source 10 && + git -C source tag -a tag -m tag HEAD~2 && + HEAD_OID=$(git -C source rev-parse HEAD) && + + git init target && + git -C target remote add origin ../source && + GIT_TEST_PROTOCOL_VERSION=2 GIT_TRACE_PACKET=$(pwd)/trace \ + git -C target fetch --depth 5 origin && + + test_grep "fetch> have $HEAD_OID" trace && + git -C target rev-parse --verify refs/tags/tag +' + +test_expect_success 'backfilling tags advertises fetched refs as haves even with atomic' ' + test_when_finished rm -rf source target trace && + + git init source && + test_commit_bulk -C source 10 && + git -C source tag -a tag -m tag HEAD~2 && + HEAD_OID=$(git -C source rev-parse HEAD) && + + git init target && + git -C target remote add origin ../source && + GIT_TEST_PROTOCOL_VERSION=2 GIT_TRACE_PACKET=$(pwd)/trace \ + git -C target fetch --atomic --depth 5 origin && + + test_grep "fetch> have $HEAD_OID" trace && + git -C target rev-parse --verify refs/tags/tag +' + test_expect_success REFFILES "FETCH_HEAD is updated even if ref updates fail" ' test_when_finished rm -rf base repo &&
--
2.56.GIT