maintenance: use random minute in systemd scheduler

The get_random_minute() method was created to allow maintenance schedules to be fixed to a random minute of the hour. This randomness is only intended to spread out the load from a number of clients, but each client should have an hour between each maintenance cycle. Add this random minute to the systemd integration. This integration is more complicated than similar changes for other schedulers because of a neat trick that systemd allows: templating. The previous implementation generated two template files with names of the form 'git-maintenance@.(timer|service)'. The '.timer' or '.service' indicates that this is a template that is picked up when we later specify '...@<schedule>.timer' or '...@<schedule>.service'. The '<schedule>' string is then used to insert into the template both the 'OnCalendar' schedule setting and the '--schedule' parameter of the 'git maintenance run' command. In order to set these schedules to a given minute, we can no longer use the 'hourly', 'daily', or 'weekly' strings for '<schedule>' and instead need to abandon the template model for the .timer files. We can still use templates for the .service files. For this reason, we split these writes into two methods. Modify the template with a custom schedule in the 'OnCalendar' setting. This schedule has some interesting differences from cron-like patterns, but is relatively easy to figure out from context. The one that might be confusing is that '*-*-*' is a date-based pattern, but this must be omitted when using 'Mon' to signal that we care about the day of the week. Monday is used since that matches the day used for the 'weekly' schedule used previously. Now that the timer files are not templates, we might want to abandon the '@' symbol in the file names. However, this would cause users with existing schedules to get two competing schedules due to different names. The work to remove the old schedule name is one thing that we can avoid by keeping the '@' symbol in our unit names. Since we are locked into this name, it makes sense that we keep the template model for the .service files. The rest of the change involves making sure we are writing these .timer and .service files before initializing the schedule with 'systemctl' and deleting the files when we are done. Some changes are also made to share the random minute along with a single computation of the execution path of the current Git executable. In addition, older Git versions may have written a 'git-maintenance@.timer' template file. Be sure to remove this when successfully enabling maintenance (or disabling maintenance). Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Aug 10, 2023 at 20:39 UTC daa787010c82b32c3fdc8a2bf2a4ce90fff5d5b0
2 files changed +142 -25
builtin/gc.c
+129 -23
@@ -2292,29 +2292,54 @@ static char *xdg_config_home_systemd(const char *filename)
2292 return xdg_config_home_for("systemd/user", filename);
2293 }
2294
2295 -static int systemd_timer_delete_unit_templates(void)
2295 +#define SYSTEMD_UNIT_FORMAT "git-maintenance@%s.%s"
2296 +
2297 +static int systemd_timer_delete_timer_file(enum schedule_priority priority)
2298 {
2299 int ret = 0;
2298 - char *filename = xdg_config_home_systemd("git-maintenance@.timer");
2300 + const char *frequency = get_frequency(priority);
2301 + char *local_timer_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "timer");
2302 + char *filename = xdg_config_home_systemd(local_timer_name);
2303 +
2304 if (unlink(filename) && !is_missing_file_error(errno))
2305 ret = error_errno(_("failed to delete '%s'"), filename);
2301 - FREE_AND_NULL(filename);
2306
2303 - filename = xdg_config_home_systemd("git-maintenance@.service");
2307 + free(filename);
2308 + free(local_timer_name);
2309 + return ret;
2310 +}
2311 +
2312 +static int systemd_timer_delete_service_template(void)
2313 +{
2314 + int ret = 0;
2315 + char *local_service_name = xstrfmt(SYSTEMD_UNIT_FORMAT, "", "service");
2316 + char *filename = xdg_config_home_systemd(local_service_name);
2317 if (unlink(filename) && !is_missing_file_error(errno))
2318 ret = error_errno(_("failed to delete '%s'"), filename);
2319
2320 free(filename);
2321 + free(local_service_name);
2322 return ret;
2323 }
2324
2311 -static int systemd_timer_write_unit_templates(const char *exec_path)
2325 +/*
2326 + * Write the schedule information into a git-maintenance@<schedule>.timer
2327 + * file using a custom minute. This timer file cannot use the templating
2328 + * system, so we generate a specific file for each.
2329 + */
2330 +static int systemd_timer_write_timer_file(enum schedule_priority schedule,
2331 + int minute)
2332 {
2333 + int res = -1;
2334 char *filename;
2335 FILE *file;
2336 const char *unit;
2337 + char *schedule_pattern = NULL;
2338 + const char *frequency = get_frequency(schedule);
2339 + char *local_timer_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "timer");
2340 +
2341 + filename = xdg_config_home_systemd(local_timer_name);
2342
2317 - filename = xdg_config_home_systemd("git-maintenance@.timer");
2343 if (safe_create_leading_directories(filename)) {
2344 error(_("failed to create directories for '%s'"), filename);
2345 goto error;
@@ -2323,6 +2348,23 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
2348 if (!file)
2349 goto error;
2350
2351 + switch (schedule) {
2352 + case SCHEDULE_HOURLY:
2353 + schedule_pattern = xstrfmt("*-*-* *:%02d:00", minute);
2354 + break;
2355 +
2356 + case SCHEDULE_DAILY:
2357 + schedule_pattern = xstrfmt("*-*-* 0:%02d:00", minute);
2358 + break;
2359 +
2360 + case SCHEDULE_WEEKLY:
2361 + schedule_pattern = xstrfmt("Mon 0:%02d:00", minute);
2362 + break;
2363 +
2364 + default:
2365 + BUG("Unhandled schedule_priority");
2366 + }
2367 +
2368 unit = "# This file was created and is maintained by Git.\n"
2369 "# Any edits made in this file might be replaced in the future\n"
2370 "# by a Git command.\n"
@@ -2331,12 +2373,12 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
2373 "Description=Optimize Git repositories data\n"
2374 "\n"
2375 "[Timer]\n"
2334 - "OnCalendar=%i\n"
2376 + "OnCalendar=%s\n"
2377 "Persistent=true\n"
2378 "\n"
2379 "[Install]\n"
2380 "WantedBy=timers.target\n";
2339 - if (fputs(unit, file) == EOF) {
2381 + if (fprintf(file, unit, schedule_pattern) < 0) {
2382 error(_("failed to write to '%s'"), filename);
2383 fclose(file);
2384 goto error;
@@ -2345,9 +2387,36 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
2387 error_errno(_("failed to flush '%s'"), filename);
2388 goto error;
2389 }
2390 +
2391 + res = 0;
2392 +
2393 +error:
2394 + free(schedule_pattern);
2395 + free(local_timer_name);
2396 free(filename);
2397 + return res;
2398 +}
2399
2350 - filename = xdg_config_home_systemd("git-maintenance@.service");
2400 +/*
2401 + * No matter the schedule, we use the same service and can make use of the
2402 + * templating system. When installing git-maintenance@<schedule>.timer,
2403 + * systemd will notice that git-maintenance@.service exists as a template
2404 + * and will use this file and insert the <schedule> into the template at
2405 + * the position of "%i".
2406 + */
2407 +static int systemd_timer_write_service_template(const char *exec_path)
2408 +{
2409 + int res = -1;
2410 + char *filename;
2411 + FILE *file;
2412 + const char *unit;
2413 + char *local_service_name = xstrfmt(SYSTEMD_UNIT_FORMAT, "", "service");
2414 +
2415 + filename = xdg_config_home_systemd(local_service_name);
2416 + if (safe_create_leading_directories(filename)) {
2417 + error(_("failed to create directories for '%s'"), filename);
2418 + goto error;
2419 + }
2420 file = fopen_or_warn(filename, "w");
2421 if (!file)
2422 goto error;
@@ -2380,17 +2449,18 @@ static int systemd_timer_write_unit_templates(const char *exec_path)
2449 error_errno(_("failed to flush '%s'"), filename);
2450 goto error;
2451 }
2383 - free(filename);
2384 - return 0;
2452 +
2453 + res = 0;
2454
2455 error:
2456 + free(local_service_name);
2457 free(filename);
2388 - systemd_timer_delete_unit_templates();
2389 - return -1;
2458 + return res;
2459 }
2460
2461 static int systemd_timer_enable_unit(int enable,
2393 - enum schedule_priority schedule)
2462 + enum schedule_priority schedule,
2463 + int minute)
2464 {
2465 const char *cmd = "systemctl";
2466 struct child_process child = CHILD_PROCESS_INIT;
@@ -2407,12 +2477,14 @@ static int systemd_timer_enable_unit(int enable,
2477 */
2478 if (!enable)
2479 child.no_stderr = 1;
2480 + else if (systemd_timer_write_timer_file(schedule, minute))
2481 + return -1;
2482
2483 get_schedule_cmd(&cmd, NULL);
2484 strvec_split(&child.args, cmd);
2485 strvec_pushl(&child.args, "--user", enable ? "enable" : "disable",
2486 "--now", NULL);
2415 - strvec_pushf(&child.args, "git-maintenance@%s.timer", frequency);
2487 + strvec_pushf(&child.args, SYSTEMD_UNIT_FORMAT, frequency, "timer");
2488
2489 if (start_command(&child))
2490 return error(_("failed to start systemctl"));
@@ -2429,24 +2501,58 @@ static int systemd_timer_enable_unit(int enable,
2501 return 0;
2502 }
2503
2504 +/*
2505 + * A previous version of Git wrote the timer units as template files.
2506 + * Clean these up, if they exist.
2507 + */
2508 +static void systemd_timer_delete_stale_timer_templates(void)
2509 +{
2510 + char *timer_template_name = xstrfmt(SYSTEMD_UNIT_FORMAT, "", "timer");
2511 + char *filename = xdg_config_home_systemd(timer_template_name);
2512 +
2513 + if (unlink(filename) && !is_missing_file_error(errno))
2514 + warning(_("failed to delete '%s'"), filename);
2515 +
2516 + free(filename);
2517 + free(timer_template_name);
2518 +}
2519 +
2520 +static int systemd_timer_delete_unit_files(void)
2521 +{
2522 + systemd_timer_delete_stale_timer_templates();
2523 +
2524 + /* Purposefully not short-circuited to make sure all are called. */
2525 + return systemd_timer_delete_timer_file(SCHEDULE_HOURLY) |
2526 + systemd_timer_delete_timer_file(SCHEDULE_DAILY) |
2527 + systemd_timer_delete_timer_file(SCHEDULE_WEEKLY) |
2528 + systemd_timer_delete_service_template();
2529 +}
2530 +
2531 static int systemd_timer_delete_units(void)
2532 {
2434 - return systemd_timer_enable_unit(0, SCHEDULE_HOURLY) ||
2435 - systemd_timer_enable_unit(0, SCHEDULE_DAILY) ||
2436 - systemd_timer_enable_unit(0, SCHEDULE_WEEKLY) ||
2437 - systemd_timer_delete_unit_templates();
2533 + int minute = get_random_minute();
2534 + /* Purposefully not short-circuited to make sure all are called. */
2535 + return systemd_timer_enable_unit(0, SCHEDULE_HOURLY, minute) |
2536 + systemd_timer_enable_unit(0, SCHEDULE_DAILY, minute) |
2537 + systemd_timer_enable_unit(0, SCHEDULE_WEEKLY, minute) |
2538 + systemd_timer_delete_unit_files();
2539 }
2540
2541 static int systemd_timer_setup_units(void)
2542 {
2543 + int minute = get_random_minute();
2544 const char *exec_path = git_exec_path();
2545
2444 - int ret = systemd_timer_write_unit_templates(exec_path) ||
2445 - systemd_timer_enable_unit(1, SCHEDULE_HOURLY) ||
2446 - systemd_timer_enable_unit(1, SCHEDULE_DAILY) ||
2447 - systemd_timer_enable_unit(1, SCHEDULE_WEEKLY);
2546 + int ret = systemd_timer_write_service_template(exec_path) ||
2547 + systemd_timer_enable_unit(1, SCHEDULE_HOURLY, minute) ||
2548 + systemd_timer_enable_unit(1, SCHEDULE_DAILY, minute) ||
2549 + systemd_timer_enable_unit(1, SCHEDULE_WEEKLY, minute);
2550 +
2551 if (ret)
2552 systemd_timer_delete_units();
2553 + else
2554 + systemd_timer_delete_stale_timer_templates();
2555 +
2556 return ret;
2557 }
2558
t/t7900-maintenance.sh
+13 -2
@@ -744,7 +744,15 @@ test_expect_success 'start and stop Linux/systemd maintenance' '
744 # start registers the repo
745 git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
746
747 - test_systemd_analyze_verify "systemd/user/git-maintenance@.service" &&
747 + for schedule in hourly daily weekly
748 + do
749 + test_path_is_file "systemd/user/git-maintenance@$schedule.timer" || return 1
750 + done &&
751 + test_path_is_file "systemd/user/git-maintenance@.service" &&
752 +
753 + test_systemd_analyze_verify "systemd/user/git-maintenance@hourly.service" &&
754 + test_systemd_analyze_verify "systemd/user/git-maintenance@daily.service" &&
755 + test_systemd_analyze_verify "systemd/user/git-maintenance@weekly.service" &&
756
757 printf -- "--user enable --now git-maintenance@%s.timer\n" hourly daily weekly >expect &&
758 test_cmp expect args &&
@@ -755,7 +763,10 @@ test_expect_success 'start and stop Linux/systemd maintenance' '
763 # stop does not unregister the repo
764 git config --get --global --fixed-value maintenance.repo "$(pwd)" &&
765
758 - test_path_is_missing "systemd/user/git-maintenance@.timer" &&
766 + for schedule in hourly daily weekly
767 + do
768 + test_path_is_missing "systemd/user/git-maintenance@$schedule.timer" || return 1
769 + done &&
770 test_path_is_missing "systemd/user/git-maintenance@.service" &&
771
772 printf -- "--user disable --now git-maintenance@%s.timer\n" hourly daily weekly >expect &&