Write commit-graph on fetch

builtin/fetch.c +4 -0

index 53ce99d2bb..d36a403859 100644
@@ -23,6 +23,7 @@
 #include "packfile.h"
 #include "list-objects-filter-options.h"
 #include "commit-reach.h"
+#include "commit-graph.h"
 
 #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
 

@@ -1715,6 +1716,20 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	string_list_clear(&list, 0);
 
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings.fetch_write_commit_graph) {
+		int commit_graph_flags = COMMIT_GRAPH_SPLIT;

Quoted 19 lines · Derrick Stolee via GitGitGadget

builtin/fetch.c +4 -0

index 53ce99d2bb..d36a403859 100644
@@ -23,6 +23,7 @@
 #include "packfile.h"
 #include "list-objects-filter-options.h"
 #include "commit-reach.h"
+#include "commit-graph.h"
 
 #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
 

@@ -1715,6 +1716,20 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	string_list_clear(&list, 0);
 
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings.fetch_write_commit_graph) {
+		int commit_graph_flags = COMMIT_GRAPH_SPLIT;

builtin/fetch.c int cmd_fetch(int argc, const char **argv, const char *prefix) · line 1731

Quoted 10 lines

builtin/fetch.c +10 -0

+		struct split_commit_graph_opts split_opts;
+		memset(&split_opts, 0, sizeof(struct split_commit_graph_opts));
+
+		if (progress)
+			commit_graph_flags |= COMMIT_GRAPH_PROGRESS;
+
+		write_commit_graph_reachable(get_object_directory(),
+					     commit_graph_flags,
+					     &split_opts);
+	}

As a low-impact change this is good.

For longer term, it feels a bit unfortunate that this is still a separate phase of the program, though. We know what new refs we added, we know what new objects we received, and we even scanned each and every one of them while running the index-pack step to store the .pack and compute the .idx file, i.e. it feels that we have most of the information already in-core to extend the commit graph for new parts of the history we just received.

Quoted 7 lines

builtin/fetch.c int cmd_fetch(int argc, const char **argv, const char *prefix) · line 1731

Quoted 4 lines

builtin/fetch.c +4 -0

+		write_commit_graph_reachable(get_object_directory(),
+					     commit_graph_flags,
+					     &split_opts);
+	}

As a low-impact change this is good.

Quoted 7 lines

For longer term, it feels a bit unfortunate that this is still a separate phase of the program, though. We know what new refs we added, we know what new objects we received, and we even scanned each and every one of them while running the index-pack step to store the .pack and compute the .idx file, i.e. it feels that we have most of the information already in-core to extend the commit graph for new parts of the history we just received.

You're right that we could isolate the new write to the refs we just received. We could use the more cumbersome write_commit_graph() method with a list of commit oids as starting points. I'm happy to make that change if we see a lot of value there.

Quoted 4 lines

You're right that we could isolate the new write to the refs we just received. We could use the more cumbersome write_commit_graph() method with a list of commit oids as starting points. I'm happy to make that change if we see a lot of value there.

Well, that is not the kind of information reuse I am talking about.

I was wondering if "index-pack" has enough information in-core after it receives and processes the incoming pack data, scanned each and every object in it, in order to write out the commit graph _without_ having to do a lot of duplicate computation and enumeration of the objects done in the current commit-graph.c::write_commit_graph(), so that it can learn a "--write-commit-graph" option that performs much better than running "git fetch && git commit-graph write".

Perhaps that would require too much refactoring of both index-pack and commit-graph code and infeasible in short to medium term and that is why I said "for longer term, it feels a bit unfortunate".

Generation numbers are a little trickier, though, because they imply an actual topological traversal. It might actually be easier to couple this with the connectivity check we do after index-pack finishes (though I've often wondered if we could drop that check in favor of making index-pack smarter about finding the boundaries).

From: Derrick Stolee <[email protected]>

The commit-graph feature is now on by default, and is being written during 'git gc' by default. Typically, Git only writes a commit-graph when a 'git gc --auto' command passes the gc.auto setting to actualy do work. This means that a commit-graph will typically fall behind the commits that are being used every day.

To stay updated with the latest commits, add a step to 'git fetch' to write a commit-graph after fetching new objects. The fetch.writeCommitGraph config setting enables writing a split commit-graph, so on average the cost of writing this file is very small. Occasionally, the commit-graph chain will collapse to a single level, and this could be slow for very large repos.

For additional use, adjust the default to be true when feature.experimental is enabled.

Do we want to to have fetch.writeCommitGraph, receive.writeCommitGraph, and then a master transfer.writeCommitGraph?

Quoted 2 lines · Jeff King

Do we want to to have fetch.writeCommitGraph, receive.writeCommitGraph, and then a master transfer.writeCommitGraph?

If anything, it may be good for consistency.

I am not sure if it is a good idea to trigger writing the commit graph when accepting a push, though. It tends to be a lot finer grained than fetching, right?

Quoted 7 lines · Junio C Hamano

Quoted 2 lines · Jeff King

Do we want to to have fetch.writeCommitGraph, receive.writeCommitGraph, and then a master transfer.writeCommitGraph?

If anything, it may be good for consistency.

Quoted 3 lines

I am not sure if it is a good idea to trigger writing the commit graph when accepting a push, though. It tends to be a lot finer grained than fetching, right?

And I expect a push to include many fewer commits than a fetch. In a server environment, I would expect to have a separate maintenance task responsible for updating the commit-graph after receiving new data, but not in an in-line fashion with the push.

Think about the situation of many pushes that happen in a short
burst: one write after all of the pushes would have close to the same performance benefits as writing with every push, but does a lot less work.

Sure, but wouldn't that similarly apply to fetching? What is it that makes bursts of pushes more likely than bursts of fetches?

Quoted 2 lines · Jeff King

Sure, but wouldn't that similarly apply to fetching? What is it that makes bursts of pushes more likely than bursts of fetches?

Because people tend to use a repository as a gathering point? You may periodically fetch from and push to a repository, and you may even do so at the same interval, but simply because there are more "other" people than you alone as a single developer in the project, your fetch tends to grab work from more people than yoru push that publish your work alone?

I suppose so. But I think the "stock git without any other job infrastructure" case would still benefit.

Quoted 2 lines · Jeff King

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.

Quoted 5 lines · Junio C Hamano

Quoted 2 lines · Jeff King

I suppose so. But I think the "stock git without any other job infrastructure" case would still benefit.

Quoted 8 lines

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.

Quoted 5 lines

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.

As to what to do on the push side, I suppose we can afford to let it simmer in the back of our heads while moving this topic forward. Whether we'd later decide to have receive.writeCommitGraph (in which case we would add transfer.writeCommitGraph, too) or not, this change on the fetch side can independently be used, right?

Quoted 5 lines · Junio C Hamano

Quoted 2 lines · Jeff King

I suppose so. But I think the "stock git without any other job infrastructure" case would still benefit.

Quoted 8 lines

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.

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.