[0/2] Fix locking issues with git fetch --multiple --jobs=<n> and fetch.writeCommitGraph

Message

Linus Arver via GitGitGadget Nov. 3, 2019, 12:21 a.m. UTC

The git fetch command recently learned to extend the --jobs=<n> option to
cover the --multiple mode: it will run multiple fetches in parallel.

Together with the recent support to write commit-graphs automatically after
each fetch by setting fetch.writeCommitGraph, this led to frequent issues
where the commit-graph-chain.lock file could not be created because a
parallel job had already created it.

This pair of patches first introduces the command-line option 
--write-commit-graph (together with the --no-* variant) and then uses it to
avoid writing the commit-graph until all fetch jobs are complete.

I don't think that we will want to rush this into Git v2.24.0 because that
release is imminent, and this is quite a corner case that I am fixing here.
It's more of a FYI that I send this before v2.24.0 is available.

Johannes Schindelin (2):
  fetch: add the command-line option `--write-commit-graph`
  fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph

 Documentation/fetch-options.txt |  4 ++++
 builtin/fetch.c                 | 10 ++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)


base-commit: efd54442381a2792186abc994060b8f7dd8b834b
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-443%2Fdscho%2Ffetch.writeCommitGraph-and-fetch-jobs-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-443/dscho/fetch.writeCommitGraph-and-fetch-jobs-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/443

Comments

Jeff King Nov. 4, 2019, 7:59 p.m. UTC | #1

On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote:

> The git fetch command recently learned to extend the --jobs=<n> option to
> cover the --multiple mode: it will run multiple fetches in parallel.
> 
> Together with the recent support to write commit-graphs automatically after
> each fetch by setting fetch.writeCommitGraph, this led to frequent issues
> where the commit-graph-chain.lock file could not be created because a
> parallel job had already created it.
> 
> This pair of patches first introduces the command-line option 
> --write-commit-graph (together with the --no-* variant) and then uses it to
> avoid writing the commit-graph until all fetch jobs are complete.

Thanks, the whole thing looks clearly explained and the patches
themselves look good. And having "--[no-]write-commit-graph" is a good
thing even independent of the problem you're fixing.

I wondered if it was worth having a test in the second patch, but I
think it would be inherently racy. So it's probably not worth the
trouble.

-Peff

Junio C Hamano Nov. 6, 2019, 1:59 a.m. UTC | #2

Jeff King <[e-mail]> writes:

> Thanks, the whole thing looks clearly explained and the patches
> themselves look good. And having "--[no-]write-commit-graph" is a good
> thing even independent of the problem you're fixing.
>
> I wondered if it was worth having a test in the second patch, but I
> think it would be inherently racy. So it's probably not worth the
> trouble.

Thanks, both.

Johannes Schindelin Nov. 6, 2019, 12:05 p.m. UTC | #3

Hi Peff,

On Mon, 4 Nov 2019, Jeff King wrote:

> On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > The git fetch command recently learned to extend the --jobs=<n> option to
> > cover the --multiple mode: it will run multiple fetches in parallel.
> >
> > Together with the recent support to write commit-graphs automatically after
> > each fetch by setting fetch.writeCommitGraph, this led to frequent issues
> > where the commit-graph-chain.lock file could not be created because a
> > parallel job had already created it.
> >
> > This pair of patches first introduces the command-line option
> > --write-commit-graph (together with the --no-* variant) and then uses it to
> > avoid writing the commit-graph until all fetch jobs are complete.
>
> Thanks, the whole thing looks clearly explained and the patches
> themselves look good. And having "--[no-]write-commit-graph" is a good
> thing even independent of the problem you're fixing.
>
> I wondered if it was worth having a test in the second patch, but I
> think it would be inherently racy. So it's probably not worth the
> trouble.

Yes, I gave testing a great deal of thought, and I failed at coming up
with any way to automate it.

Thanks,
Dscho