From: Patrick Steinhardt <ps@pks.im>
To: git@vger.kernel.org
Cc: Christian Couder <christian.couder@gmail.com>,
Jonathan Tan <jonathantanmy@google.com>,
Elijah Newren <newren@gmail.com>
Subject: [PATCH v2 7/7] fetch: make `--atomic` flag cover pruning of refs
Date: Thu, 17 Feb 2022 14:04:41 +0100 [thread overview]
Message-ID: <2ad16530e5df1119a7c17d3a382b420187c93c63.1645102965.git.ps@pks.im> (raw)
In-Reply-To: <cover.1645102965.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 5171 bytes --]
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 | 8 ++------
2 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 67af842091..9a2b5c03a4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1338,11 +1338,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
@@ -1363,13 +1366,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) {
@@ -1388,6 +1400,8 @@ static int prune_refs(struct refspec *rs, struct ref *ref_map,
}
}
+cleanup:
+ strbuf_release(&err);
free(url);
free_refs(stale_refs);
return result;
@@ -1629,10 +1643,10 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (rs->nr) {
- retcode = prune_refs(rs, ref_map, transport->url);
+ retcode = prune_refs(rs, transaction, ref_map, transport->url);
} else {
retcode = prune_refs(&transport->remote->fetch,
- ref_map,
+ transaction, ref_map,
transport->url);
}
if (retcode != 0)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 70d51f343b..48e14e2dab 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -354,17 +354,13 @@ test_expect_success 'fetch --atomic --prune executes a single reference transact
head_oid=$(git rev-parse HEAD) &&
# Fetching with the `--atomic` flag should update all references in a
- # single transaction. It is currently missing coverage of pruned
- # references though, and as a result those may be committed to disk
- # even if updating references fails later.
+ # single transaction.
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2022-02-17 13:05 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 13:04 [PATCH v2 0/7] fetch: improve atomicity of `--atomic` flag Patrick Steinhardt
2022-02-17 13:04 ` [PATCH v2 1/7] fetch: increase test coverage of fetches Patrick Steinhardt
2022-02-17 15:18 ` Christian Couder
2022-02-21 7:57 ` Patrick Steinhardt
2022-02-17 20:41 ` Junio C Hamano
2022-02-17 22:43 ` Junio C Hamano
2022-02-18 6:49 ` Patrick Steinhardt
2022-02-18 16:59 ` Junio C Hamano
2022-03-03 0:25 ` Junio C Hamano
2022-03-03 6:47 ` Patrick Steinhardt
2022-02-17 13:04 ` [PATCH v2 2/7] fetch: backfill tags before setting upstream Patrick Steinhardt
2022-02-17 22:07 ` Junio C Hamano
2022-02-17 13:04 ` [PATCH v2 3/7] fetch: control lifecycle of FETCH_HEAD in a single place Patrick Steinhardt
2022-02-17 22:12 ` Junio C Hamano
2022-02-17 13:04 ` [PATCH v2 4/7] fetch: report errors when backfilling tags fails Patrick Steinhardt
2022-02-17 22:16 ` Junio C Hamano
2022-02-17 13:04 ` [PATCH v2 5/7] refs: add interface to iterate over queued transactional updates Patrick Steinhardt
2022-02-17 13:04 ` [PATCH v2 6/7] fetch: make `--atomic` flag cover backfilling of tags Patrick Steinhardt
2022-02-17 22:27 ` Junio C Hamano
2022-02-17 13:04 ` Patrick Steinhardt [this message]
2022-02-17 15:50 ` [PATCH v2 0/7] fetch: improve atomicity of `--atomic` flag Christian Couder
2022-02-17 22:30 ` Junio C Hamano
2022-02-21 8:02 ` [PATCH v3 " Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 1/7] fetch: increase test coverage of fetches Patrick Steinhardt
2022-03-03 0:30 ` Junio C Hamano
2022-03-03 0:32 ` Junio C Hamano
2022-03-03 6:43 ` Patrick Steinhardt
2022-03-03 6:49 ` Junio C Hamano
2022-03-03 6:51 ` Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 2/7] fetch: backfill tags before setting upstream Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 3/7] fetch: control lifecycle of FETCH_HEAD in a single place Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 4/7] fetch: report errors when backfilling tags fails Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 5/7] refs: add interface to iterate over queued transactional updates Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 6/7] fetch: make `--atomic` flag cover backfilling of tags Patrick Steinhardt
2022-02-21 8:02 ` [PATCH v3 7/7] fetch: make `--atomic` flag cover pruning of refs Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2ad16530e5df1119a7c17d3a382b420187c93c63.1645102965.git.ps@pks.im \
--to=ps@pks.im \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).