[PATCH 6/6] fetch: make `--atomic` flag cover pruning of refs
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 11, 2022, 07:47 UTC
- Message-ID
- <682f16117b743bec59c533e15ae5a88d39250222.1644565025.git.ps@pks.im>
- In-Reply-To
- <cover.1644565025.git.ps@pks.im>
When fetching with the `--prune` flag we will delete any local references matching the fetch refspec which have disappeared on the remote. This step is not currently covered by the `--atomic` flag: we delete branches even though updating of local references has failed, which means that the fetch is not an all-or-nothing operation.
Fix this bug by passing in the global transaction into `prune_refs()`: if one is given, then we'll only queue up deletions and not commit them right away.
This change also improves performance when pruning many branches in a repository with a big packed-refs file: every references is pruned in its own transaction, which means that we potentially have to rewrite the packed-refs files for every single reference we're about to prune.
The following benchmark demonstrates this: it performs a pruning fetch from a repository with a single reference into a repository with 100k references, which causes us to prune all but one reference. This is of course a very artificial setup, but serves to demonstrate the impact of only having to write the packed-refs file once:
Benchmark 1: git fetch --prune --atomic +refs/*:refs/* (HEAD~)
Time (mean ± σ): 2.366 s ± 0.021 s [User: 0.858 s, System: 1.508 s]
Range (min … max): 2.328 s … 2.407 s 10 runs Benchmark 2: git fetch --prune --atomic +refs/*:refs/* (HEAD)
Time (mean ± σ): 1.369 s ± 0.017 s [User: 0.715 s, System: 0.641 s]
Range (min … max): 1.346 s … 1.400 s 10 runs Summary
'git fetch --prune --atomic +refs/*:refs/* (HEAD)' ran
1.73 ± 0.03 times faster than 'git fetch --prune --atomic +refs/*:refs/* (HEAD~)'Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/fetch.c | 30 ++++++++++++++++++++++-------- t/t5510-fetch.sh | 4 +--- 2 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c index 348e64cf2c..75e791a4b4 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1333,11 +1333,14 @@ static int fetch_and_consume_refs(struct transport *transport, return ret; } -static int prune_refs(struct refspec *rs, struct ref *ref_map, +static int prune_refs(struct refspec *rs, + struct ref_transaction *transaction, + struct ref *ref_map, const char *raw_url) { int url_len, i, result = 0; struct ref *ref, *stale_refs = get_stale_heads(rs, ref_map); + struct strbuf err = STRBUF_INIT; char *url; int summary_width = transport_summary_width(stale_refs); const char *dangling_msg = dry_run @@ -1358,13 +1361,22 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, url_len = i - 3; if (!dry_run) { - struct string_list refnames = STRING_LIST_INIT_NODUP; + if (transaction) { + for (ref = stale_refs; ref; ref = ref->next) { + result = ref_transaction_delete(transaction, ref->name, NULL, 0, + "fetch: prune", &err); + if (result) + goto cleanup; + } + } else { + struct string_list refnames = STRING_LIST_INIT_NODUP; - for (ref = stale_refs; ref; ref = ref->next) - string_list_append(&refnames, ref->name); + for (ref = stale_refs; ref; ref = ref->next) + string_list_append(&refnames, ref->name); - result = delete_refs("fetch: prune", &refnames, 0); - string_list_clear(&refnames, 0); + result = delete_refs("fetch: prune", &refnames, 0); + string_list_clear(&refnames, 0); + } } if (verbosity >= 0) { @@ -1383,6 +1395,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map, } } +cleanup: + strbuf_release(&err); free(url); free_refs(stale_refs); return result; @@ -1624,10 +1638,10 @@ static int do_fetch(struct transport *transport, * don't care whether --tags was specified. */ if (rs->nr) { - prune_refs(rs, ref_map, transport->url); + prune_refs(rs, transaction, ref_map, transport->url); } else { prune_refs(&transport->remote->fetch, - ref_map, + transaction, ref_map, transport->url); } } diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 93a0db3c68..afa6bf9f7d 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -349,11 +349,9 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact cat >expected <<-EOF && prepared $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion - committed - $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion - prepared $ZERO_OID $head_oid refs/remotes/origin/new-branch committed + $ZERO_OID $ZERO_OID refs/remotes/origin/scheduled-for-deletion $ZERO_OID $head_oid refs/remotes/origin/new-branch EOF
-- 2.35.1