Re: [PATCH 1/1] fetch: add fetch.writeCommitGraph config setting

From: Jeff King <peff@peff.net>
Date: Sep 7, 2019, 04:46 UTC
Message-ID: <20190907044650.GB24463@sigill.intra.peff.net>
In-Reply-To: <25ed1562-dc74-1ebc-46ff-d6cd643504a4@gmail.com>

On Fri, Sep 06, 2019 at 05:04:17PM -0400, Derrick Stolee wrote:

Show 20 quoted lines

> On 9/6/2019 4:42 PM, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> > 
> >> I suppose so. But I think the "stock git without any other job
> >> infrastructure" case would still benefit.
> > 
> > Oh, no question about it.
> > 
> > I did question if an automatic writing would benefit the side that
> > receives a push when you brought up the usual "fetch.* and receive.*
> > for separate configuration, transfer.* when want to rule them both"
> > convention, which is a good idea if only for consistency, but the
> > question was if it helps the receiving end of a push to the same
> > degree as it would help the repository that fetches.
> 
> I think the "stock git without any other job infrastructure" is
> a very important scenario. Putting the simplest version of
> "commit-graph writes in-line with every push" seems to be ripe
> for failure under load. I'd rather think deeply about what is
> best for this scenario.

If it's going to be a problem under load, that makes me worry about the same thing for fetches. Whether you see a lot of either depends on your workflow. But as long as neither option is enabled by default (or at least if it becomes common knowledge to _disable_ them if you have a high rate), it may be OK.

I do agree that the normal "busy" repos you and I have both seen in enterprise settings (where people are literally pushing multiple times per second at peak) would want some kind of throttling. But I think there's a long tail of "push once a week" repos.

Show 5 quoted lines

> Spit-balling: what if we used the same mechanism as running GC
> in the background to launch 'git commit-graph write' commands?
> And we could have the command give up (without failure) if the
> commit-graph.lock file is already created, so concurrent pushes
> would not fight each other.

I have mixed feelings. It's nice not to stand in the critical path of the user, but background tasks have a way of finding funny corner cases (e.g., packfiles moving around, or the issues we've had with deciding when to shut down auto-gc for a grace period due to warnings/errors).

I think since this is generally additive (adding new commit-graph files), it might be less finicky, though.

-Peff