From: Karthik Nayak <karthik.188@gmail.com>
To: git@vger.kernel.org
Cc: jltobler@gmail.com, ps@pks.im, gitster@pobox.com,
sunshine@sunshineco.com,
Christian Couder <chriscool@tuxfamily.org>,
Karthik Nayak <karthik.188@gmail.com>
Subject: [PATCH v5 0/2] refs: fix some bugs with batched-updates
Date: Fri, 20 Jun 2025 09:15:43 +0200 [thread overview]
Message-ID: <20250620-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v5-0-f35ee6b59a82@gmail.com> (raw)
In-Reply-To: <20250602-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v1-0-903d1db3f10e@gmail.com>
In 23fc8e4f61 (refs: implement batch reference update support,
2025-04-08) we introduced a mechanism to batch reference updates. The
idea being that we wanted to batch updates similar to a transaction, but
allow certain updates to fail. This would help reference backends
optimize such operations and also remove the overhead of processing
updates individually. Especially for the reftable backend, where
batching updates would ensure that the autocompaction would only occur
at the end of the batch instead of after every reference update.
As of 9d2962a7c4 (receive-pack: use batched reference updates,
2025-05-19) we also updated the 'git-fetch(1)' and 'git-receive-pack(1)'
command to use batched reference updates. This series fixes some bugs
that we found at GitLab by running our Gitaly service with the `next`
build of Git.
The first being in the files backend, which missed skipping over failed
updates in certain flows. When certain individual updates fail, we mark
them as such. However, we missed skipping over such failed updates,
which can cause a SEGFAULT.
The other is in the git-receive-pack(1) implementation when a user
pushes multiple branches such as:
delete refs/heads/branch/conflict
create refs/heads/branch
Before using batched updates, the references would be applied
sequentially and hence no conflicts would arise. With batched updates,
while the first update applies, the second fails due to F/D conflict. A
similar issue was present in 'git-fetch(1)' and was fixed by using
separating out reference pruning into a separate transaction. Apply a
similar mechanism for 'git-receive-pack(1)' and separate out reference
deletions into its own batch.
This is based off master 7014b55638 (A bit more topics for -rc1,
2025-05-30), with the changes from kn/fetch-push-bulk-ref-update merged
in.
---
Changes in v5:
- Modify the commit message in patch 2 to also mention the commit
which adds multiple transactions to 'git-fetch(1)'.
- Also add a NEEDSWORK to the existing comment to mention how conflict
resolution within transactions can remove the need for multiple
transactions.
- Link to v4: https://lore.kernel.org/r/20250613-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v4-0-ebf53edb9795@gmail.com
Changes in v4:
- Swapped out F/D for D/F in the second commit, since we are talking
about conflicts between a directory and a file, also D/F is more
consistent.
- Fixed some typos in the second commit.
- Changed comment to single line.
- Link to v3: https://lore.kernel.org/r/20250606-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v3-0-e1c41693bd35@gmail.com
Changes in v3:
- Cleanup the second commit message and remove stale comments.
- In the second commit, only create the transaction if needed.
- Link to v2: https://lore.kernel.org/r/20250605-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v2-0-26cd05b8a79e@gmail.com
Changes in v2:
- Modify the test in the first commit to no longer do a quiet grep,
and some more tests.
- Remove the second commit as it was unnecessary.
- Modify the commit message in the last commit, to also talk about how
we now use 2 transactions at minimum but this is still better than
before.
- Modify the logic in the last commit to no longer use an XOR and
instead loop over the two different scenarios (deletion updates, other
updates).
- Link to v1: https://lore.kernel.org/r/20250602-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v1-0-903d1db3f10e@gmail.com
---
builtin/receive-pack.c | 102 ++++++++++++++++++++++++++++++++-----------------
refs/files-backend.c | 7 ++++
t/t1400-update-ref.sh | 45 ++++++++++++++++++++++
t/t5516-fetch-push.sh | 17 +++++++--
4 files changed, 133 insertions(+), 38 deletions(-)
Karthik Nayak (2):
refs/files: skip updates with errors in batched updates
receive-pack: handle reference deletions separately
Range-diff versus v4:
1: 0ab0890bf1 = 1: 9248bfd474 refs/files: skip updates with errors in batched updates
2: e45ec3139b ! 2: 00bf816b2d receive-pack: handle reference deletions separately
@@ Commit message
sequentially and hence no conflicts would arise. With batched updates,
while the first update applies, the second fails due to D/F conflict. A
similar issue was present in 'git-fetch(1)' and was fixed by separating
- out reference pruning into a separate transaction. Apply a similar
- mechanism for 'git-receive-pack(1)' and separate out reference deletions
- into its own batch.
+ out reference pruning into a separate transaction in the commit 'fetch:
+ use batched reference updates'. Apply a similar mechanism for
+ 'git-receive-pack(1)' and separate out reference deletions into its own
+ batch.
This means 'git-receive-pack(1)' will now use up to two transactions,
whereas before using batched updates it would use _at least_ two
@@ builtin/receive-pack.c: static void execute_commands_non_atomic(struct command *
+ * are treated as conflicts in batched updates. This is because
+ * we don't do conflict resolution inside a transaction. To
+ * mitigate this, delete references in a separate batch.
++ *
++ * NEEDSWORK: Add conflict resolution between deletion and creation
++ * of reference updates within a transaction. With that, we can
++ * combine the two phases.
+ */
+ enum processing_phase {
+ PHASE_DELETIONS,
base-commit: 931c39f05e078e0df968a439379cb04b5c4666ef
change-id: 20250528-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-24ff53680144
Thanks
- Karthik
next prev parent reply other threads:[~2025-06-20 7:15 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-02 9:57 [PATCH 0/3] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-02 9:57 ` [PATCH 1/3] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-02 12:00 ` Patrick Steinhardt
2025-06-02 12:46 ` Karthik Nayak
2025-06-02 15:06 ` Junio C Hamano
2025-06-02 17:13 ` Karthik Nayak
2025-06-02 9:57 ` [PATCH 2/3] t5516: use double quotes for tests with variables Karthik Nayak
2025-06-02 16:59 ` Eric Sunshine
2025-06-02 17:13 ` Karthik Nayak
2025-06-02 9:57 ` [PATCH 3/3] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-02 11:59 ` Patrick Steinhardt
2025-06-02 12:54 ` Karthik Nayak
2025-06-02 15:20 ` Junio C Hamano
2025-06-02 15:56 ` Patrick Steinhardt
2025-06-04 11:08 ` Karthik Nayak
2025-06-05 8:19 ` [PATCH v2 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-05 8:19 ` [PATCH v2 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-05 8:19 ` [PATCH v2 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-05 8:47 ` Patrick Steinhardt
2025-06-05 9:08 ` Karthik Nayak
2025-06-06 8:41 ` [PATCH v3 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-06 8:41 ` [PATCH v3 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-06 8:41 ` [PATCH v3 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-12 17:03 ` Christian Couder
2025-06-12 20:40 ` Junio C Hamano
2025-06-13 7:23 ` Karthik Nayak
2025-06-13 8:10 ` [PATCH v4 0/2] refs: fix some bugs with batched-updates Karthik Nayak
2025-06-13 8:10 ` [PATCH v4 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-13 8:10 ` [PATCH v4 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-13 15:46 ` Junio C Hamano
2025-06-19 9:39 ` Karthik Nayak
2025-06-13 12:43 ` [PATCH v4 0/2] refs: fix some bugs with batched-updates Christian Couder
2025-06-13 18:57 ` Junio C Hamano
2025-06-20 7:15 ` Karthik Nayak [this message]
2025-06-20 7:15 ` [PATCH v5 1/2] refs/files: skip updates with errors in batched updates Karthik Nayak
2025-06-20 7:15 ` [PATCH v5 2/2] receive-pack: handle reference deletions separately Karthik Nayak
2025-06-20 16:21 ` [PATCH v5 0/2] refs: fix some bugs with batched-updates Junio C Hamano
2025-06-21 11:08 ` Karthik Nayak
2025-06-22 4:23 ` Junio C Hamano
2025-06-22 14:20 ` Karthik Nayak
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250620-6769-address-test-failures-in-the-next-branch-caused-by-batched-reference-updates-v5-0-f35ee6b59a82@gmail.com \
--to=karthik.188@gmail.com \
--cc=chriscool@tuxfamily.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jltobler@gmail.com \
--cc=ps@pks.im \
--cc=sunshine@sunshineco.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.
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 NNTP newsgroup(s).