Maintenance builtin, allowing 'gc --auto' customization
v3 cover Maintenance builtin, allowing 'gc --auto' customization
In reply to Derrick Stolee via GitGitGadgetThis is a second attempt at redesigning Git's repository maintenance patterns. The first attempt [1] included a way to run jobs in the background using a long-lived process; that idea was rejected and is not included in this series. A future series will use the OS to handle scheduling tasks.
[1]
https://lore.kernel.org/git/[email protected]/
As mentioned before, git gc already plays the role of maintaining Git repositories. It has accumulated several smaller pieces in its long history, including:
1. Repacking all reachable objects into one pack-file (and deleting unreachable objects).
2. Packing refs.
3. Expiring reflogs.
4. Clearing rerere logs.
5. Updating the commit-graph file.
6. Pruning worktrees.
While expiring reflogs, clearing rererelogs, and deleting unreachable objects are suitable under the guise of "garbage collection", packing refs and updating the commit-graph file are not as obviously fitting. Further, these operations are "all or nothing" in that they rewrite almost all repository data, which does not perform well at extremely large scales. These operations can also be disruptive to foreground Git commands when git gc --auto triggers during routine use.
This series does not intend to change what git gc does, but instead create new choices for automatic maintenance activities, of which git gc remains the only one enabled by default.
The new maintenance tasks are:
* 'commit-graph' : write and verify a single layer of an incremental commit-graph.
* 'loose-objects' : prune packed loose objects, then create a new pack from a batch of loose objects.
* 'pack-files' : expire redundant packs from the multi-pack-index, then repack using the multi-pack-index's incremental repack strategy.
* 'fetch' : fetch from each remote, storing the refs in 'refs/prefetch//'.
These tasks are all disabled by default, but can be enabled with config options or run explicitly using "git maintenance run --task=". There are additional config options to allow customizing the conditions for which the tasks run during the '--auto' option. ('fetch' will never run with the '--auto' option.)
Because 'gc' is implemented as a maintenance task, the most dramatic change
of this series is to convert the 'git gc --auto' calls into 'git maintenance run --auto' calls at the end of some Git commands. By default, the only change is that 'git gc --auto' will be run below an additional 'git maintenance' process.
The 'git maintenance' builtin has a 'run' subcommand so it can be extended later with subcommands that manage background maintenance, such as 'start', 'stop', 'pause', or 'schedule'. These are not the subject of this series, as it is important to focus on the maintenance activities themselves.
An expert user could set up scheduled background maintenance themselves with the current series. I have the following crontab data set up to run maintenance on an hourly basis:
0 * * * * git -C /<path-to-repo> maintenance run --no-quiet >>/<path-to-repo>/.git/maintenance.log
My config includes all tasks except the 'gc' task. The hourly run is over-aggressive, but is sufficient for testing. I'll replace it with daily when I feel satisfied.
Hopefully this direction is seen as a positive one. My goal was to add more options for expert users, along with the flexibility to create background maintenance via the OS in a later series.
OUTLINE
=======
Patches 1-4 remove some references to the_repository in builtin/gc.c before we start depending on code in that builtin.
Patches 5-7 create the 'git maintenance run' builtin and subcommand as a simple shim over 'git gc' and replaces calls to 'git gc --auto' from other commands.
Patches 8-15 create new maintenance tasks. These are the same tasks sent in the previous RFC.
Patches 16-21 create more customization through config and perform other polish items.
FUTURE WORK
===========
* Add 'start', 'stop', and 'schedule' subcommands to initialize the commands run in the background. You can see my progress towards this goal
here: https://github.com/gitgitgadget/git/pull/680
* Split the 'gc' builtin into smaller maintenance tasks that are enabled by default, but might have different '--auto' conditions and more config options.
* Replace config like 'gc.writeCommitGraph' and 'fetch.writeCommitGraph' with use of the 'commit-graph' task.
* Update the builtin to use struct repository *r properly, especially when calling subcommands.
UPDATES in V2
=============
I'm sending this between v2.28.0-rc2 adn v2.28.0 as the release things have become a bit quiet.
* The biggest disruption to the range-diff is that I removed the premature use of struct repository *r and instead continue to rely on the_repository. This means several patches were dropped that did prep work in builtin/gc.c.
* I dropped the task hashmap and opted for a linear scan. This task list will always be too small to justify the extra complication of the hashmap.
* struct maintenance_opts is properly static now.
* Some tasks are renamed: fetch -> prefetch, pack-files -> incremental-repack.
* With the rename, the prefetch task uses refs/prefetch/ instead of refs/hidden/.
* A trace2 region around the task executions are added.
UPDATES in V3
=============
* This series is now based on jk/strvec, as there are several places where I was adding new callers to argv_array_push* and run_command_v_opt() which have been replaced with strvec_push*() and run_command(), using a 'struct child_process'.
* I added and updates Junio's patch from jc/no-update-fetch-head into the proper place before the 'prefetch' task. The 'prefetch' task now uses --no-write-fetch-head to avoid issues with FETCH_HEAD.
* Since there were concerns around core.multiPackIndex, I added some extra care checking for that config being enabled. Since that already needed to adjust the config value from its existing implementation in midx.c, I added it to repo-settings and made it enabled by default.
* Lots of feedback from the previous round. Thanks, all! I fully expect to have at least one more round of feedback, but things are improving quite a lot.
Thanks, -Stolee
Derrick Stolee (19):
maintenance: create basic maintenance runner
maintenance: add --quiet option
maintenance: replace run_auto_gc()
maintenance: initialize task array
maintenance: add commit-graph task
maintenance: add --task option
maintenance: take a lock on the objects directory
maintenance: add prefetch task
maintenance: add loose-objects task
midx: enable core.multiPackIndex by default
maintenance: add incremental-repack task
maintenance: auto-size incremental-repack batch
maintenance: create maintenance.<task>.enabled config
maintenance: use pointers to check --auto
maintenance: add auto condition for commit-graph task
maintenance: create auto condition for loose-objects
maintenance: add incremental-repack auto condition
midx: use start_delayed_progress()
maintenance: add trace2 regions for task execution
Junio C Hamano (1):
fetch: optionally allow disabling FETCH_HEAD update
.gitignore | 1 + Documentation/config.txt | 2 + Documentation/config/core.txt | 4 +- Documentation/config/fetch.txt | 7 + Documentation/config/maintenance.txt | 32 ++ Documentation/fetch-options.txt | 16 +- Documentation/git-clone.txt | 6 +- Documentation/git-maintenance.txt | 127 +++++ builtin.h | 1 + builtin/am.c | 2 +- builtin/commit.c | 2 +- builtin/fetch.c | 25 +- builtin/gc.c | 713 +++++++++++++++++++++++++++ builtin/merge.c | 2 +- builtin/pull.c | 3 +- builtin/rebase.c | 4 +- commit-graph.c | 8 +- commit-graph.h | 1 + git.c | 1 + midx.c | 23 +- midx.h | 1 + object.h | 1 + repo-settings.c | 6 + repository.h | 2 + run-command.c | 16 +- run-command.h | 2 +- t/t5319-multi-pack-index.sh | 15 +- t/t5510-fetch.sh | 41 +- t/t5514-fetch-multiple.sh | 2 +- t/t5521-pull-options.sh | 16 + t/t7900-maintenance.sh | 215 ++++++++ 31 files changed, 1240 insertions(+), 57 deletions(-) create mode 100644 Documentation/config/maintenance.txt create mode 100644 Documentation/git-maintenance.txt create mode 100755 t/t7900-maintenance.sh
base-commit: 90dfbf331c0ade4c15c74276c466e32e384f9ceb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-671%2Fderrickstolee%2Fmaintenance%2Fgc-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-671/derrickstolee/maintenance/gc-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/671
Range-diff vs v2:
1: 63ec602a07 ! 1: 12fe73bb72 maintenance: create basic maintenance runner
@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
+
+static int maintenance_task_gc(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
++ struct child_process child = CHILD_PROCESS_INIT;
+
-+ argv_array_pushl(&cmd, "gc", NULL);
++ child.git_cmd = 1;
++ strvec_push(&child.args, "gc");
+
+ if (opts.auto_flag)
-+ argv_array_pushl(&cmd, "--auto", NULL);
++ strvec_push(&child.args, "--auto");
+
+ close_object_store(the_repository->objects);
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
-+
-+ return result;
++ return run_command(&child);
+}
+
+static int maintenance_run(void)
@@ t/t7900-maintenance.sh (new)
+
+test_description='git maintenance builtin'
+
-+GIT_TEST_COMMIT_GRAPH=0
-+GIT_TEST_MULTI_PACK_INDEX=0
-+
+. ./test-lib.sh
+
+test_expect_success 'help text' '
-+ test_must_fail git maintenance -h 2>err &&
++ test_expect_code 129 git maintenance -h 2>err &&
+ test_i18ngrep "usage: git maintenance run" err
+'
+
-+test_expect_success 'gc [--auto]' '
++test_expect_success 'run [--auto]' '
+ GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run &&
+ GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto &&
+ grep ",\"gc\"]" run-no-auto.txt &&
2: 1d37e55cb7 ! 2: 6e533e43d7 maintenance: add --quiet option
@@ builtin/gc.c: static const char * const builtin_maintenance_usage[] = {
@@ builtin/gc.c: static int maintenance_task_gc(void) if (opts.auto_flag)
- argv_array_pushl(&cmd, "--auto", NULL);
+ strvec_push(&child.args, "--auto");
+ if (opts.quiet)
-+ argv_array_pushl(&cmd, "--quiet", NULL);
++ strvec_push(&child.args, "--quiet");
close_object_store(the_repository->objects);
- result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
+ return run_command(&child);
@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
static struct option builtin_maintenance_options[] = {
OPT_BOOL(0, "auto", &opts.auto_flag,
@@ t/t7900-maintenance.sh: test_expect_success 'help text' '
test_i18ngrep "usage: git maintenance run" err
'
--test_expect_success 'gc [--auto]' '
+-test_expect_success 'run [--auto]' '
- GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run &&
-+test_expect_success 'gc [--auto|--quiet]' '
++test_expect_success 'run [--auto|--quiet]' '
+ GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run --no-quiet &&
GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto &&
+ GIT_TRACE2_EVENT="$(pwd)/run-quiet.txt" git maintenance run --quiet && 3: f164d1a0b4 ! 3: c4674fc211 maintenance: replace run_auto_gc()
@@ Commit message
Rename run_auto_gc() to run_auto_maintenance() to be clearer what is happening on this call, and to expose all callers in the current diff.
+ Rewrite the method to use a struct child_process to simplify the calls
+ slightly.
Since 'git fetch' already allows disabling the 'git gc --auto' subprocess, add an equivalent option with a different name to be more
@@ Documentation/fetch-options.txt: ifndef::git-pull[]
Allow several <repository> and <group> arguments to be specified. No <refspec>s may be specified.
-+--[no-]maintenance::
++--[no-]auto-maintenance::
--[no-]auto-gc::
- Run git gc --auto at the end to perform garbage collection
- if needed. This is enabled by default.
-+ Run git maintenance run --auto at the end to perform garbage
-+ collection if needed. This is enabled by default.
++ Run git maintenance run --auto at the end to perform automatic
++ repository maintenance if needed. (--[no-]auto-gc is a synonym.)
++ This is enabled by default.
--[no-]write-commit-graph::
Write a commit-graph after fetching. This overrides the config
@@ Documentation/git-clone.txt: repository using this option and then delete branch
-which automatically call git gc --auto. (See linkgit:git-gc[1].) -If these objects are removed and were referenced by the cloned repository, -then the cloned repository will become corrupt.
-+which automatically call git maintenance run --auto and git gc --auto. -+(See linkgit:git-maintenance[1] and linkgit:git-gc[1].) If these objects -+are removed and were referenced by the cloned repository, then the cloned -+repository will become corrupt.
++which automatically call git maintenance run --auto. (See ++linkgit:git-maintenance[1].) If these objects are removed and were referenced ++by the cloned repository, then the cloned repository will become corrupt.
+
Note that running git repack without the --local option in a repository cloned with --shared will copy objects from the source repository into a pack
@@ builtin/fetch.c: static struct option builtin_fetch_options[] = {
OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
N_("report that we have only objects reachable from this object")),
OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
-+ OPT_BOOL(0, "maintenance", &enable_auto_gc,
++ OPT_BOOL(0, "auto-maintenance", &enable_auto_gc,
+ N_("run 'maintenance --auto' after fetching")),
OPT_BOOL(0, "auto-gc", &enable_auto_gc,
- N_("run 'gc --auto' after fetching")),
@@ run-command.c: int run_processes_parallel_tr2(int n, get_next_task_fn get_next_t
-int run_auto_gc(int quiet)
+int run_auto_maintenance(int quiet)
{
- struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
- int status;
+- struct strvec argv_gc_auto = STRVEC_INIT;
+- int status;
++ struct child_process maint = CHILD_PROCESS_INIT;
-- argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
-+ argv_array_pushl(&argv_gc_auto, "maintenance", "run", "--auto", NULL);
- if (quiet)
- argv_array_push(&argv_gc_auto, "--quiet");
-+ else
-+ argv_array_push(&argv_gc_auto, "--no-quiet");
+- strvec_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+- if (quiet)
+- strvec_push(&argv_gc_auto, "--quiet");
+- status = run_command_v_opt(argv_gc_auto.items, RUN_GIT_CMD);
+- strvec_clear(&argv_gc_auto);
+- return status;
++ maint.git_cmd = 1;
++ strvec_pushl(&maint.args, "maintenance", "run", "--auto", NULL);
++ strvec_push(&maint.args, quiet ? "--quiet" : "--no-quiet");
+
- status = run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
- argv_array_clear(&argv_gc_auto);
- return status;
++ return run_command(&maint);
+ }
## run-command.h ##
@@ run-command.h: int run_hook_ve(const char *const *env, const char *name, va_list args);
4: 8e260bccf1 ! 4: b9332c1318 maintenance: initialize task array
@@ Commit message
future command-line argument) along with a function pointer to its
implementation and a boolean for whether the step is enabled.
- A list of pointers to these structs are initialized with the full list
- of implemented tasks along with a default order. For now, this list only
- contains the "gc" task. This task is also the only task enabled by
- default.
+ A list these structs are initialized with the full list of implemented
+ tasks along with a default order. For now, this list only contains the
+ "gc" task. This task is also the only task enabled by default.
+ The run subcommand will return a nonzero exit code if any task fails.
+ However, it will attempt all tasks in its loop before returning with the
+ failure. Also each failed task will send an error message.
+
+ Helped-by: Taylor Blau <[email protected]>
+ Helped-by: Junio C Hamano <[email protected]>
Signed-off-by: Derrick Stolee <[email protected]>
## builtin/gc.c ##
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- return 0;
- }
-
-+#define MAX_NUM_TASKS 1
-+
- static const char * const builtin_maintenance_usage[] = {
- N_("git maintenance run [<options>]"),
- NULL
@@ builtin/gc.c: static int maintenance_task_gc(void)
- return result;
+ return run_command(&child);
}
+typedef int maintenance_task_fn(void);
@@ builtin/gc.c: static int maintenance_task_gc(void)
+ unsigned enabled:1;
+};
+
-+static struct maintenance_task *tasks[MAX_NUM_TASKS];
-+static int num_tasks;
++enum maintenance_task_label {
++ TASK_GC,
++
++ /* Leave as final value */
++ TASK__COUNT
++};
++
++static struct maintenance_task tasks[] = {
++ [TASK_GC] = {
++ "gc",
++ maintenance_task_gc,
++ 1,
++ },
++};
+
static int maintenance_run(void)
{
@@ builtin/gc.c: static int maintenance_task_gc(void)
+ int i;
+ int result = 0;
+
-+ for (i = 0; !result && i < num_tasks; i++) {
-+ if (!tasks[i]->enabled)
++ for (i = 0; i < TASK__COUNT; i++) {
++ if (!tasks[i].enabled)
+ continue;
-+ result = tasks[i]->fn();
++
++ if (tasks[i].fn()) {
++ error(_("task '%s' failed"), tasks[i].name);
++ result = 1;
++ }
+ }
+
+ return result;
-+}
-+
-+static void initialize_tasks(void)
-+{
-+ int i;
-+ num_tasks = 0;
-+
-+ for (i = 0; i < MAX_NUM_TASKS; i++)
-+ tasks[i] = xcalloc(1, sizeof(struct maintenance_task));
-+
-+ tasks[num_tasks]->name = "gc";
-+ tasks[num_tasks]->fn = maintenance_task_gc;
-+ tasks[num_tasks]->enabled = 1;
-+ num_tasks++;
}
int cmd_maintenance(int argc, const char **argv, const char *prefix)
-@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- builtin_maintenance_options);
-
- opts.quiet = !isatty(2);
-+ initialize_tasks();
-
- argc = parse_options(argc, argv, prefix,
- builtin_maintenance_options,
5: 04552b1d2e ! 5: a4d9836bed maintenance: add commit-graph task
@@ Documentation/git-maintenance.txt: run::
stands for "garbage collection," but this task performs many
## builtin/gc.c ##
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- return 0;
- }
-
--#define MAX_NUM_TASKS 1
-+#define MAX_NUM_TASKS 2
-
- static const char * const builtin_maintenance_usage[] = {
- N_("git maintenance run [<options>]"),
@@ builtin/gc.c: static struct maintenance_opts {
int quiet;
} opts;
+static int run_write_commit_graph(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
++ struct child_process child = CHILD_PROCESS_INIT;
+
-+ argv_array_pushl(&cmd, "commit-graph", "write",
-+ "--split", "--reachable", NULL);
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "commit-graph", "write",
++ "--split", "--reachable", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
-+
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
++ strvec_push(&child.args, "--no-progress");
+
-+ return result;
++ return !!run_command(&child);
+}
+
+static int run_verify_commit_graph(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
++ struct child_process child = CHILD_PROCESS_INIT;
+
-+ argv_array_pushl(&cmd, "commit-graph", "verify",
-+ "--shallow", NULL);
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "commit-graph", "verify",
++ "--shallow", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
++ strvec_push(&child.args, "--no-progress");
+
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
-+
-+ return result;
++ return !!run_command(&child);
+}
+
+static int maintenance_task_commit_graph(void)
@@ builtin/gc.c: static struct maintenance_opts {
+ struct repository *r = the_repository;
+ char *chain_path;
+
-+ /* Skip commit-graph when --auto is specified. */
-+ if (opts.auto_flag)
-+ return 0;
-+
+ close_object_store(r->objects);
+ if (run_write_commit_graph()) {
+ error(_("failed to write commit-graph"));
@@ builtin/gc.c: static struct maintenance_opts {
+
static int maintenance_task_gc(void)
{
- int result;
-@@ builtin/gc.c: static void initialize_tasks(void)
- tasks[num_tasks]->fn = maintenance_task_gc;
- tasks[num_tasks]->enabled = 1;
- num_tasks++;
-+
-+ tasks[num_tasks]->name = "commit-graph";
-+ tasks[num_tasks]->fn = maintenance_task_commit_graph;
-+ num_tasks++;
- }
+ struct child_process child = CHILD_PROCESS_INIT;
+@@ builtin/gc.c: struct maintenance_task {
+
+ enum maintenance_task_label {
+ TASK_GC,
++ TASK_COMMIT_GRAPH,
- int cmd_maintenance(int argc, const char **argv, const char *prefix)
+ /* Leave as final value */
+ TASK__COUNT
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ maintenance_task_gc,
+ 1,
+ },
++ [TASK_COMMIT_GRAPH] = {
++ "commit-graph",
++ maintenance_task_commit_graph,
++ },
+ };
+
+ static int maintenance_run(void)
## commit-graph.c ##
@@ commit-graph.c: static char *get_split_graph_filename(struct object_directory *odb,
@@ commit-graph.h: struct commit;
/*
## t/t7900-maintenance.sh ##
-@@ t/t7900-maintenance.sh: test_expect_success 'help text' '
- test_i18ngrep "usage: git maintenance run" err
- '
+@@ t/t7900-maintenance.sh: test_description='git maintenance builtin'
--test_expect_success 'gc [--auto|--quiet]' '
-+test_expect_success 'run [--auto|--quiet]' '
- GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run --no-quiet &&
- GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto &&
- GIT_TRACE2_EVENT="$(pwd)/run-quiet.txt" git maintenance run --quiet &&
+ . ./test-lib.sh
+
++GIT_TEST_COMMIT_GRAPH=0
++
+ test_expect_success 'help text' '
+ test_expect_code 129 git maintenance -h 2>err &&
+ test_i18ngrep "usage: git maintenance run" err
6: a09b1c1687 ! 6: dafb0d9bbc maintenance: add --task option
@@ Commit message
references. We use the hashmap to match the --task=<task> arguments into
the task struct data.
+ Keep in mind that the 'enabled' member of the maintenance_task struct is
+ a placeholder for a future 'maintenance.<task>.enabled' config option.
+ Thus, we use the 'enabled' member to specify which tasks are run when
+ the user does not specify any --task=<task> arguments. The 'enabled'
+ member should be ignored if --task=<task> appears.
+
Signed-off-by: Derrick Stolee <[email protected]>
## Documentation/git-maintenance.txt ##
@@ builtin/gc.c: typedef int maintenance_task_fn(void);
const char *name;
maintenance_task_fn *fn;
- unsigned enabled:1;
-+ int task_order;
+ unsigned enabled:1,
+ selected:1;
++ int selected_order;
};
- static struct maintenance_task *tasks[MAX_NUM_TASKS];
- static int num_tasks;
+ enum maintenance_task_label {
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ },
+ };
+static int compare_tasks_by_selection(const void *a_, const void *b_)
+{
+ const struct maintenance_task *a, *b;
-+ a = (const struct maintenance_task *)a_;
-+ b = (const struct maintenance_task *)b_;
+
-+ return b->task_order - a->task_order;
++ a = (const struct maintenance_task *)&a_;
++ b = (const struct maintenance_task *)&b_;
++
++ return b->selected_order - a->selected_order;
+}
+
static int maintenance_run(void)
@@ builtin/gc.c: typedef int maintenance_task_fn(void);
int result = 0;
+ if (opts.tasks_selected)
-+ QSORT(tasks, num_tasks, compare_tasks_by_selection);
++ QSORT(tasks, TASK__COUNT, compare_tasks_by_selection);
+
- for (i = 0; !result && i < num_tasks; i++) {
-- if (!tasks[i]->enabled)
-+ if (opts.tasks_selected && !tasks[i]->selected)
+ for (i = 0; i < TASK__COUNT; i++) {
+- if (!tasks[i].enabled)
++ if (opts.tasks_selected && !tasks[i].selected)
+ continue;
+
-+ if (!opts.tasks_selected && !tasks[i]->enabled)
++ if (!opts.tasks_selected && !tasks[i].enabled)
continue;
-+
- result = tasks[i]->fn();
- }
-@@ builtin/gc.c: static void initialize_tasks(void)
- num_tasks++;
+ if (tasks[i].fn()) {
+@@ builtin/gc.c: static int maintenance_run(void)
+ return result;
}
+static int task_option_parse(const struct option *opt,
@@ builtin/gc.c: static void initialize_tasks(void)
+
+ BUG_ON_OPT_NEG(unset);
+
-+ if (!arg || !strlen(arg)) {
-+ error(_("--task requires a value"));
-+ return 1;
-+ }
-+
+ opts.tasks_selected++;
+
-+ for (i = 0; i < MAX_NUM_TASKS; i++) {
-+ if (tasks[i] && !strcasecmp(tasks[i]->name, arg)) {
-+ task = tasks[i];
++ for (i = 0; i < TASK__COUNT; i++) {
++ if (!strcasecmp(tasks[i].name, arg)) {
++ task = &tasks[i];
+ break;
+ }
+ }
@@ builtin/gc.c: static void initialize_tasks(void)
+ }
+
+ task->selected = 1;
-+ task->task_order = opts.tasks_selected;
++ task->selected_order = opts.tasks_selected;
+
+ return 0;
+} 7: e9260a9c3f ! 7: 1b00524da3 maintenance: take a lock on the objects directory
@@ builtin/gc.c: static int maintenance_run(void)
+ free(lock_path);
if (opts.tasks_selected)
- QSORT(tasks, num_tasks, compare_tasks_by_selection);
+ QSORT(tasks, TASK__COUNT, compare_tasks_by_selection);
@@ builtin/gc.c: static int maintenance_run(void)
- result = tasks[i]->fn();
+ }
}
+ rollback_lock_file(&lk);
-: ---------- > 8: 0e94e04dcd fetch: optionally allow disabling FETCH_HEAD update 8: 3165b8916d ! 9: 9e38ade15c maintenance: add prefetch task
@@ Commit message
2. --refmap= removes the configured refspec which usually updates refs/remotes/<remote>/* with the refs advertised by the remote.
+ While this looks confusing, this was documented and tested by
+ b40a50264ac (fetch: document and test --refmap="", 2020-01-21),
+ including this sentence in the documentation:
+
+ Providing an empty <refspec> to the --refmap option
+ causes Git to ignore the configured refspecs and rely
+ entirely on the refspecs supplied as command-line arguments.
3. By adding a new refspec "+refs/heads/*:refs/prefetch/<remote>/*" we can ensure that we actually load the new values somewhere in
@@ Commit message
4. --prune will delete the refs/prefetch/<remote> refs that no longer appear on the remote.
+ 5. --no-write-fetch-head prevents updating FETCH_HEAD.
+
We've been using this step as a critical background job in Scalar
[1] (and VFS for Git). This solved a pain point that was showing up in user reports: fetching was a pain! Users do not like waiting to
@@ Documentation/git-maintenance.txt: since it will not expire .graph files that
the expiration delay.
+prefetch::
-+ The fetch task updates the object directory with the latest objects
-+ from all registered remotes. For each remote, a git fetch command
-+ is run. The refmap is custom to avoid updating local or remote
++ The prefetch task updates the object directory with the latest
++ objects from all registered remotes. For each remote, a git fetch
++ command is run. The refmap is custom to avoid updating local or remote
+ branches (those in refs/heads or refs/remotes). Instead, the
+ remote refs are stored in refs/prefetch/<remote>/. Also, tags are
+ not updated.
++
-+This means that foreground fetches are still required to update the -+remote refs, but the users is notified when the branches and tags are -+updated on the remote.
++This is done to avoid disrupting the remote-tracking branches. The end users ++expect these refs to stay unmoved unless they initiate a fetch. With prefetch ++task, however, the objects necessary to complete a later real fetch would ++already be obtained, so the real fetch would go faster. In the ideal case, ++it will just become an update to bunch of remote-tracking branches without ++any object transfer.
+
gc::
Cleanup unnecessary files and optimize the local repository. "GC"
@@ builtin/gc.c
#define FAILED_RUN "failed to run %s"
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- return 0;
- }
-
--#define MAX_NUM_TASKS 2
-+#define MAX_NUM_TASKS 3
-
- static const char * const builtin_maintenance_usage[] = {
- N_("git maintenance run [<options>]"),
@@ builtin/gc.c: static int maintenance_task_commit_graph(void)
return 1;
}
+static int fetch_remote(const char *remote)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ struct strbuf refmap = STRBUF_INIT;
++ struct child_process child = CHILD_PROCESS_INIT;
+
-+ argv_array_pushl(&cmd, "fetch", remote, "--prune",
-+ "--no-tags", "--refmap=", NULL);
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "fetch", remote, "--prune", "--no-tags",
++ "--no-write-fetch-head", "--refmap=", NULL);
+
-+ strbuf_addf(&refmap, "+refs/heads/*:refs/prefetch/%s/*", remote);
-+ argv_array_push(&cmd, refmap.buf);
++ strvec_pushf(&child.args, "+refs/heads/*:refs/prefetch/%s/*", remote);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--quiet");
++ strvec_push(&child.args, "--quiet");
+
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+
-+ strbuf_release(&refmap);
-+ return result;
++ return !!run_command(&child);
+}
+
+static int fill_each_remote(struct remote *remote, void *cbdata)
@@ builtin/gc.c: static int maintenance_task_commit_graph(void)
+ goto cleanup;
+ }
+
-+ /*
-+ * Do not modify the result based on the success of the 'fetch'
-+ * operation, as a loss of network could cause 'fetch' to fail
-+ * quickly. We do not want that to stop the rest of our
-+ * background operations.
-+ */
+ for (item = remotes.items;
+ item && item < remotes.items + remotes.nr;
+ item++)
-+ fetch_remote(item->string);
++ result |= fetch_remote(item->string);
+
+cleanup:
+ string_list_clear(&remotes, 0);
@@ builtin/gc.c: static int maintenance_task_commit_graph(void)
+
static int maintenance_task_gc(void)
{
- int result;
-@@ builtin/gc.c: static void initialize_tasks(void)
- for (i = 0; i < MAX_NUM_TASKS; i++)
- tasks[i] = xcalloc(1, sizeof(struct maintenance_task));
+ struct child_process child = CHILD_PROCESS_INIT;
+@@ builtin/gc.c: struct maintenance_task {
+ };
-+ tasks[num_tasks]->name = "prefetch";
-+ tasks[num_tasks]->fn = maintenance_task_prefetch;
-+ num_tasks++;
-+
- tasks[num_tasks]->name = "gc";
- tasks[num_tasks]->fn = maintenance_task_gc;
- tasks[num_tasks]->enabled = 1;
+ enum maintenance_task_label {
++ TASK_PREFETCH,
+ TASK_GC,
+ TASK_COMMIT_GRAPH,
+
+@@ builtin/gc.c: enum maintenance_task_label {
+ };
+
+ static struct maintenance_task tasks[] = {
++ [TASK_PREFETCH] = {
++ "prefetch",
++ maintenance_task_prefetch,
++ },
+ [TASK_GC] = {
+ "gc",
+ maintenance_task_gc,
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'run --task duplicate' '
9: 83648f4865 ! 10: 0128fdfd1a maintenance: add loose-objects task
@@ Documentation/git-maintenance.txt: gc::
--auto::
## builtin/gc.c ##
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- return 0;
- }
-
--#define MAX_NUM_TASKS 3
-+#define MAX_NUM_TASKS 4
-
- static const char * const builtin_maintenance_usage[] = {
- N_("git maintenance run [<options>]"),
@@ builtin/gc.c: static int maintenance_task_gc(void)
- return result;
+ return run_command(&child);
}
+static int prune_packed(void)
+{
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ argv_array_pushl(&cmd, "prune-packed", NULL);
++ struct child_process child = CHILD_PROCESS_INIT;
++
++ child.git_cmd = 1;
++ strvec_push(&child.args, "prune-packed");
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--quiet");
++ strvec_push(&child.args, "--quiet");
+
-+ return run_command_v_opt(cmd.argv, RUN_GIT_CMD);
++ return !!run_command(&child);
+}
+
+struct write_loose_object_data {
@@ builtin/gc.c: static int maintenance_task_gc(void)
+ int batch_size;
+};
+
-+static int loose_object_exists(const struct object_id *oid,
-+ const char *path,
-+ void *data)
++static int bail_on_loose(const struct object_id *oid,
++ const char *path,
++ void *data)
+{
+ return 1;
+}
@@ builtin/gc.c: static int maintenance_task_gc(void)
+ struct repository *r = the_repository;
+ int result = 0;
+ struct write_loose_object_data data;
-+ struct strbuf prefix = STRBUF_INIT;
-+ struct child_process *pack_proc;
++ struct child_process pack_proc = CHILD_PROCESS_INIT;
+
+ /*
+ * Do not start pack-objects process
+ * if there are no loose objects.
+ */
+ if (!for_each_loose_file_in_objdir(r->objects->odb->path,
-+ loose_object_exists,
++ bail_on_loose,
+ NULL, NULL, NULL))
+ return 0;
+
-+ pack_proc = xmalloc(sizeof(*pack_proc));
-+
-+ child_process_init(pack_proc);
++ pack_proc.git_cmd = 1;
+
-+ strbuf_addstr(&prefix, r->objects->odb->path);
-+ strbuf_addstr(&prefix, "/pack/loose");
-+
-+ argv_array_pushl(&pack_proc->args, "git", "pack-objects", NULL);
++ strvec_push(&pack_proc.args, "pack-objects");
+ if (opts.quiet)
-+ argv_array_push(&pack_proc->args, "--quiet");
-+ argv_array_push(&pack_proc->args, prefix.buf);
++ strvec_push(&pack_proc.args, "--quiet");
++ strvec_pushf(&pack_proc.args, "%s/pack/loose", r->objects->odb->path);
+
-+ pack_proc->in = -1;
++ pack_proc.in = -1;
+
-+ if (start_command(pack_proc)) {
++ if (start_command(&pack_proc)) {
+ error(_("failed to start 'git pack-objects' process"));
-+ result = 1;
-+ goto cleanup;
++ return 1;
+ }
+
-+ data.in = xfdopen(pack_proc->in, "w");
++ data.in = xfdopen(pack_proc.in, "w");
+ data.count = 0;
+ data.batch_size = 50000;
+
@@ builtin/gc.c: static int maintenance_task_gc(void)
+
+ fclose(data.in);
+
-+ if (finish_command(pack_proc)) {
++ if (finish_command(&pack_proc)) {
+ error(_("failed to finish 'git pack-objects' process"));
+ result = 1;
+ }
+
-+cleanup:
-+ strbuf_release(&prefix);
-+ free(pack_proc);
+ return result;
+}
+
@@ builtin/gc.c: static int maintenance_task_gc(void)
typedef int maintenance_task_fn(void);
struct maintenance_task {
-@@ builtin/gc.c: static void initialize_tasks(void)
- tasks[num_tasks]->fn = maintenance_task_prefetch;
- num_tasks++;
+@@ builtin/gc.c: struct maintenance_task {
-+ tasks[num_tasks]->name = "loose-objects";
-+ tasks[num_tasks]->fn = maintenance_task_loose_objects;
-+ num_tasks++;
-+
- tasks[num_tasks]->name = "gc";
- tasks[num_tasks]->fn = maintenance_task_gc;
- tasks[num_tasks]->enabled = 1;
+ enum maintenance_task_label {
+ TASK_PREFETCH,
++ TASK_LOOSE_OBJECTS,
+ TASK_GC,
+ TASK_COMMIT_GRAPH,
+
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ "prefetch",
+ maintenance_task_prefetch,
+ },
++ [TASK_LOOSE_OBJECTS] = {
++ "loose-objects",
++ maintenance_task_loose_objects,
++ },
+ [TASK_GC] = {
+ "gc",
+ maintenance_task_gc,
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'prefetch multiple remotes' '
@@ t/t7900-maintenance.sh: test_expect_success 'prefetch multiple remotes' '
+
+ # Hack to stop maintenance from running during "git commit"
+ echo in use >.git/objects/maintenance.lock &&
++
++ # Assuming that "git commit" creates at least one loose object
+ test_commit create-loose-object &&
+ rm .git/objects/maintenance.lock &&
+
@@ t/t7900-maintenance.sh: test_expect_success 'prefetch multiple remotes' '
+ test_cmp obj-dir-before obj-dir-between &&
+ ls .git/objects/pack/*.pack >packs-between &&
+ test_line_count = 2 packs-between &&
++ ls .git/objects/pack/loose-*.pack >loose-packs &&
++ test_line_count = 1 loose-packs &&
+
+ # The second run deletes loose objects
+ # but does not create a pack-file.
-: ---------- > 11: c2baf6e119 midx: enable core.multiPackIndex by default
10: b6328c2106 ! 12: 00f47c4848 maintenance: add incremental-repack task
@@ Commit message
size" is calculated by taking the size of the pack-file divided
by the number of objects in the pack-file and multiplied by the
number of objects from the multi-pack-index with offset in that
- pack-file. The expected size approximats how much data from that
+ pack-file. The expected size approximates how much data from that
pack-file will contribute to the resulting pack-file size. The
intention is that the resulting pack-file will be close in size
to the provided batch size.
@@ builtin/gc.c
#define FAILED_RUN "failed to run %s"
-@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
- return 0;
- }
-
--#define MAX_NUM_TASKS 4
-+#define MAX_NUM_TASKS 5
-
- static const char * const builtin_maintenance_usage[] = {
- N_("git maintenance run [<options>]"),
@@ builtin/gc.c: static int maintenance_task_loose_objects(void)
return prune_packed() || pack_loose();
}
+static int multi_pack_index_write(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ argv_array_pushl(&cmd, "multi-pack-index", "write", NULL);
++ struct child_process child = CHILD_PROCESS_INIT;
++
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "multi-pack-index", "write", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
++ strvec_push(&child.args, "--no-progress");
+
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
++ if (run_command(&child))
++ return error(_("failed to write multi-pack-index"));
+
-+ return result;
++ return 0;
+}
+
+static int rewrite_multi_pack_index(void)
@@ builtin/gc.c: static int maintenance_task_loose_objects(void)
+ unlink(midx_name);
+ free(midx_name);
+
-+ if (multi_pack_index_write()) {
-+ error(_("failed to rewrite multi-pack-index"));
-+ return 1;
-+ }
-+
-+ return 0;
++ return multi_pack_index_write();
+}
+
-+static int multi_pack_index_verify(void)
++static int multi_pack_index_verify(const char *message)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ argv_array_pushl(&cmd, "multi-pack-index", "verify", NULL);
++ struct child_process child = CHILD_PROCESS_INIT;
++
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "multi-pack-index", "verify", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
++ strvec_push(&child.args, "--no-progress");
+
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
++ if (run_command(&child)) {
++ warning(_("'git multi-pack-index verify' failed %s"), message);
++ return 1;
++ }
+
-+ return result;
++ return 0;
+}
+
+static int multi_pack_index_expire(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ argv_array_pushl(&cmd, "multi-pack-index", "expire", NULL);
++ struct child_process child = CHILD_PROCESS_INIT;
++
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "multi-pack-index", "expire", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
++ strvec_push(&child.args, "--no-progress");
+
+ close_object_store(the_repository->objects);
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ argv_array_clear(&cmd);
+
-+ return result;
++ if (run_command(&child))
++ return error(_("'git multi-pack-index expire' failed"));
++
++ return 0;
+}
+
+static int multi_pack_index_repack(void)
+{
-+ int result;
-+ struct argv_array cmd = ARGV_ARRAY_INIT;
-+ argv_array_pushl(&cmd, "multi-pack-index", "repack", NULL);
++ struct child_process child = CHILD_PROCESS_INIT;
++
++ child.git_cmd = 1;
++ strvec_pushl(&child.args, "multi-pack-index", "repack", NULL);
+
+ if (opts.quiet)
-+ argv_array_push(&cmd, "--no-progress");
++ strvec_push(&child.args, "--no-progress");
+
-+ argv_array_push(&cmd, "--batch-size=0");
++ strvec_push(&child.args, "--batch-size=0");
+
+ close_object_store(the_repository->objects);
-+ result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
+
-+ if (result && multi_pack_index_verify()) {
-+ warning(_("multi-pack-index verify failed after repack"));
-+ result = rewrite_multi_pack_index();
-+ }
++ if (run_command(&child))
++ return error(_("'git multi-pack-index repack' failed"));
+
-+ return result;
++ return 0;
+}
+
+static int maintenance_task_incremental_repack(void)
+{
-+ if (multi_pack_index_write()) {
-+ error(_("failed to write multi-pack-index"));
-+ return 1;
++ prepare_repo_settings(the_repository);
++ if (!the_repository->settings.core_multi_pack_index) {
++ warning(_("skipping incremental-repack task because core.multiPackIndex is disabled"));
++ return 0;
+ }
+
-+ if (multi_pack_index_verify()) {
-+ warning(_("multi-pack-index verify failed after initial write"));
-+ return rewrite_multi_pack_index();
-+ }
-+
-+ if (multi_pack_index_expire()) {
-+ error(_("multi-pack-index expire failed"));
++ if (multi_pack_index_write())
+ return 1;
-+ }
-+
-+ if (multi_pack_index_verify()) {
-+ warning(_("multi-pack-index verify failed after expire"));
++ if (multi_pack_index_verify("after initial write"))
+ return rewrite_multi_pack_index();
-+ }
-+
-+ if (multi_pack_index_repack()) {
-+ error(_("multi-pack-index repack failed"));
++ if (multi_pack_index_expire())
+ return 1;
-+ }
-+
++ if (multi_pack_index_verify("after expire step"))
++ return !!rewrite_multi_pack_index();
++ if (multi_pack_index_repack())
++ return 1;
++ if (multi_pack_index_verify("after repack step"))
++ return !!rewrite_multi_pack_index();
+ return 0;
+}
+
typedef int maintenance_task_fn(void);
struct maintenance_task {
-@@ builtin/gc.c: static void initialize_tasks(void)
- tasks[num_tasks]->fn = maintenance_task_loose_objects;
- num_tasks++;
+@@ builtin/gc.c: struct maintenance_task {
+ enum maintenance_task_label {
+ TASK_PREFETCH,
+ TASK_LOOSE_OBJECTS,
++ TASK_INCREMENTAL_REPACK,
+ TASK_GC,
+ TASK_COMMIT_GRAPH,
-+ tasks[num_tasks]->name = "incremental-repack";
-+ tasks[num_tasks]->fn = maintenance_task_incremental_repack;
-+ num_tasks++;
-+
- tasks[num_tasks]->name = "gc";
- tasks[num_tasks]->fn = maintenance_task_gc;
- tasks[num_tasks]->enabled = 1;
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ "loose-objects",
+ maintenance_task_loose_objects,
+ },
++ [TASK_INCREMENTAL_REPACK] = {
++ "incremental-repack",
++ maintenance_task_incremental_repack,
++ },
+ [TASK_GC] = {
+ "gc",
+ maintenance_task_gc,
## midx.c ##
@@
@@ midx.h: struct multi_pack_index {
int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);
int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);
+ ## t/t5319-multi-pack-index.sh ##
+@@
+ test_description='multi-pack-indexes'
+ . ./test-lib.sh
+
++GIT_TEST_MULTI_PACK_INDEX=0
+ objdir=.git/objects
+
+ midx_read_expect () {
+
## t/t7900-maintenance.sh ##
+@@ t/t7900-maintenance.sh: test_description='git maintenance builtin'
+ . ./test-lib.sh
+
+ GIT_TEST_COMMIT_GRAPH=0
++GIT_TEST_MULTI_PACK_INDEX=0
+
+ test_expect_success 'help text' '
+ test_expect_code 129 git maintenance -h 2>err &&
@@ t/t7900-maintenance.sh: test_expect_success 'loose-objects task' '
test_cmp packs-between packs-after
' 11: 478c7f1d0b ! 13: ef2a231956 maintenance: auto-size incremental-repack batch
@@ Commit message
When repacking during the 'incremental-repack' task, we use the --batch-size option in 'git multi-pack-index repack'. The initial setting used --batch-size=0 to repack everything into a single pack-file. This is
- not sustaintable for a large repository. The amount of work required is
+ not sustainable for a large repository. The amount of work required is
also likely to use too many system resources for a background job.
Update the 'incremental-repack' task by dynamically computing a
@@ Commit message
## builtin/gc.c ##
@@ builtin/gc.c: static int multi_pack_index_expire(void)
- return result;
+ return 0;
}
-+#define TWO_GIGABYTES (2147483647)
-+#define UNSET_BATCH_SIZE ((unsigned long)-1) ++#define TWO_GIGABYTES (0x7FFF)
+
+static off_t get_auto_pack_size(void)
+{
@@ builtin/gc.c: static int multi_pack_index_expire(void)
+
static int multi_pack_index_repack(void)
{
- int result;
- struct argv_array cmd = ARGV_ARRAY_INIT;
-+ struct strbuf batch_arg = STRBUF_INIT;
-+
- argv_array_pushl(&cmd, "multi-pack-index", "repack", NULL);
-
+ struct child_process child = CHILD_PROCESS_INIT;
+@@ builtin/gc.c: static int multi_pack_index_repack(void)
if (opts.quiet)
- argv_array_push(&cmd, "--no-progress");
+ strvec_push(&child.args, "--no-progress");
-- argv_array_push(&cmd, "--batch-size=0");
-+ strbuf_addf(&batch_arg, "--batch-size=%"PRIuMAX,
-+ (uintmax_t)get_auto_pack_size());
-+ argv_array_push(&cmd, batch_arg.buf);
+- strvec_push(&child.args, "--batch-size=0");
++ strvec_pushf(&child.args, "--batch-size=%"PRIuMAX,
++ (uintmax_t)get_auto_pack_size());
close_object_store(the_repository->objects);
- result = run_command_v_opt(cmd.argv, RUN_GIT_CMD);
-+ strbuf_release(&batch_arg);
- if (result && multi_pack_index_verify()) {
- warning(_("multi-pack-index verify failed after repack"));
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'incremental-repack task' '
12: a3c64930a0 ! 14: 99840c4b8f maintenance: create maintenance.<task>.enabled config
@@ Documentation/git-maintenance.txt: SUBCOMMANDS
## builtin/gc.c ##
@@ builtin/gc.c: static int maintenance_run(void)
- static void initialize_tasks(void)
- {
- int i;
-+ struct strbuf config_name = STRBUF_INIT;
- num_tasks = 0;
+ return result;
+ }
- for (i = 0; i < MAX_NUM_TASKS; i++)
-@@ builtin/gc.c: static void initialize_tasks(void)
- tasks[num_tasks]->name = "commit-graph";
- tasks[num_tasks]->fn = maintenance_task_commit_graph;
- num_tasks++;
-+
-+ for (i = 0; i < num_tasks; i++) {
++static void initialize_task_config(void)
++{
++ int i;
++ struct strbuf config_name = STRBUF_INIT;
++ for (i = 0; i < TASK__COUNT; i++) {
+ int config_value;
+
+ strbuf_setlen(&config_name, 0);
-+ strbuf_addf(&config_name, "maintenance.%s.enabled", tasks[i]->name);
++ strbuf_addf(&config_name, "maintenance.%s.enabled",
++ tasks[i].name);
+
+ if (!git_config_get_bool(config_name.buf, &config_value))
-+ tasks[i]->enabled = config_value;
++ tasks[i].enabled = config_value;
+ }
+
+ strbuf_release(&config_name);
- }
-
++}
++
static int task_option_parse(const struct option *opt,
+ const char *arg, int unset)
+ {
+@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
+ builtin_maintenance_options);
+
+ opts.quiet = !isatty(2);
++ initialize_task_config();
+
+ argc = parse_options(argc, argv, prefix,
+ builtin_maintenance_options,
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'run [--auto|--quiet]' ' 13: dbacc2b76c ! 15: a087c63572 maintenance: use pointers to check --auto
@@ builtin/gc.c: static int maintenance_task_incremental_repack(void)
const char *name;
maintenance_task_fn *fn;
+ maintenance_auto_fn *auto_condition;
- int task_order;
unsigned enabled:1,
selected:1;
+ int selected_order;
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ [TASK_GC] = {
+ "gc",
+ maintenance_task_gc,
++ need_to_gc,
+ 1,
+ },
+ [TASK_COMMIT_GRAPH] = {
@@ builtin/gc.c: static int maintenance_run(void)
- if (!opts.tasks_selected && !tasks[i]->enabled)
+ if (!opts.tasks_selected && !tasks[i].enabled)
continue;
+ if (opts.auto_flag &&
-+ (!tasks[i]->auto_condition ||
-+ !tasks[i]->auto_condition()))
++ (!tasks[i].auto_condition ||
++ !tasks[i].auto_condition()))
+ continue;
+
- result = tasks[i]->fn();
- }
-
-@@ builtin/gc.c: static void initialize_tasks(void)
-
- tasks[num_tasks]->name = "gc";
- tasks[num_tasks]->fn = maintenance_task_gc;
-+ tasks[num_tasks]->auto_condition = need_to_gc;
- tasks[num_tasks]->enabled = 1;
- num_tasks++;
-
-@@ builtin/gc.c: int cmd_maintenance(int argc, const char **argv, const char *prefix)
- builtin_maintenance_options);
-
- opts.quiet = !isatty(2);
+ if (tasks[i].fn()) {
+ error(_("task '%s' failed"), tasks[i].name);
+ result = 1;
+@@ builtin/gc.c: static void initialize_task_config(void)
+ {
+ int i;
+ struct strbuf config_name = STRBUF_INIT;
+ gc_config();
- initialize_tasks();
++
+ for (i = 0; i < TASK__COUNT; i++) {
+ int config_value;
- argc = parse_options(argc, argv, prefix,
## t/t5514-fetch-multiple.sh ##
@@ t/t5514-fetch-multiple.sh: test_expect_success 'git fetch --multiple (two remotes)' '
14: 9af2309f08 ! 16: ef3a854508 maintenance: add auto condition for commit-graph task
@@ builtin/gc.c: static struct maintenance_opts {
+
static int run_write_commit_graph(void)
{
- int result;
-@@ builtin/gc.c: static void initialize_tasks(void)
+ struct child_process child = CHILD_PROCESS_INIT;
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ [TASK_COMMIT_GRAPH] = {
+ "commit-graph",
+ maintenance_task_commit_graph,
++ should_write_commit_graph,
+ },
+ };
- tasks[num_tasks]->name = "commit-graph";
- tasks[num_tasks]->fn = maintenance_task_commit_graph;
-+ tasks[num_tasks]->auto_condition = should_write_commit_graph;
- num_tasks++;
-
- for (i = 0; i < num_tasks; i++) {
## object.h ##
@@ object.h: struct object_array {
15: 42e316ca58 ! 17: 6ac3a58f2f maintenance: create auto condition for loose-objects
@@ builtin/gc.c: struct write_loose_object_data {
+ NULL, NULL, &count);
+}
+
- static int loose_object_exists(const struct object_id *oid,
- const char *path,
- void *data)
-@@ builtin/gc.c: static void initialize_tasks(void)
-
- tasks[num_tasks]->name = "loose-objects";
- tasks[num_tasks]->fn = maintenance_task_loose_objects;
-+ tasks[num_tasks]->auto_condition = loose_object_auto_condition;
- num_tasks++;
-
- tasks[num_tasks]->name = "incremental-repack";
+ static int bail_on_loose(const struct object_id *oid,
+ const char *path,
+ void *data)
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ [TASK_LOOSE_OBJECTS] = {
+ "loose-objects",
+ maintenance_task_loose_objects,
++ loose_object_auto_condition,
+ },
+ [TASK_INCREMENTAL_REPACK] = {
+ "incremental-repack",
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'loose-objects task' '
16: 3d527cb0dd ! 18: 801b262d1c maintenance: add incremental-repack auto condition
@@ builtin/gc.c: static int maintenance_task_loose_objects(void)
+
static int multi_pack_index_write(void)
{
- int result;
-@@ builtin/gc.c: static void initialize_tasks(void)
-
- tasks[num_tasks]->name = "incremental-repack";
- tasks[num_tasks]->fn = maintenance_task_incremental_repack;
-+ tasks[num_tasks]->auto_condition = incremental_repack_auto_condition;
- num_tasks++;
-
- tasks[num_tasks]->name = "gc";
+ struct child_process child = CHILD_PROCESS_INIT;
+@@ builtin/gc.c: static struct maintenance_task tasks[] = {
+ [TASK_INCREMENTAL_REPACK] = {
+ "incremental-repack",
+ maintenance_task_incremental_repack,
++ incremental_repack_auto_condition,
+ },
+ [TASK_GC] = {
+ "gc",
## t/t7900-maintenance.sh ##
@@ t/t7900-maintenance.sh: test_expect_success 'incremental-repack task' '
17: a0f00f8ab8 = 19: 9b4cef7635 midx: use start_delayed_progress() 18: f24db7739f ! 20: 39eb83ad1e maintenance: add trace2 regions for task execution
@@ Commit message
## builtin/gc.c ##
@@ builtin/gc.c: static int maintenance_run(void)
- !tasks[i]->auto_condition()))
+ !tasks[i].auto_condition()))
continue;
-+ trace2_region_enter("maintenance", tasks[i]->name, r);
- result = tasks[i]->fn();
-+ trace2_region_leave("maintenance", tasks[i]->name, r);
++ trace2_region_enter("maintenance", tasks[i].name, r);
+ if (tasks[i].fn()) {
+ error(_("task '%s' failed"), tasks[i].name);
+ result = 1;
+ }
++ trace2_region_leave("maintenance", tasks[i].name, r);
}
rollback_lock_file(&lk);
Signature · 1 line
gitgitgadget
PATCH v3 1/20 maintenance: create basic maintenance runner
From: Derrick Stolee <[email protected]>
The 'gc' builtin is our current entrypoint for automatically maintaining a repository. This one tool does many operations, such as repacking the repository, packing refs, and rewriting the commit-graph file. The name implies it performs "garbage collection" which means several different things, and some users may not want to use this operation that rewrites the entire object database.
Create a new 'maintenance' builtin that will become a more general- purpose command. To start, it will only support the 'run' subcommand, but will later expand to add subcommands for scheduling maintenance in the background.
For now, the 'maintenance' builtin is a thin shim over the 'gc' builtin. In fact, the only option is the '--auto' toggle, which is handed directly to the 'gc' builtin. The current change is isolated to this simple operation to prevent more interesting logic from being lost in all of the boilerplate of adding a new builtin.
Use existing builtin/gc.c file because we want to share code between the two builtins. It is possible that we will have 'maintenance' replace the 'gc' builtin entirely at some point, leaving 'git gc' as an alias for some specific arguments to 'git maintenance run'.
--- .gitignore | 1 + Documentation/git-maintenance.txt | 57 +++++++++++++++++++++++++++++++ builtin.h | 1 + builtin/gc.c | 56 ++++++++++++++++++++++++++++++ git.c | 1 + t/t7900-maintenance.sh | 19 +++++++++++ 6 files changed, 135 insertions(+) create mode 100644 Documentation/git-maintenance.txt create mode 100755 t/t7900-maintenance.sh
.gitignore +1 -0
@@ -90,6 +90,7 @@ /git-ls-tree /git-mailinfo /git-mailsplit +/git-maintenance /git-merge /git-merge-base /git-merge-index
Documentation/git-maintenance.txt new file +57 -0
57 lines not on this page · raw message
builtin.h +1 -0
@@ -167,6 +167,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix);int cmd_ls_remote (int argc,const char **argv,const char *prefix);int cmd_mailinfo (int argc,const char **argv,const char *prefix);int cmd_mailsplit (int argc,const char **argv,const char *prefix); +int cmd_maintenance (int argc,const char **argv,const char *prefix);int cmd_merge (int argc,const char **argv,const char *prefix);int cmd_merge_base (int argc,const char **argv,const char *prefix);int cmd_merge_index (int argc,const char **argv,const char *prefix);
builtin/gc.c +56 -0
59 lines not on this page · raw message
git.c +1 -0
@@ -529,6 +529,7 @@ static struct cmd_struct commands[] = { { "ls-tree", cmd_ls_tree, RUN_SETUP }, { "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "mailsplit", cmd_mailsplit, NO_PARSEOPT }, + { "maintenance", cmd_maintenance, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE }, { "merge-base", cmd_merge_base, RUN_SETUP }, { "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
t/t7900-maintenance.sh new file +19 -0
@@ -0,0 +1,19 @@ +#!/bin/sh + +test_description='git maintenance builtin' + +. ./test-lib.sh + +test_expect_success 'help text' ' + test_expect_code 129 git maintenance -h 2>err && + test_i18ngrep "usage: git maintenance run" err +' + +test_expect_success 'run [--auto]' ' + GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run && + GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto && + grep ",\"gc\"]" run-no-auto.txt && + grep ",\"gc\",\"--auto\"]" run-auto.txt +' + +test_done
Signature · 1 line
gitgitgadget
PATCH v3 2/20 maintenance: add --quiet option
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Maintenance activities are commonly used as steps in larger scripts. Providing a '--quiet' option allows those scripts to be less noisy when run on a terminal window. Turn this mode on by default when stderr is not a terminal.
Pipe the option to the 'git gc' child process.
--- Documentation/git-maintenance.txt | 3 +++ builtin/gc.c | 7 +++++++ t/t7900-maintenance.sh | 8 +++++--- 3 files changed, 15 insertions(+), 3 deletions(-)
Documentation/git-maintenance.txt +3 -0
@@ -52,6 +52,9 @@ OPTIONS in the `gc.auto` config setting, or when the number of pack-files exceeds the `gc.autoPackLimit` config setting. +--quiet:: + Do not report progress or other information over `stderr`. + GIT --- Part of the linkgit:git[1] suite
builtin/gc.c +7 -0
@@ -707,6 +707,7 @@ static const char * const builtin_maintenance_usage[] = {static struct maintenance_opts {int auto_flag; +int quiet; } opts;static int maintenance_task_gc (void )
@@ -718,6 +719,8 @@ static int maintenance_task_gc(void)if (opts.auto_flag)strvec_push (&child.args,"--auto" ); +if (opts.quiet) +strvec_push (&child.args,"--quiet" );close_object_store (the_repository->objects);return run_command (&child);
@@ -733,6 +736,8 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix)static struct option builtin_maintenance_options[] = {OPT_BOOL (0 ,"auto" , &opts.auto_flag,N_ ("run tasks based on the state of the repository" )), +OPT_BOOL (0 ,"quiet" , &opts.quiet, +N_ ("do not report progress or other information over stderr" )),OPT_END () };
@@ -742,6 +747,8 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix)usage_with_options (builtin_maintenance_usage, builtin_maintenance_options); + opts.quiet = !isatty (2 ); + argc = parse_options(argc, argv, prefix, builtin_maintenance_options, builtin_maintenance_usage,
t/t7900-maintenance.sh +5 -3
@@ -9,11 +9,13 @@ test_expect_success 'help text' ' test_i18ngrep "usage: git maintenance run" err ' -test_expect_success 'run [--auto]' ' - GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run && +test_expect_success 'run [--auto|--quiet]' ' + GIT_TRACE2_EVENT="$(pwd)/run-no-auto.txt" git maintenance run --no-quiet && GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto && + GIT_TRACE2_EVENT="$(pwd)/run-quiet.txt" git maintenance run --quiet && grep ",\"gc\"]" run-no-auto.txt && - grep ",\"gc\",\"--auto\"]" run-auto.txt + grep ",\"gc\",\"--auto\"" run-auto.txt && + grep ",\"gc\",\"--quiet\"" run-quiet.txt ' test_done
Signature · 1 line
gitgitgadget
PATCH v3 3/20 maintenance: replace run_auto_gc()
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The run_auto_gc() method is used in several places to trigger a check for repo maintenance after some Git commands, such as 'git commit' or 'git fetch'.
To allow for extra customization of this maintenance activity, replace the 'git gc --auto [--quiet]' call with one to 'git maintenance run --auto [--quiet]'. As we extend the maintenance builtin with other steps, users will be able to select different maintenance activities.
Rename run_auto_gc() to run_auto_maintenance() to be clearer what is happening on this call, and to expose all callers in the current diff. Rewrite the method to use a struct child_process to simplify the calls slightly.
Since 'git fetch' already allows disabling the 'git gc --auto' subprocess, add an equivalent option with a different name to be more descriptive of the new behavior: '--[no-]maintenance'. Update the documentation to include these options at the same time.
--- Documentation/fetch-options.txt | 6 ++++-- Documentation/git-clone.txt | 6 +++--- builtin/am.c | 2 +- builtin/commit.c | 2 +- builtin/fetch.c | 6 ++++-- builtin/merge.c | 2 +- builtin/rebase.c | 4 ++-- run-command.c | 16 +++++++--------- run-command.h | 2 +- t/t5510-fetch.sh | 2 +- 10 files changed, 25 insertions(+), 23 deletions(-)
Documentation/fetch-options.txt +4 -2
@@ -86,9 +86,11 @@ ifndef::git-pull[] Allow several <repository> and <group> arguments to be specified. No <refspec>s may be specified. +--[no-]auto-maintenance:: --[no-]auto-gc:: - Run `git gc --auto` at the end to perform garbage collection - if needed. This is enabled by default. + Run `git maintenance run --auto` at the end to perform automatic + repository maintenance if needed. (`--[no-]auto-gc` is a synonym.) + This is enabled by default. --[no-]write-commit-graph:: Write a commit-graph after fetching. This overrides the config
Documentation/git-clone.txt +3 -3
@@ -78,9 +78,9 @@ repository using this option and then delete branches (or use any other Git command that makes any existing commit unreferenced) in the source repository, some objects may become unreferenced (or dangling). These objects may be removed by normal Git operations (such as `git commit`) -which automatically call `git gc --auto`. (See linkgit:git-gc[1].) -If these objects are removed and were referenced by the cloned repository, -then the cloned repository will become corrupt. +which automatically call `git maintenance run --auto`. (See +linkgit:git-maintenance[1].) If these objects are removed and were referenced +by the cloned repository, then the cloned repository will become corrupt. + Note that running `git repack` without the `--local` option in a repository cloned with `--shared` will copy objects from the source repository into a pack
builtin/am.c +1 -1
@@ -1795,7 +1795,7 @@ static void am_run(struct am_state *state, int resume)if (!state->rebasing) {am_destroy (state);close_object_store (the_repository->objects); -run_auto_gc (state->quiet); +run_auto_maintenance (state->quiet); } }
builtin/commit.c +1 -1
@@ -1702,7 +1702,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)git_test_write_commit_graph_or_die ();repo_rerere (the_repository,0 ); -run_auto_gc (quiet); +run_auto_maintenance (quiet);run_commit_hook (use_editor,get_index_file (),"post-commit" ,NULL );if (amend && !no_post_rewrite) {commit_post_rewrite (the_repository, current_head, &oid);
builtin/fetch.c +4 -2
@@ -196,8 +196,10 @@ static struct option builtin_fetch_options[] = {OPT_STRING_LIST (0 ,"negotiation-tip" , &negotiation_tip,N_ ("revision" ),N_ ("report that we have only objects reachable from this object" )),OPT_PARSE_LIST_OBJECTS_FILTER (&filter_options), +OPT_BOOL (0 ,"auto-maintenance" , &enable_auto_gc, +N_ ("run 'maintenance --auto' after fetching" )),OPT_BOOL (0 ,"auto-gc" , &enable_auto_gc, -N_ ("run 'gc --auto' after fetching" )), +N_ ("run 'maintenance --auto' after fetching" )),OPT_BOOL (0 ,"show-forced-updates" , &fetch_show_forced_updates,N_ ("check for forced-updates on all updated branches" )), OPT_BOOL(0, "write-commit-graph", &fetch_write_commit_graph,
@@ -1882,7 +1884,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)close_object_store (the_repository->objects);if (enable_auto_gc) -run_auto_gc (verbosity <0 ); +run_auto_maintenance (verbosity <0 );return result; }
builtin/merge.c +1 -1
@@ -457,7 +457,7 @@ static void finish(struct commit *head_commit,* user should see them. */ close_object_store (the_repository->objects); -run_auto_gc (verbosity <0 ); +run_auto_maintenance (verbosity <0 ); } }if (new_head && show_diffstat) {
builtin/rebase.c +2 -2
@@ -728,10 +728,10 @@ static int finish_rebase(struct rebase_options *opts)apply_autostash (state_dir_path ("autostash" , opts));close_object_store (the_repository->objects);/* -* We ignore errors in 'gc --auto', since the +* We ignore errors in 'git maintenance run --auto', since the * user should see them. */ -run_auto_gc (!(opts->flags & (REBASE_NO_QUIET |REBASE_VERBOSE ))); +run_auto_maintenance (!(opts->flags & (REBASE_NO_QUIET |REBASE_VERBOSE )));if (opts->type ==REBASE_MERGE ) {struct replay_opts replay =REPLAY_OPTS_INIT ;
run-command.c +7 -9
@@ -1866,15 +1866,13 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task,return result; } -int run_auto_gc (int quiet) +int run_auto_maintenance (int quiet) { -struct strvec argv_gc_auto =STRVEC_INIT ; -int status; +struct child_process maint =CHILD_PROCESS_INIT ; -strvec_pushl (&argv_gc_auto,"gc" ,"--auto" ,NULL ); -if (quiet) -strvec_push (&argv_gc_auto,"--quiet" ); - status =run_command_v_opt (argv_gc_auto.items,RUN_GIT_CMD ); -strvec_clear (&argv_gc_auto); -return status; + maint.git_cmd =1 ; +strvec_pushl (&maint.args,"maintenance" ,"run" ,"--auto" ,NULL ); +strvec_push (&maint.args, quiet ?"--quiet" :"--no-quiet" ); + +return run_command (&maint); }
run-command.h +1 -1
@@ -221,7 +221,7 @@ int run_hook_ve(const char *const *env, const char *name, va_list args);/* * Trigger an auto-gc */ -int run_auto_gc (int quiet); +int run_auto_maintenance (int quiet);#define RUN_COMMAND_NO_STDIN 1#define RUN_GIT_CMD 2/*If this is to be git sub-command */
t/t5510-fetch.sh +1 -1
@@ -919,7 +919,7 @@ test_expect_success 'fetching with auto-gc does not lock up' ' git config fetch.unpackLimit 1 && git config gc.autoPackLimit 1 && git config gc.autoDetach false && - GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && + GIT_ASK_YESNO="$D/askyesno" git fetch --verbose >fetch.out 2>&1 && test_i18ngrep "Auto packing the repository" fetch.out && ! grep "Should I try again" fetch.out )
Signature · 1 line
gitgitgadget
PATCH v3 4/20 maintenance: initialize task array
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
In anticipation of implementing multiple maintenance tasks inside the 'maintenance' builtin, use a list of structs to describe the work to be done.
The struct maintenance_task stores the name of the task (as given by a future command-line argument) along with a function pointer to its implementation and a boolean for whether the step is enabled.
A list these structs are initialized with the full list of implemented tasks along with a default order. For now, this list only contains the "gc" task. This task is also the only task enabled by default.
The run subcommand will return a nonzero exit code if any task fails. However, it will attempt all tasks in its loop before returning with the failure. Also each failed task will send an error message.
--- builtin/gc.c | 38 +++++++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-)
builtin/gc.c +37 -1
@@ -726,9 +726,45 @@ static int maintenance_task_gc(void)return run_command (&child); } +typedef int maintenance_task_fn (void ); + +struct maintenance_task { +const char *name; +maintenance_task_fn *fn; +unsigned enabled:1 ; +}; + +enum maintenance_task_label { +TASK_GC , + +/* Leave as final value */ +TASK__COUNT +}; + +static struct maintenance_task tasks[] = { + [TASK_GC ] = { +"gc" , + maintenance_task_gc, +1 , + }, +}; +static int maintenance_run (void ) { -return maintenance_task_gc (); +int i; +int result =0 ; + +for (i =0 ; i <TASK__COUNT ; i++) { +if (!tasks[i].enabled) +continue ; + +if (tasks[i].fn ()) { +error (_ ("task '%s' failed" ), tasks[i].name); + result =1 ; + } + } + +return result; }int cmd_maintenance (int argc,const char **argv,const char *prefix)
Signature · 1 line
gitgitgadget
PATCH v3 5/20 maintenance: add commit-graph task
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The first new task in the 'git maintenance' builtin is the 'commit-graph' job. It is based on the sequence of events in the 'commit-graph' job in Scalar [1]. This sequence is as follows:
1. git commit-graph write --reachable --split
2. git commit-graph verify --shallow
3. If the verify succeeds, stop.
4. Delete the commit-graph-chain file.
5. git commit-graph write --reachable --split
By writing an incremental commit-graph file using the "--split" option we minimize the disruption from this operation. The default behavior is to merge layers until the new "top" layer is less than half the size of the layer below. This provides quick writes most of the time, with the longer writes following a power law distribution.
Most importantly, concurrent Git processes only look at the commit-graph-chain file for a very short amount of time, so they will verly likely not be holding a handle to the file when we try to replace it. (This only matters on Windows.)
If a concurrent process reads the old commit-graph-chain file, but our job expires some of the .graph files before they can be read, then those processes will see a warning message (but not fail). This could be avoided by a future update to use the --expire-time argument when writing the commit-graph.
By using 'git commit-graph verify --shallow' we can ensure that the file we just wrote is valid. This is an extra safety precaution that is faster than our 'write' subcommand. In the rare situation that the newest layer of the commit-graph is corrupt, we can "fix" the corruption by deleting the commit-graph-chain file and rewrite the full commit-graph as a new one-layer commit graph. This does not completely prevent _that_ file from being corrupt, but it does recompute the commit-graph by parsing commits from the object database. In our use of this step in Scalar and VFS for Git, we have only seen this issue arise because our microsoft/git fork reverted 43d3561 ("commit-graph write: don't die if the existing graph is corrupt" 2019-03-25) for a while to keep commit-graph writes very fast. We dropped the revert when updating to v2.23.0. The verify still has potential for catching corrupt data across the layer boundary: if the new file has commit X with parent Y in an old file but the commit ID for Y in the old file had a bitswap, then we will notice that in the 'verify' command.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/CommitGraphStep.cs
--- Documentation/git-maintenance.txt | 18 +++++++++ builtin/gc.c | 63 +++++++++++++++++++++++++++++++ commit-graph.c | 8 ++-- commit-graph.h | 1 + t/t7900-maintenance.sh | 2 + 5 files changed, 88 insertions(+), 4 deletions(-)
Documentation/git-maintenance.txt +18 -0
@@ -35,6 +35,24 @@ run:: TASKS ----- +commit-graph:: + The `commit-graph` job updates the `commit-graph` files incrementally, + then verifies that the written data is correct. If the new layer has an + issue, then the chain file is removed and the `commit-graph` is + rewritten from scratch. ++ +The verification only checks the top layer of the `commit-graph` chain. +If the incremental write merged the new commits with at least one +existing layer, then there is potential for on-disk corruption being +carried forward into the new file. This will be noticed and the new +commit-graph file will be clean as Git reparses the commit data from +the object database. ++ +The incremental write is safe to run alongside concurrent Git processes +since it will not expire `.graph` files that were in the previous +`commit-graph-chain` file. They will be deleted by a later run based on +the expiration delay. + gc:: Cleanup unnecessary files and optimize the local repository. "GC" stands for "garbage collection," but this task performs many
builtin/gc.c +63 -0
81 lines not on this page · raw message
commit-graph.c +4 -4
@@ -172,7 +172,7 @@ static char *get_split_graph_filename(struct object_directory *odb, oid_hex); } -static char *get_chain_filename (struct object_directory *odb) +char *get_commit_graph_chain_filename (struct object_directory *odb) {return xstrfmt ("%s/info/commit-graphs/commit-graph-chain" , odb->path); }
@@ -521,7 +521,7 @@ static struct commit_graph *load_commit_graph_chain(struct repository *r,struct stat st;struct object_id *oids;int i =0 , valid =1 , count; -char *chain_name =get_chain_filename (odb); +char *chain_name =get_commit_graph_chain_filename (odb);FILE *fp;int stat_res;
@@ -1619,7 +1619,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) }if (ctx->split) { -char *lock_name =get_chain_filename (ctx->odb); +char *lock_name =get_commit_graph_chain_filename (ctx->odb);hold_lock_file_for_update_mode (&lk, lock_name,LOCK_DIE_ON_ERROR ,0444 );
@@ -1996,7 +1996,7 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx)if (ctx->split_opts && ctx->split_opts->expire_time) expire_time = ctx->split_opts->expire_time;if (!ctx->split) { -char *chain_file_name =get_chain_filename (ctx->odb); +char *chain_file_name =get_commit_graph_chain_filename (ctx->odb);unlink (chain_file_name);free (chain_file_name); ctx->num_commit_graphs_after =0 ;
commit-graph.h +1 -0
@@ -25,6 +25,7 @@ struct commit;struct bloom_filter_settings ;char *get_commit_graph_filename (struct object_directory *odb); +char *get_commit_graph_chain_filename (struct object_directory *odb);int open_commit_graph (const char *graph_file,int *fd,struct stat *st);/*
t/t7900-maintenance.sh +2 -0
@@ -4,6 +4,8 @@ test_description='git maintenance builtin' . ./test-lib.sh +GIT_TEST_COMMIT_GRAPH=0 + test_expect_success 'help text' ' test_expect_code 129 git maintenance -h 2>err && test_i18ngrep "usage: git maintenance run" err
Signature · 1 line
gitgitgadget
PATCH v3 6/20 maintenance: add --task option
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
A user may want to only run certain maintenance tasks in a certain order. Add the --task=<task> option, which allows a user to specify an ordered list of tasks to run. These cannot be run multiple times, however.
Here is where our array of maintenance_task pointers becomes critical. We can sort the array of pointers based on the task order, but we do not want to move the struct data itself in order to preserve the hashmap references. We use the hashmap to match the --task=<task> arguments into the task struct data.
Keep in mind that the 'enabled' member of the maintenance_task struct is a placeholder for a future 'maintenance.<task>.enabled' config option. Thus, we use the 'enabled' member to specify which tasks are run when the user does not specify any --task=<task> arguments. The 'enabled' member should be ignored if --task=<task> appears.
--- Documentation/git-maintenance.txt | 4 +++ builtin/gc.c | 59 +++++++++++++++++++++++++++++-- t/t7900-maintenance.sh | 23 ++++++++++++ 3 files changed, 84 insertions(+), 2 deletions(-)
Documentation/git-maintenance.txt +4 -0
@@ -73,6 +73,10 @@ OPTIONS --quiet:: Do not report progress or other information over `stderr`. +--task=<task>:: + If this option is specified one or more times, then only run the + specified tasks in the specified order. + GIT --- Part of the linkgit:git[1] suite
builtin/gc.c +57 -2
95 lines not on this page · raw message
t/t7900-maintenance.sh +23 -0
@@ -20,4 +20,27 @@ test_expect_success 'run [--auto|--quiet]' ' grep ",\"gc\",\"--quiet\"" run-quiet.txt ' +test_expect_success 'run --task=<task>' ' + GIT_TRACE2_EVENT="$(pwd)/run-commit-graph.txt" git maintenance run --task=commit-graph && + GIT_TRACE2_EVENT="$(pwd)/run-gc.txt" git maintenance run --task=gc && + GIT_TRACE2_EVENT="$(pwd)/run-commit-graph.txt" git maintenance run --task=commit-graph && + GIT_TRACE2_EVENT="$(pwd)/run-both.txt" git maintenance run --task=commit-graph --task=gc && + ! grep ",\"gc\"" run-commit-graph.txt && + grep ",\"gc\"" run-gc.txt && + grep ",\"gc\"" run-both.txt && + grep ",\"commit-graph\",\"write\"" run-commit-graph.txt && + ! grep ",\"commit-graph\",\"write\"" run-gc.txt && + grep ",\"commit-graph\",\"write\"" run-both.txt +' + +test_expect_success 'run --task=bogus' ' + test_must_fail git maintenance run --task=bogus 2>err && + test_i18ngrep "is not a valid task" err +' + +test_expect_success 'run --task duplicate' ' + test_must_fail git maintenance run --task=gc --task=gc 2>err && + test_i18ngrep "cannot be selected multiple times" err +' + test_done
Signature · 1 line
gitgitgadget
PATCH v3 7/20 maintenance: take a lock on the objects directory
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Performing maintenance on a Git repository involves writing data to the .git directory, which is not safe to do with multiple writers attempting the same operation. Ensure that only one 'git maintenance' process is running at a time by holding a file-based lock. Simply the presence of the .git/maintenance.lock file will prevent future maintenance. This lock is never committed, since it does not represent meaningful data. Instead, it is only a placeholder.
If the lock file already exists, then fail silently. This will become very important later when we implement the 'fetch' task, as this is our stop-gap from creating a recursive process loop between 'git fetch' and 'git maintenance run'.
--- builtin/gc.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
builtin/gc.c +20 -0
@@ -829,6 +829,25 @@ static int maintenance_run(void) {int i;int result =0 ; +struct lock_file lk; +struct repository *r = the_repository; +char *lock_path =xstrfmt ("%s/maintenance" , r->objects->odb->path); + +if (hold_lock_file_for_update (&lk, lock_path,LOCK_NO_DEREF ) <0 ) { +/* +* Another maintenance command is running. +* +* If --auto was provided, then it is likely due to a +* recursive process stack. Do not report an error in +* that case. +*/ +if (!opts.auto_flag && !opts.quiet) +error (_ ("lock file '%s' exists, skipping maintenance" ), + lock_path); +free (lock_path); +return 0 ; + } +free (lock_path);if (opts.tasks_selected)QSORT (tasks,TASK__COUNT , compare_tasks_by_selection);
@@ -846,6 +865,7 @@ static int maintenance_run(void) } } +rollback_lock_file (&lk);return result; }
Signature · 1 line
gitgitgadget
PATCH v3 8/20 fetch: optionally allow disabling FETCH_HEAD update
In reply to Derrick Stolee via GitGitGadgetFrom: Junio C Hamano <[email protected]>
If you run fetch but record the result in remote-tracking branches, and either if you do nothing with the fetched refs (e.g. you are merely mirroring) or if you always work from the remote-tracking refs (e.g. you fetch and then merge origin/branchname separately), you can get away with having no FETCH_HEAD at all.
Teach "git fetch" a command line option "--[no-]write-fetch-head" and "fetch.writeFetchHEAD" configuration variable. Without either, the default is to write FETCH_HEAD, and the usual rule that the command line option defeats configured default applies.
Note that under "--dry-run" mode, FETCH_HEAD is never written; otherwise you'd see list of objects in the file that you do not actually have. Passing --write-fetch-head does not force git fetch to write the file.
Also note that this option is explicitly passed when "git pull" internally invokes "git fetch", so that those who configured their "git fetch" not to write FETCH_HEAD would not be able to break the cooperation between these two commands. "git pull" must see what "git fetch" got recorded in FETCH_HEAD to work correctly.
--- Documentation/config/fetch.txt | 7 ++++++ Documentation/fetch-options.txt | 10 +++++++++ builtin/fetch.c | 19 +++++++++++++--- builtin/pull.c | 3 ++- t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++-- t/t5521-pull-options.sh | 16 ++++++++++++++ 6 files changed, 88 insertions(+), 6 deletions(-)
Documentation/config/fetch.txt +7 -0
@@ -91,3 +91,10 @@ fetch.writeCommitGraph:: merge and the write may take longer. Having an updated commit-graph file helps performance of many Git commands, including `git merge-base`, `git push -f`, and `git log --graph`. Defaults to false. + +fetch.writeFetchHEAD:: + Setting it to false tells `git fetch` not to write the list + of remote refs fetched in the `FETCH_HEAD` file directly + under `$GIT_DIR`. Can be countermanded from the command + line with the `--[no-]write-fetch-head` option. Defaults to + true.
Documentation/fetch-options.txt +10 -0
@@ -64,6 +64,16 @@ documented in linkgit:git-config[1]. --dry-run:: Show what would be done, without making any changes. +ifndef::git-pull[] +--[no-]write-fetch-head:: + Write the list of remote refs fetched in the `FETCH_HEAD` + file directly under `$GIT_DIR`. This is the default unless + the configuration variable `fetch.writeFetchHEAD` is set to + false. Passing `--no-write-fetch-head` from the command + line tells Git not to write the file. Under `--dry-run` + option, the file is never written. +endif::git-pull[] + -f:: --force:: When 'git fetch' is used with `<src>:<dst>` refspec it may
builtin/fetch.c +16 -3
@@ -56,6 +56,7 @@ static int prune_tags = -1; /* unspecified */#define PRUNE_TAGS_BY_DEFAULT 0/* do we prune tags by default? */ static int all, append, dry_run, force, keep, multiple, update_head_ok; +static int write_fetch_head =1 ;static int verbosity, deepen_relative, set_upstream;static int progress =-1 ;static int enable_auto_gc =1 ;
@@ -118,6 +119,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb)return 0 ; } +if (!strcmp (k,"fetch.writefetchhead" )) { + write_fetch_head =git_config_bool (k, v); +return 0 ; + }return git_default_config (k, v, cb); }
@@ -162,6 +167,8 @@ static struct option builtin_fetch_options[] = { PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules),OPT_BOOL (0 ,"dry-run" , &dry_run,N_ ("dry run" )), +OPT_BOOL (0 ,"write-fetch-head" , &write_fetch_head, +N_ ("write fetched references to the FETCH_HEAD file" )),OPT_BOOL ('k' ,"keep" , &keep,N_ ("keep downloaded pack" )),OPT_BOOL ('u' ,"update-head-ok" , &update_head_ok, N_("allow updating of HEAD ref")),
@@ -895,7 +902,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,const char *what, *kind;struct ref *rm;char *url; -const char *filename = dry_run ?"/dev/null" :git_path_fetch_head (the_repository); +const char *filename = (!write_fetch_head + ?"/dev/null" + :git_path_fetch_head (the_repository));int want_status;int summary_width =transport_summary_width (ref_map);
@@ -1329,7 +1338,7 @@ static int do_fetch(struct transport *transport, }/* if not appending, truncate FETCH_HEAD */ -if (!append && !dry_run) { +if (!append && write_fetch_head) { retcode =truncate_fetch_head ();if (retcode) goto cleanup;
@@ -1596,7 +1605,7 @@ static int fetch_multiple(struct string_list *list, int max_children)int i, result =0 ;struct strvec argv =STRVEC_INIT ; -if (!append && !dry_run) { +if (!append && write_fetch_head) {int errcode =truncate_fetch_head ();if (errcode)return errcode;
@@ -1797,6 +1806,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)if (depth || deepen_since || deepen_not.nr) deepen =1 ; +/* FETCH_HEAD never gets updated in --dry-run mode */ +if (dry_run) + write_fetch_head =0 ; +if (all) {if (argc ==1 )die (_ ("fetch --all does not take a repository argument" ));
builtin/pull.c +2 -1
@@ -527,7 +527,8 @@ static int run_fetch(const char *repo, const char **refspecs)struct strvec args =STRVEC_INIT ;int ret; -strvec_pushl (&args,"fetch" ,"--update-head-ok" ,NULL ); +strvec_pushl (&args,"fetch" ,"--update-head-ok" , +"--write-fetch-head" ,NULL );/* Shared options */ argv_push_verbosity (&args);
t/t5510-fetch.sh +37 -2
@@ -539,13 +539,48 @@ test_expect_success 'fetch into the current branch with --update-head-ok' ' ' -test_expect_success 'fetch --dry-run' ' - +test_expect_success 'fetch --dry-run does not touch FETCH_HEAD' ' rm -f .git/FETCH_HEAD && git fetch --dry-run . && ! test -f .git/FETCH_HEAD ' +test_expect_success '--no-write-fetch-head does not touch FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success '--write-fetch-head gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git fetch --dry-run --write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --dry-run . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --no-write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch --write-fetch-head . && + test -f .git/FETCH_HEAD +' + test_expect_success "should be able to fetch with duplicate refspecs" ' mkdir dups && (
t/t5521-pull-options.sh +16 -0
@@ -77,6 +77,7 @@ test_expect_success 'git pull -q -v --no-rebase' ' test_must_be_empty out && test -s err) ' + test_expect_success 'git pull --cleanup errors early on invalid argument' ' mkdir clonedcleanup && (cd clonedcleanup && git init &&
@@ -85,6 +86,21 @@ test_expect_success 'git pull --cleanup errors early on invalid argument' ' test -s err) ' +test_expect_success 'git pull --no-write-fetch-head fails' ' + mkdir clonedwfh && + (cd clonedwfh && git init && + test_must_fail git pull --no-write-fetch-head "../parent" >out 2>err && + test_must_be_empty out && + test_i18ngrep "no-write-fetch-head" err) +' + +test_expect_success 'git pull succeeds with fetch.writeFetchHEAD=false' ' + mkdir clonedwfhconfig && + (cd clonedwfhconfig && git init && + git config fetch.writeFetchHEAD false && + git pull "../parent" >out 2>err && + grep FETCH_HEAD err) +' test_expect_success 'git pull --force' ' mkdir clonedoldstyle &&
Signature · 1 line
gitgitgadget
PATCH v3 9/20 maintenance: add prefetch task
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
When working with very large repositories, an incremental 'git fetch' command can download a large amount of data. If there are many other users pushing to a common repo, then this data can rival the initial pack-file size of a 'git clone' of a medium-size repo.
Users may want to keep the data on their local repos as close as possible to the data on the remote repos by fetching periodically in the background. This can break up a large daily fetch into several smaller hourly fetches.
The task is called "prefetch" because it is work done in advance of a foreground fetch to make that 'git fetch' command much faster.
However, if we simply ran 'git fetch <remote>' in the background, then the user running a foregroudn 'git fetch <remote>' would lose some important feedback when a new branch appears or an existing branch updates. This is especially true if a remote branch is force-updated and this isn't noticed by the user because it occurred in the background. Further, the functionality of 'git push --force-with-lease' becomes suspect.
When running 'git fetch <remote> <options>' in the background, use the following options for careful updating:
1. --no-tags prevents getting a new tag when a user wants to see the new tags appear in their foreground fetches.
2. --refmap= removes the configured refspec which usually updates refs/remotes/<remote>/* with the refs advertised by the remote. While this looks confusing, this was documented and tested by b40a50264ac (fetch: document and test --refmap="", 2020-01-21), including this sentence in the documentation:
Providing an empty <refspec> to the --refmap option causes Git to ignore the configured refspecs and rely entirely on the refspecs supplied as command-line arguments.
3. By adding a new refspec "+refs/heads/*:refs/prefetch/<remote>/*" we can ensure that we actually load the new values somewhere in our refspace while not updating refs/heads or refs/remotes. By storing these refs here, the commit-graph job will update the commit-graph with the commits from these hidden refs.
4. --prune will delete the refs/prefetch/<remote> refs that no longer appear on the remote.
5. --no-write-fetch-head prevents updating FETCH_HEAD.
We've been using this step as a critical background job in Scalar
[1] (and VFS for Git). This solved a pain point that was showing up in user reports: fetching was a pain! Users do not like waiting to download the data that was created while they were away from their machines. After implementing background fetch, the foreground fetch commands sped up significantly because they mostly just update refs and download a small amount of new data. The effect is especially dramatic when paried with --no-show-forced-udpates (through fetch.showForcedUpdates=false).
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/FetchStep.cs
--- Documentation/git-maintenance.txt | 15 +++++++++ builtin/gc.c | 52 +++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 24 ++++++++++++++ 3 files changed, 91 insertions(+)
Documentation/git-maintenance.txt +15 -0
@@ -53,6 +53,21 @@ since it will not expire `.graph` files that were in the previous `commit-graph-chain` file. They will be deleted by a later run based on the expiration delay. +prefetch:: + The `prefetch` task updates the object directory with the latest + objects from all registered remotes. For each remote, a `git fetch` + command is run. The refmap is custom to avoid updating local or remote + branches (those in `refs/heads` or `refs/remotes`). Instead, the + remote refs are stored in `refs/prefetch/<remote>/`. Also, tags are + not updated. ++ +This is done to avoid disrupting the remote-tracking branches. The end users +expect these refs to stay unmoved unless they initiate a fetch. With prefetch +task, however, the objects necessary to complete a later real fetch would +already be obtained, so the real fetch would go faster. In the ideal case, +it will just become an update to bunch of remote-tracking branches without +any object transfer. + gc:: Cleanup unnecessary files and optimize the local repository. "GC" stands for "garbage collection," but this task performs many
builtin/gc.c +52 -0
76 lines not on this page · raw message
t/t7900-maintenance.sh +24 -0
@@ -43,4 +43,28 @@ test_expect_success 'run --task duplicate' ' test_i18ngrep "cannot be selected multiple times" err ' +test_expect_success 'run --task=prefetch with no remotes' ' + git maintenance run --task=prefetch 2>err && + test_must_be_empty err +' + +test_expect_success 'prefetch multiple remotes' ' + git clone . clone1 && + git clone . clone2 && + git remote add remote1 "file://$(pwd)/clone1" && + git remote add remote2 "file://$(pwd)/clone2" && + git -C clone1 switch -c one && + git -C clone2 switch -c two && + test_commit -C clone1 one && + test_commit -C clone2 two && + GIT_TRACE2_EVENT="$(pwd)/run-prefetch.txt" git maintenance run --task=prefetch && + grep ",\"fetch\",\"remote1\"" run-prefetch.txt && + grep ",\"fetch\",\"remote2\"" run-prefetch.txt && + test_path_is_missing .git/refs/remotes && + test_cmp clone1/.git/refs/heads/one .git/refs/prefetch/remote1/one && + test_cmp clone2/.git/refs/heads/two .git/refs/prefetch/remote2/two && + git log prefetch/remote1/one && + git log prefetch/remote2/two +' + test_done
Signature · 1 line
gitgitgadget
PATCH v3 10/20 maintenance: add loose-objects task
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
One goal of background maintenance jobs is to allow a user to disable auto-gc (gc.auto=0) but keep their repository in a clean state. Without any cleanup, loose objects will clutter the object database and slow operations. In addition, the loose objects will take up extra space because they are not stored with deltas against similar objects.
Create a 'loose-objects' task for the 'git maintenance run' command. This helps clean up loose objects without disrupting concurrent Git commands using the following sequence of events:
1. Run 'git prune-packed' to delete any loose objects that exist in a pack-file. Concurrent commands will prefer the packed version of the object to the loose version. (Of course, there are exceptions for commands that specifically care about the location of an object. These are rare for a user to run on purpose, and we hope a user that has selected background maintenance will not be trying to do foreground maintenance.)
2. Run 'git pack-objects' on a batch of loose objects. These objects are grouped by scanning the loose object directories in lexicographic order until listing all loose objects -or- reaching 50,000 objects. This is more than enough if the loose objects are created only by a user doing normal development. We noticed users with _millions_ of loose objects because VFS for Git downloads blobs on-demand when a file read operation requires populating a virtual file. This has potential of happening in partial clones if someone runs 'git grep' or otherwise evades the batch-download feature for requesting promisor objects.
This step is based on a similar step in Scalar [1] and VFS for Git.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/LooseObjectsStep.cs
--- Documentation/git-maintenance.txt | 11 ++++ builtin/gc.c | 97 +++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 39 +++++++++++++ 3 files changed, 147 insertions(+)
Documentation/git-maintenance.txt +11 -0
@@ -76,6 +76,17 @@ gc:: It can also be disruptive in some situations, as it deletes stale data. +loose-objects:: + The `loose-objects` job cleans up loose objects and places them into + pack-files. In order to prevent race conditions with concurrent Git + commands, it follows a two-step process. First, it deletes any loose + objects that already exist in a pack-file; concurrent Git processes + will examine the pack-file for the object data instead of the loose + object. Second, it creates a new pack-file (starting with "loose-") + containing a batch of loose objects. The batch size is limited to 50 + thousand objects to prevent the job from taking too long on a + repository with many loose objects. + OPTIONS ------- --auto::
builtin/gc.c +97 -0
115 lines not on this page · raw message
t/t7900-maintenance.sh +39 -0
@@ -67,4 +67,43 @@ test_expect_success 'prefetch multiple remotes' ' git log prefetch/remote2/two ' +test_expect_success 'loose-objects task' ' + # Repack everything so we know the state of the object dir + git repack -adk && + + # Hack to stop maintenance from running during "git commit" + echo in use >.git/objects/maintenance.lock && + + # Assuming that "git commit" creates at least one loose object + test_commit create-loose-object && + rm .git/objects/maintenance.lock && + + ls .git/objects >obj-dir-before && + test_file_not_empty obj-dir-before && + ls .git/objects/pack/*.pack >packs-before && + test_line_count = 1 packs-before && + + # The first run creates a pack-file + # but does not delete loose objects. + git maintenance run --task=loose-objects && + ls .git/objects >obj-dir-between && + test_cmp obj-dir-before obj-dir-between && + ls .git/objects/pack/*.pack >packs-between && + test_line_count = 2 packs-between && + ls .git/objects/pack/loose-*.pack >loose-packs && + test_line_count = 1 loose-packs && + + # The second run deletes loose objects + # but does not create a pack-file. + git maintenance run --task=loose-objects && + ls .git/objects >obj-dir-after && + cat >expect <<-\EOF && + info + pack + EOF + test_cmp expect obj-dir-after && + ls .git/objects/pack/*.pack >packs-after && + test_cmp packs-between packs-after +' + test_done
Signature · 1 line
gitgitgadget
PATCH v3 11/20 midx: enable core.multiPackIndex by default
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The core.multiPackIndex setting has been around since c4d25228ebb (config: create core.multiPackIndex setting, 2018-07-12), but has been disabled by default. If a user wishes to use the multi-pack-index feature, then they must enable this config and run 'git multi-pack-index write'.
The multi-pack-index feature is relatively stable now, so make the config option true by default. For users that do not use a multi-pack-index, the only extra cost will be a file lookup to see if a multi-pack-index file exists (once per process, per object directory).
Also, this config option will be referenced by an upcoming "incremental-repack" task in the maintenance builtin, so move the config option into the repository settings struct. Note that if GIT_TEST_MULTI_PACK_INDEX=1, then we want to ignore the config option and treat core.multiPackIndex as enabled.
--- Documentation/config/core.txt | 4 ++-- midx.c | 11 +++-------- repo-settings.c | 6 ++++++ repository.h | 2 ++ 4 files changed, 13 insertions(+), 10 deletions(-)
Documentation/config/core.txt +2 -2
@@ -606,8 +606,8 @@ core.useReplaceRefs:: core.multiPackIndex:: Use the multi-pack-index file to track multiple packfiles using a - single index. See link:technical/multi-pack-index.html[the - multi-pack-index design document]. + single index. See linkgit:git-multi-pack-index[1] for more + information. Defaults to true. core.sparseCheckout:: Enable "sparse checkout" feature. See linkgit:git-sparse-checkout[1]
midx.c +3 -8
@@ -10,6 +10,7 @@#include "progress.h" #include "trace2.h" #include "run-command.h" +#include "repository.h" #define MIDX_SIGNATURE 0x4d494458/* "MIDX" */ #define MIDX_VERSION 1
@@ -384,15 +385,9 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i {struct multi_pack_index *m;struct multi_pack_index *m_search; -int config_value; -static int env_value =-1 ; -if (env_value <0 ) - env_value =git_env_bool (GIT_TEST_MULTI_PACK_INDEX ,0 ); - -if (!env_value && - (repo_config_get_bool (r,"core.multipackindex" , &config_value) || - !config_value)) +prepare_repo_settings (r); +if (!r->settings.core_multi_pack_index)return 0 ;for (m_search = r->objects->multi_pack_index; m_search; m_search = m_search->next)
repo-settings.c +6 -0
@@ -1,6 +1,7 @@#include "cache.h" #include "config.h" #include "repository.h" +#include "midx.h" #define UPDATE_DEFAULT_BOOL (s,v) do { if (s == -1) { s = v; } } while(0)
@@ -47,6 +48,11 @@ void prepare_repo_settings(struct repository *r) r->settings.pack_use_sparse = value;UPDATE_DEFAULT_BOOL (r->settings.pack_use_sparse,1 ); + value =git_env_bool (GIT_TEST_MULTI_PACK_INDEX ,0 ); +if (value || !repo_config_get_bool (r,"core.multipackindex" , &value)) + r->settings.core_multi_pack_index = value; +UPDATE_DEFAULT_BOOL (r->settings.core_multi_pack_index,1 ); +if (!repo_config_get_bool (r,"feature.manyfiles" , &value) && value) {UPDATE_DEFAULT_BOOL (r->settings.index_version,4 );UPDATE_DEFAULT_BOOL (r->settings.core_untracked_cache,UNTRACKED_CACHE_WRITE );
repository.h +2 -0
@@ -37,6 +37,8 @@ struct repo_settings {int pack_use_sparse;enum fetch_negotiation_setting fetch_negotiation_algorithm; + +int core_multi_pack_index; };struct repository {
Signature · 1 line
gitgitgadget
PATCH v3 12/20 maintenance: add incremental-repack task
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The previous change cleaned up loose objects using the 'loose-objects' that can be run safely in the background. Add a similar job that performs similar cleanups for pack-files.
One issue with running 'git repack' is that it is designed to repack all pack-files into a single pack-file. While this is the most space-efficient way to store object data, it is not time or memory efficient. This becomes extremely important if the repo is so large that a user struggles to store two copies of the pack on their disk.
Instead, perform an "incremental" repack by collecting a few small pack-files into a new pack-file. The multi-pack-index facilitates this process ever since 'git multi-pack-index expire' was added in 19575c7 (multi-pack-index: implement 'expire' subcommand, 2019-06-10) and 'git multi-pack-index repack' was added in ce1e4a1 (midx: implement midx_repack(), 2019-06-10).
The 'incremental-repack' task runs the following steps:
1. 'git multi-pack-index write' creates a multi-pack-index file if one did not exist, and otherwise will update the multi-pack-index with any new pack-files that appeared since the last write. This is particularly relevant with the background fetch job.
When the multi-pack-index sees two copies of the same object, it stores the offset data into the newer pack-file. This means that some old pack-files could become "unreferenced" which I will use to mean "a pack-file that is in the pack-file list of the multi-pack-index but none of the objects in the multi-pack-index reference a location inside that pack-file."
2. 'git multi-pack-index expire' deletes any unreferenced pack-files and updaes the multi-pack-index to drop those pack-files from the list. This is safe to do as concurrent Git processes will see the multi-pack-index and not open those packs when looking for object contents. (Similar to the 'loose-objects' job, there are some Git commands that open pack-files regardless of the multi-pack-index, but they are rarely used. Further, a user that self-selects to use background operations would likely refrain from using those commands.)
3. 'git multi-pack-index repack --bacth-size=<size>' collects a set of pack-files that are listed in the multi-pack-index and creates a new pack-file containing the objects whose offsets are listed by the multi-pack-index to be in those objects. The set of pack- files is selected greedily by sorting the pack-files by modified time and adding a pack-file to the set if its "expected size" is smaller than the batch size until the total expected size of the selected pack-files is at least the batch size. The "expected size" is calculated by taking the size of the pack-file divided by the number of objects in the pack-file and multiplied by the number of objects from the multi-pack-index with offset in that pack-file. The expected size approximates how much data from that pack-file will contribute to the resulting pack-file size. The intention is that the resulting pack-file will be close in size to the provided batch size.
The next run of the incremental-repack task will delete these repacked pack-files during the 'expire' step.
In this version, the batch size is set to "0" which ignores the size restrictions when selecting the pack-files. It instead selects all pack-files and repacks all packed objects into a single pack-file. This will be updated in the next change, but it requires doing some calculations that are better isolated to a separate change.
Each of the above steps update the multi-pack-index file. After each step, we verify the new multi-pack-index. If the new multi-pack-index is corrupt, then delete the multi-pack-index, rewrite it from scratch, and stop doing the later steps of the job. This is intended to be an extra-safe check without leaving a repo with many pack-files without a multi-pack-index.
These steps are based on a similar background maintenance step in Scalar (and VFS for Git) [1]. This was incredibly effective for users of the Windows OS repository. After using the same VFS for Git repository for over a year, some users had _thousands_ of pack-files that combined to up to 250 GB of data. We noticed a few users were running into the open file descriptor limits (due in part to a bug in the multi-pack-index fixed by af96fe3 (midx: add packs to packed_git linked list, 2019-04-29).
These pack-files were mostly small since they contained the commits and trees that were pushed to the origin in a given hour. The GVFS protocol includes a "prefetch" step that asks for pre-computed pack- files containing commits and trees by timestamp. These pack-files were grouped into "daily" pack-files once a day for up to 30 days. If a user did not request prefetch packs for over 30 days, then they would get the entire history of commits and trees in a new, large pack-file. This led to a large number of pack-files that had poor delta compression.
By running this pack-file maintenance step once per day, these repos with thousands of packs spanning 200+ GB dropped to dozens of pack- files spanning 30-50 GB. This was done all without removing objects from the system and using a constant batch size of two gigabytes. Once the work was done to reduce the pack-files to small sizes, the batch size of two gigabytes means that not every run triggers a repack operation, so the following run will not expire a pack-file. This has kept these repos in a "clean" state.
[1] https://github.com/microsoft/scalar/blob/master/Scalar.Common/Maintenance/PackfileMaintenanceStep.cs
--- Documentation/git-maintenance.txt | 15 ++++ builtin/gc.c | 112 ++++++++++++++++++++++++++++++ midx.c | 2 +- midx.h | 1 + t/t5319-multi-pack-index.sh | 1 + t/t7900-maintenance.sh | 38 ++++++++++ 6 files changed, 168 insertions(+), 1 deletion(-)
Documentation/git-maintenance.txt +15 -0
@@ -87,6 +87,21 @@ loose-objects:: thousand objects to prevent the job from taking too long on a repository with many loose objects. +incremental-repack:: + The `incremental-repack` job repacks the object directory + using the `multi-pack-index` feature. In order to prevent race + conditions with concurrent Git commands, it follows a two-step + process. First, it deletes any pack-files included in the + `multi-pack-index` where none of the objects in the + `multi-pack-index` reference those pack-files; this only happens + if all objects in the pack-file are also stored in a newer + pack-file. Second, it selects a group of pack-files whose "expected + size" is below the batch size until the group has total expected + size at least the batch size; see the `--batch-size` option for + the `repack` subcommand in linkgit:git-multi-pack-index[1]. The + default batch-size is zero, which is a special case that attempts + to repack all pack-files into a single pack-file. + OPTIONS ------- --auto::
builtin/gc.c +112 -0
136 lines not on this page · raw message
midx.c +1 -1
@@ -37,7 +37,7 @@#define PACK_EXPIRED UINT_MAX -static char *get_midx_filename (const char *object_dir) +char *get_midx_filename (const char *object_dir) {return xstrfmt ("%s/pack/multi-pack-index" , object_dir); }
midx.h +1 -0
@@ -37,6 +37,7 @@ struct multi_pack_index {#define MIDX_PROGRESS (1 << 0) +char *get_midx_filename (const char *object_dir);struct multi_pack_index *load_multi_pack_index (const char *object_dir,int local);int prepare_midx_pack (struct repository *r,struct multi_pack_index *m,uint32_t pack_int_id);int bsearch_midx (const struct object_id *oid,struct multi_pack_index *m,uint32_t *result);
t/t5319-multi-pack-index.sh +1 -0
@@ -3,6 +3,7 @@ test_description='multi-pack-indexes' . ./test-lib.sh +GIT_TEST_MULTI_PACK_INDEX=0 objdir=.git/objects midx_read_expect () {
t/t7900-maintenance.sh +38 -0
@@ -5,6 +5,7 @@ test_description='git maintenance builtin' . ./test-lib.sh GIT_TEST_COMMIT_GRAPH=0 +GIT_TEST_MULTI_PACK_INDEX=0 test_expect_success 'help text' ' test_expect_code 129 git maintenance -h 2>err &&
@@ -106,4 +107,41 @@ test_expect_success 'loose-objects task' ' test_cmp packs-between packs-after ' +test_expect_success 'incremental-repack task' ' + packDir=.git/objects/pack && + for i in $(test_seq 1 5) + do + test_commit $i || return 1 + done && + + # Create three disjoint pack-files with size BIG, small, small. + echo HEAD~2 | git pack-objects --revs $packDir/test-1 && + test_tick && + git pack-objects --revs $packDir/test-2 <<-\EOF && + HEAD~1 + ^HEAD~2 + EOF + test_tick && + git pack-objects --revs $packDir/test-3 <<-\EOF && + HEAD + ^HEAD~1 + EOF + rm -f $packDir/pack-* && + rm -f $packDir/loose-* && + ls $packDir/*.pack >packs-before && + test_line_count = 3 packs-before && + + # the job repacks the two into a new pack, but does not + # delete the old ones. + git maintenance run --task=incremental-repack && + ls $packDir/*.pack >packs-between && + test_line_count = 4 packs-between && + + # the job deletes the two old packs, and does not write + # a new one because only one pack remains. + git maintenance run --task=incremental-repack && + ls .git/objects/pack/*.pack >packs-after && + test_line_count = 1 packs-after +' + test_done
Signature · 1 line
gitgitgadget
PATCH v3 13/20 maintenance: auto-size incremental-repack batch
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
When repacking during the 'incremental-repack' task, we use the --batch-size option in 'git multi-pack-index repack'. The initial setting used --batch-size=0 to repack everything into a single pack-file. This is not sustainable for a large repository. The amount of work required is also likely to use too many system resources for a background job.
Update the 'incremental-repack' task by dynamically computing a --batch-size option based on the current pack-file structure.
The dynamic default size is computed with this idea in mind for a client repository that was cloned from a very large remote: there is likely one "big" pack-file that was created at clone time. Thus, do not try repacking it as it is likely packed efficiently by the server.
Instead, we select the second-largest pack-file, and create a batch size that is one larger than that pack-file. If there are three or more pack-files, then this guarantees that at least two will be combined into a new pack-file.
Of course, this means that the second-largest pack-file size is likely to grow over time and may eventually surpass the initially-cloned pack-file. Recall that the pack-file batch is selected in a greedy
manner: the packs are considered from oldest to newest and are selected if they have size smaller than the batch size until the total selected size is larger than the batch size. Thus, that oldest "clone" pack will be first to repack after the new data creates a pack larger than that.
We also want to place some limits on how large these pack-files become, in order to bound the amount of time spent repacking. A maximum batch-size of two gigabytes means that large repositories will never be packed into a single pack-file using this job, but also that repack is rather expensive. This is a trade-off that is valuable to have if the maintenance is being run automatically or in the background. Users who truly want to optimize for space and performance (and are willing to pay the upfront cost of a full repack) can use the 'gc' task to do so.
--- builtin/gc.c | 43 +++++++++++++++++++++++++++++++++++++++++- t/t7900-maintenance.sh | 5 +++-- 2 files changed, 45 insertions(+), 3 deletions(-)
builtin/gc.c +42 -1
@@ -988,6 +988,46 @@ static int multi_pack_index_expire(void)return 0 ; } +#define TWO_GIGABYTES (0x7FFF) + +static off_t get_auto_pack_size (void ) +{ +/* +* The "auto" value is special: we optimize for +* one large pack-file (i.e. from a clone) and +* expect the rest to be small and they can be +* repacked quickly. +* +* The strategy we select here is to select a +* size that is one more than the second largest +* pack-file. This ensures that we will repack +* at least two packs if there are three or more +* packs. +*/ +off_t max_size =0 ; +off_t second_largest_size =0 ; +off_t result_size; +struct packed_git *p; +struct repository *r = the_repository; + +reprepare_packed_git (r); +for (p =get_all_packs (r); p; p = p->next) { +if (p->pack_size > max_size) { + second_largest_size = max_size; + max_size = p->pack_size; + }else if (p->pack_size > second_largest_size) + second_largest_size = p->pack_size; + } + + result_size = second_largest_size +1 ; + +/* But limit ourselves to a batch size of 2g */ +if (result_size >TWO_GIGABYTES ) + result_size =TWO_GIGABYTES ; + +return result_size; +} +static int multi_pack_index_repack (void ) {struct child_process child =CHILD_PROCESS_INIT ;
@@ -998,7 +1038,8 @@ static int multi_pack_index_repack(void)if (opts.quiet)strvec_push (&child.args,"--no-progress" ); -strvec_push (&child.args,"--batch-size=0" ); +strvec_pushf (&child.args,"--batch-size=%" PRIuMAX, + (uintmax_t )get_auto_pack_size ());close_object_store (the_repository->objects);
t/t7900-maintenance.sh +3 -2
@@ -138,10 +138,11 @@ test_expect_success 'incremental-repack task' ' test_line_count = 4 packs-between && # the job deletes the two old packs, and does not write - # a new one because only one pack remains. + # a new one because the batch size is not high enough to + # pack the largest pack-file. git maintenance run --task=incremental-repack && ls .git/objects/pack/*.pack >packs-after && - test_line_count = 1 packs-after + test_line_count = 2 packs-after ' test_done
Signature · 1 line
gitgitgadget
Re: [PATCH v3 13/20] maintenance: auto-size incremental-repack batch
builtin/gc.c static int multi_pack_index_expire(void) · line 991
Quoted 9 lines · Derrick Stolee via GitGitGadget
builtin/gc.c +1
-0@@ -988,6 +988,46 @@ static int multi_pack_index_expire(void)return 0 ; } +#define TWO_GIGABYTES (0x7FFF)
You meant (0x7FFFFFFF) here, right?
Chris
Re: [PATCH v3 13/20] maintenance: auto-size incremental-repack batch
Quoted 10 lines · Chris Torek
Quoted 8 lines · Derrick Stolee via GitGitGadget
builtin/gc.c +0
-0@@ -988,6 +988,46 @@ static int multi_pack_index_expire(void)return 0 ; }
Quoted 3 lines
builtin/gc.c static int multi_pack_index_expire(void) · line 991
Quoted 1 line
builtin/gc.c +1
-0+#define TWO_GIGABYTES (0x7FFF)You meant (
0x7FFFFFFF) here, right?
Ugh. Of course. Thanks for being diligent here. I hadn't pulled this branch into microsoft/git for testing with Scalar yet, where I would have noticed this.
I'm tempted to create an EXPENSIVE test that actually checks this condition. Let's see what I can do.
Thanks,
-Stolee
PATCH v3 14/20 maintenance: create maintenance.<task>.enabled config
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Currently, a normal run of "git maintenance run" will only run the 'gc' task, as it is the only one enabled. This is mostly for backwards- compatible reasons since "git maintenance run --auto" commands replaced previous "git gc --auto" commands after some Git processes. Users could manually run specific maintenance tasks by calling "git maintenance run --task=<task>" directly.
Allow users to customize which steps are run automatically using config. The 'maintenance.<task>.enabled' option then can turn on these other tasks (or turn off the 'gc' task).
--- Documentation/config.txt | 2 ++ Documentation/config/maintenance.txt | 4 ++++ Documentation/git-maintenance.txt | 6 +++++- builtin/gc.c | 19 +++++++++++++++++++ t/t7900-maintenance.sh | 12 ++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 Documentation/config/maintenance.txt
Documentation/config.txt +2 -0
@@ -396,6 +396,8 @@ include::config/mailinfo.txt[] include::config/mailmap.txt[] +include::config/maintenance.txt[] + include::config/man.txt[] include::config/merge.txt[]
Documentation/config/maintenance.txt new file +4 -0
@@ -0,0 +1,4 @@ +maintenance.<task>.enabled:: + This boolean config option controls whether the maintenance task + with name `<task>` is run when no `--task` option is specified. + By default, only `maintenance.gc.enabled` is true.
Documentation/git-maintenance.txt +5 -1
@@ -30,7 +30,11 @@ SUBCOMMANDS ----------- run:: - Run one or more maintenance tasks. + Run one or more maintenance tasks. If one or more `--task` options + are specified, then those tasks are run in that order. Otherwise, + the tasks are determined by which `maintenance.<task>.enabled` + config options are true. By default, only `maintenance.gc.enabled` + is true. TASKS -----
builtin/gc.c +19 -0
@@ -1171,6 +1171,24 @@ static int maintenance_run(void)return result; } +static void initialize_task_config (void ) +{ +int i; +struct strbuf config_name =STRBUF_INIT ; +for (i =0 ; i <TASK__COUNT ; i++) { +int config_value; + +strbuf_setlen (&config_name,0 ); +strbuf_addf (&config_name,"maintenance.%s.enabled" , + tasks[i].name); + +if (!git_config_get_bool (config_name.buf, &config_value)) + tasks[i].enabled = config_value; + } + +strbuf_release (&config_name); +} +static int task_option_parse (const struct option *opt,const char *arg,int unset) {
@@ -1224,6 +1242,7 @@ int cmd_maintenance(int argc, const char **argv, const char *prefix) builtin_maintenance_options); opts.quiet = !isatty (2 ); +initialize_task_config (); argc = parse_options(argc, argv, prefix, builtin_maintenance_options,
t/t7900-maintenance.sh +12 -0
@@ -21,6 +21,18 @@ test_expect_success 'run [--auto|--quiet]' ' grep ",\"gc\",\"--quiet\"" run-quiet.txt ' +test_expect_success 'maintenance.<task>.enabled' ' + git config maintenance.gc.enabled false && + git config maintenance.commit-graph.enabled true && + git config maintenance.loose-objects.enabled true && + GIT_TRACE2_EVENT="$(pwd)/run-config.txt" git maintenance run && + ! grep ",\"fetch\"" run-config.txt && + ! grep ",\"gc\"" run-config.txt && + ! grep ",\"multi-pack-index\"" run-config.txt && + grep ",\"commit-graph\"" run-config.txt && + grep ",\"prune-packed\"" run-config.txt +' + test_expect_success 'run --task=<task>' ' GIT_TRACE2_EVENT="$(pwd)/run-commit-graph.txt" git maintenance run --task=commit-graph && GIT_TRACE2_EVENT="$(pwd)/run-gc.txt" git maintenance run --task=gc &&
Signature · 1 line
gitgitgadget
PATCH v3 15/20 maintenance: use pointers to check --auto
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The 'git maintenance run' command has an '--auto' option. This is used by other Git commands such as 'git commit' or 'git fetch' to check if maintenance should be run after adding data to the repository.
Previously, this --auto option was only used to add the argument to the 'git gc' command as part of the 'gc' task. We will be expanding the other tasks to perform a check to see if they should do work as part of the --auto flag, when they are enabled by config.
First, update the 'gc' task to perform the auto check inside the maintenance process. This prevents running an extra 'git gc --auto' command when not needed. It also shows a model for other tasks.
Second, use the 'auto_condition' function pointer as a signal for whether we enable the maintenance task under '--auto'. For instance, we do not want to enable the 'fetch' task in '--auto' mode, so that function pointer will remain NULL.
Now that we are not automatically calling 'git gc', a test in t5514-fetch-multiple.sh must be changed to watch for 'git maintenance' instead.
We continue to pass the '--auto' option to the 'git gc' command when necessary, because of the gc.autoDetach config option changes behavior. Likely, we will want to absorb the daemonizing behavior implied by gc.autoDetach as a maintenance.autoDetach config option.
--- builtin/gc.c | 16 ++++++++++++++++ t/t5514-fetch-multiple.sh | 2 +- t/t7900-maintenance.sh | 2 +- 3 files changed, 18 insertions(+), 2 deletions(-)
builtin/gc.c +16 -0
@@ -1074,9 +1074,17 @@ static int maintenance_task_incremental_repack(void)typedef int maintenance_task_fn (void ); +/* +* An auto condition function returns 1 if the task should run +* and 0 if the task should NOT run. See needs_to_gc() for an +* example. +*/ +typedef int maintenance_auto_fn (void ); +struct maintenance_task {const char *name;maintenance_task_fn *fn; +maintenance_auto_fn *auto_condition;unsigned enabled:1 , selected:1 ;int selected_order;
@@ -1109,6 +1117,7 @@ static struct maintenance_task tasks[] = { [TASK_GC] = {"gc" , maintenance_task_gc, + need_to_gc, 1, }, [TASK_COMMIT_GRAPH] = {
@@ -1161,6 +1170,11 @@ static int maintenance_run(void)if (!opts.tasks_selected && !tasks[i].enabled)continue ; +if (opts.auto_flag && + (!tasks[i].auto_condition || + !tasks[i].auto_condition ())) +continue ; +if (tasks[i].fn ()) {error (_ ("task '%s' failed" ), tasks[i].name); result =1 ;
@@ -1175,6 +1189,8 @@ static void initialize_task_config(void) {int i;struct strbuf config_name =STRBUF_INIT ; +gc_config (); +for (i =0 ; i <TASK__COUNT ; i++) {int config_value;
t/t5514-fetch-multiple.sh +1 -1
@@ -108,7 +108,7 @@ test_expect_success 'git fetch --multiple (two remotes)' ' GIT_TRACE=1 git fetch --multiple one two 2>trace && git branch -r > output && test_cmp ../expect output && - grep "built-in: git gc" trace >gc && + grep "built-in: git maintenance" trace >gc && test_line_count = 1 gc ) '
t/t7900-maintenance.sh +1 -1
@@ -17,7 +17,7 @@ test_expect_success 'run [--auto|--quiet]' ' GIT_TRACE2_EVENT="$(pwd)/run-auto.txt" git maintenance run --auto && GIT_TRACE2_EVENT="$(pwd)/run-quiet.txt" git maintenance run --quiet && grep ",\"gc\"]" run-no-auto.txt && - grep ",\"gc\",\"--auto\"" run-auto.txt && + ! grep ",\"gc\",\"--auto\"" run-auto.txt && grep ",\"gc\",\"--quiet\"" run-quiet.txt '
Signature · 1 line
gitgitgadget
PATCH v3 16/20 maintenance: add auto condition for commit-graph task
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Instead of writing a new commit-graph in every 'git maintenance run --auto' process (when maintenance.commit-graph.enalbed is configured to be true), only write when there are "enough" commits not in a commit-graph file.
This count is controlled by the maintenance.commit-graph.auto config option.
To compute the count, use a depth-first search starting at each ref, and leaving markers using the PARENT1 flag. If this count reaches the limit, then terminate early and start the task. Otherwise, this operation will peel every ref and parse the commit it points to. If these are all in the commit-graph, then this is typically a very fast operation. Users with many refs might feel a slow-down, and hence could consider updating their limit to be very small. A negative value will force the step to run every time.
--- Documentation/config/maintenance.txt | 10 ++++ builtin/gc.c | 76 ++++++++++++++++++++++++++++ object.h | 1 + 3 files changed, 87 insertions(+)
Documentation/config/maintenance.txt +10 -0
@@ -2,3 +2,13 @@ maintenance.<task>.enabled:: This boolean config option controls whether the maintenance task with name `<task>` is run when no `--task` option is specified. By default, only `maintenance.gc.enabled` is true. + +maintenance.commit-graph.auto:: + This integer config option controls how often the `commit-graph` task + should be run as part of `git maintenance run --auto`. If zero, then + the `commit-graph` task will not run with the `--auto` option. A + negative value will force the task to run every time. Otherwise, a + positive value implies the command should run when the number of + reachable commits that are not in the commit-graph file is at least + the value of `maintenance.commit-graph.auto`. The default value is + 100.
builtin/gc.c +76 -0
94 lines not on this page · raw message
object.h +1 -0
@@ -74,6 +74,7 @@ struct object_array {* list-objects-filter.c: 21 * builtin/fsck.c: 0--3 * builtin/index-pack.c: 2021 +* builtin/maintenance.c: 16 * builtin/pack-objects.c: 20 * builtin/reflog.c: 10--12 * builtin/show-branch.c: 0-------------------------------------------26
Signature · 1 line
gitgitgadget
PATCH v3 17/20 maintenance: create auto condition for loose-objects
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The loose-objects task deletes loose objects that already exist in a pack-file, then place the remaining loose objects into a new pack-file. If this step runs all the time, then we risk creating pack-files with very few objects with every 'git commit' process. To prevent overwhelming the packs directory with small pack-files, place a minimum number of objects to justify the task.
The 'maintenance.loose-objects.auto' config option specifies a minimum number of loose objects to justify the task to run under the '--auto' option. This defaults to 100 loose objects. Setting the value to zero will prevent the step from running under '--auto' while a negative value will force it to run every time.
--- Documentation/config/maintenance.txt | 9 +++++++++ builtin/gc.c | 30 ++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 25 +++++++++++++++++++++++ 3 files changed, 64 insertions(+)
Documentation/config/maintenance.txt +9 -0
@@ -12,3 +12,12 @@ maintenance.commit-graph.auto:: reachable commits that are not in the commit-graph file is at least the value of `maintenance.commit-graph.auto`. The default value is 100. + +maintenance.loose-objects.auto:: + This integer config option controls how often the `loose-objects` task + should be run as part of `git maintenance run --auto`. If zero, then + the `loose-objects` task will not run with the `--auto` option. A + negative value will force the task to run every time. Otherwise, a + positive value implies the command should run when the number of + loose objects is at least the value of `maintenance.loose-objects.auto`. + The default value is 100.
builtin/gc.c +30 -0
@@ -927,6 +927,35 @@ struct write_loose_object_data {int batch_size; }; +static int loose_object_auto_limit =100 ; + +static int loose_object_count (const struct object_id *oid, +const char *path, +void *data) +{ +int *count = (int *)data; +if (++(*count) >= loose_object_auto_limit) +return 1 ; +return 0 ; +} + +static int loose_object_auto_condition (void ) +{ +int count =0 ; + +git_config_get_int ("maintenance.loose-objects.auto" , + &loose_object_auto_limit); + +if (!loose_object_auto_limit) +return 0 ; +if (loose_object_auto_limit <0 ) +return 1 ; + +return for_each_loose_file_in_objdir (the_repository->objects->odb->path, + loose_object_count, +NULL ,NULL , &count); +} +static int bail_on_loose (const struct object_id *oid,const char *path,void *data)
@@ -1184,6 +1213,7 @@ static struct maintenance_task tasks[] = { [TASK_LOOSE_OBJECTS] = {"loose-objects" , maintenance_task_loose_objects, + loose_object_auto_condition, }, [TASK_INCREMENTAL_REPACK] = { "incremental-repack",
t/t7900-maintenance.sh +25 -0
@@ -119,6 +119,31 @@ test_expect_success 'loose-objects task' ' test_cmp packs-between packs-after ' +test_expect_success 'maintenance.loose-objects.auto' ' + git repack -adk && + GIT_TRACE2_EVENT="$(pwd)/trace-lo1.txt" \ + git -c maintenance.loose-objects.auto=1 maintenance \ + run --auto --task=loose-objects && + ! grep "\"prune-packed\"" trace-lo1.txt && + for i in 1 2 + do + printf data-A-$i | git hash-object -t blob --stdin -w && + GIT_TRACE2_EVENT="$(pwd)/trace-loA-$i" \ + git -c maintenance.loose-objects.auto=2 \ + maintenance run --auto --task=loose-objects && + ! grep "\"prune-packed\"" trace-loA-$i && + printf data-B-$i | git hash-object -t blob --stdin -w && + GIT_TRACE2_EVENT="$(pwd)/trace-loB-$i" \ + git -c maintenance.loose-objects.auto=2 \ + maintenance run --auto --task=loose-objects && + grep "\"prune-packed\"" trace-loB-$i && + GIT_TRACE2_EVENT="$(pwd)/trace-loC-$i" \ + git -c maintenance.loose-objects.auto=2 \ + maintenance run --auto --task=loose-objects && + grep "\"prune-packed\"" trace-loC-$i || return 1 + done +' + test_expect_success 'incremental-repack task' ' packDir=.git/objects/pack && for i in $(test_seq 1 5)
Signature · 1 line
gitgitgadget
PATCH v3 18/20 maintenance: add incremental-repack auto condition
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The incremental-repack task updates the multi-pack-index by deleting pack- files that have been replaced with new packs, then repacking a batch of small pack-files into a larger pack-file. This incremental repack is faster than rewriting all object data, but is slower than some other maintenance activities.
The 'maintenance.incremental-repack.auto' config option specifies how many pack-files should exist outside of the multi-pack-index before running the step. These pack-files could be created by 'git fetch' commands or by the loose-objects task. The default value is 10.
Setting the option to zero disables the task with the '--auto' option, and a negative value makes the task run every time.
--- Documentation/config/maintenance.txt | 9 ++++++++ builtin/gc.c | 31 ++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 30 +++++++++++++++++++++++++++ 3 files changed, 70 insertions(+)
Documentation/config/maintenance.txt +9 -0
@@ -21,3 +21,12 @@ maintenance.loose-objects.auto:: positive value implies the command should run when the number of loose objects is at least the value of `maintenance.loose-objects.auto`. The default value is 100. + +maintenance.incremental-repack.auto:: + This integer config option controls how often the `incremental-repack` + task should be run as part of `git maintenance run --auto`. If zero, + then the `incremental-repack` task will not run with the `--auto` + option. A negative value will force the task to run every time. + Otherwise, a positive value implies the command should run when the + number of pack-files not in the multi-pack-index is at least the value + of `maintenance.incremental-repack.auto`. The default value is 10.
builtin/gc.c +31 -0
@@ -31,6 +31,7 @@#include "remote.h" #include "midx.h" #include "refs.h" +#include "object-store.h" #define FAILED_RUN "failed to run %s"
@@ -1029,6 +1030,35 @@ static int maintenance_task_loose_objects(void)return prune_packed () ||pack_loose (); } +static int incremental_repack_auto_condition (void ) +{ +struct packed_git *p; +int enabled; +int incremental_repack_auto_limit =10 ; +int count =0 ; + +if (git_config_get_bool ("core.multiPackIndex" , &enabled) || + !enabled) +return 0 ; + +git_config_get_int ("maintenance.incremental-repack.auto" , + &incremental_repack_auto_limit); + +if (!incremental_repack_auto_limit) +return 0 ; +if (incremental_repack_auto_limit <0 ) +return 1 ; + +for (p =get_packed_git (the_repository); + count < incremental_repack_auto_limit && p; + p = p->next) { +if (!p->multi_pack_index) + count++; + } + +return count >= incremental_repack_auto_limit; +} +static int multi_pack_index_write (void ) {struct child_process child =CHILD_PROCESS_INIT ;
@@ -1218,6 +1248,7 @@ static struct maintenance_task tasks[] = { [TASK_INCREMENTAL_REPACK] = {"incremental-repack" , maintenance_task_incremental_repack, + incremental_repack_auto_condition, }, [TASK_GC] = { "gc",
t/t7900-maintenance.sh +30 -0
@@ -182,4 +182,34 @@ test_expect_success 'incremental-repack task' ' test_line_count = 2 packs-after ' +test_expect_success 'maintenance.incremental-repack.auto' ' + git repack -adk && + git config core.multiPackIndex true && + git multi-pack-index write && + GIT_TRACE2_EVENT=1 git -c maintenance.incremental-repack.auto=1 \ + maintenance run --auto --task=incremental-repack >out && + ! grep "\"multi-pack-index\"" out && + for i in 1 2 + do + test_commit A-$i && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-A-$i git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack && + ! grep "\"multi-pack-index\"" trace-A-$i && + test_commit B-$i && + git pack-objects --revs .git/objects/pack/pack <<-\EOF && + HEAD + ^HEAD~1 + EOF + GIT_TRACE2_EVENT=$(pwd)/trace-B-$i git \ + -c maintenance.incremental-repack.auto=2 \ + maintenance run --auto --task=incremental-repack >out && + grep "\"multi-pack-index\"" trace-B-$i >/dev/null || return 1 + done +' + test_done
Signature · 1 line
gitgitgadget
PATCH v3 19/20 midx: use start_delayed_progress()
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Now that the multi-pack-index may be written as part of auto maintenance at the end of a command, reduce the progress output when the operations are quick. Use start_delayed_progress() instead of start_progress().
Update t5319-multi-pack-index.sh to use GIT_PROGRESS_DELAY=0 now that the progress indicators are conditional.
--- midx.c | 10 +++++----- t/t5319-multi-pack-index.sh | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-)
midx.c +5 -5
@@ -832,7 +832,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * packs.pack_paths_checked =0 ;if (flags &MIDX_PROGRESS ) - packs.progress =start_progress (_ ("Adding packfiles to multi-pack-index" ),0 ); + packs.progress =start_delayed_progress (_ ("Adding packfiles to multi-pack-index" ),0 );else packs.progress =NULL ;
@@ -969,7 +969,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * }if (flags &MIDX_PROGRESS ) - progress =start_progress (_ ("Writing chunks to multi-pack-index" ), + progress =start_delayed_progress (_ ("Writing chunks to multi-pack-index" ), num_chunks);for (i =0 ; i < num_chunks; i++) {if (written != chunk_offsets[i])
@@ -1104,7 +1104,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flagreturn 0 ;if (flags &MIDX_PROGRESS ) - progress =start_progress (_ ("Looking for referenced packfiles" ), + progress =start_delayed_progress (_ ("Looking for referenced packfiles" ), m->num_packs);for (i =0 ; i < m->num_packs; i++) {if (prepare_midx_pack (r, m, i))
@@ -1225,7 +1225,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla count =xcalloc (m->num_packs,sizeof (uint32_t ));if (flags &MIDX_PROGRESS ) - progress =start_progress (_ ("Counting referenced objects" ), + progress =start_delayed_progress (_ ("Counting referenced objects" ), m->num_objects);for (i =0 ; i < m->num_objects; i++) {int pack_int_id =nth_midxed_pack_int_id (m, i);
@@ -1235,7 +1235,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flastop_progress (&progress);if (flags &MIDX_PROGRESS ) - progress =start_progress (_ ("Finding and deleting unreferenced packfiles" ), + progress =start_delayed_progress (_ ("Finding and deleting unreferenced packfiles" ), m->num_packs);for (i =0 ; i < m->num_packs; i++) {char *pack_name;
t/t5319-multi-pack-index.sh +7 -7
@@ -173,12 +173,12 @@ test_expect_success 'write progress off for redirected stderr' ' ' test_expect_success 'write force progress on for stderr' ' - git multi-pack-index --object-dir=$objdir --progress write 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress write 2>err && test_file_not_empty err ' test_expect_success 'write with the --no-progress option' ' - git multi-pack-index --object-dir=$objdir --no-progress write 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress write 2>err && test_line_count = 0 err '
@@ -335,17 +335,17 @@ test_expect_success 'git-fsck incorrect offset' ' ' test_expect_success 'repack progress off for redirected stderr' ' - git multi-pack-index --object-dir=$objdir repack 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir repack 2>err && test_line_count = 0 err ' test_expect_success 'repack force progress on for stderr' ' - git multi-pack-index --object-dir=$objdir --progress repack 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress repack 2>err && test_file_not_empty err ' test_expect_success 'repack with the --no-progress option' ' - git multi-pack-index --object-dir=$objdir --no-progress repack 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress repack 2>err && test_line_count = 0 err '
@@ -489,7 +489,7 @@ test_expect_success 'expire progress off for redirected stderr' ' test_expect_success 'expire force progress on for stderr' ' ( cd dup && - git multi-pack-index --progress expire 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --progress expire 2>err && test_file_not_empty err ) '
@@ -497,7 +497,7 @@ test_expect_success 'expire force progress on for stderr' ' test_expect_success 'expire with the --no-progress option' ' ( cd dup && - git multi-pack-index --no-progress expire 2>err && + GIT_PROGRESS_DELAY=0 git multi-pack-index --no-progress expire 2>err && test_line_count = 0 err ) '
Signature · 1 line
gitgitgadget
PATCH v3 20/20 maintenance: add trace2 regions for task execution
In reply to Derrick Stolee via GitGitGadgetFrom: Derrick Stolee <[email protected]>
--- builtin/gc.c | 2 ++ 1 file changed, 2 insertions(+)
builtin/gc.c +2 -0
@@ -1312,10 +1312,12 @@ static int maintenance_run(void) !tasks[i].auto_condition()))continue ; +trace2_region_enter ("maintenance" , tasks[i].name, r);if (tasks[i].fn ()) {error (_ ("task '%s' failed" ), tasks[i].name); result =1 ; } +trace2_region_leave ("maintenance" , tasks[i].name, r); }rollback_lock_file (&lk);
Signature · 1 line
gitgitgadget
Quoted 3 lines · Derrick Stolee via GitGitGadget
* I added and updates Junio's patch from jc/no-update-fetch-head into the proper place before the 'prefetch' task. The 'prefetch' task now uses --no-write-fetch-head to avoid issues with
FETCH_HEAD.
The update to tests for "git pull" looks reasonable, too.
Thanks.
Quoted 4 lines · Derrick Stolee via GitGitGadget
* This series is now based on jk/strvec, as there are several places where I was adding new callers to argv_array_push* and
run_command_v_opt()which have been replaced with strvec_push*() andrun_command(), using a 'structchild_process'.
It probably has been a good way to gain experience with the renamed API for you to rebase this series, but I think we'd have to spend a similar amount of effort yet again, as I expect that the jk/strvec series would be rerolled at least once more, with the items[] array renamed to something else, like v[].
Let's see how it goes.
Thanks.
Quoted 7 lines · Junio C Hamano
Quoted 4 lines · Derrick Stolee via GitGitGadget
* This series is now based on jk/strvec, as there are several places where I was adding new callers to argv_array_push* and
run_command_v_opt()which have been replaced with strvec_push*() andrun_command(), using a 'structchild_process'.
Quoted 7 lines
It probably has been a good way to gain experience with the renamed API for you to rebase this series, but I think we'd have to spend a similar amount of effort yet again, as I expect that the jk/strvec series would be rerolled at least once more, with the items[] array renamed to something else, like v[].
Let's see how it goes.
So, ... jk/strvec got updated, and just applying these twenty patches on top no longer compiles, so I rebased them ;-)
I also adjusted the merge-fix for the 6 patches on es/config-hooks topic before making tonight's final push-out of 'seen'.
Thanks.
Quoted 1 line · Derrick Stolee via GitGitGadget
This is a second attempt...
Just chiming in on this version to point out that what would have been v4 of this series is now split across two series:
[1] https://lore.kernel.org/git/[email protected]/
[2] https://lore.kernel.org/git/[email protected]/
Thanks,
-Stolee