pid1: drop unit caches only based on mtime

v2:
- do not watch mtime of transient and generated dirs

  We'd reload the map after every transient unit we created, which we don't
  need to do, since we create those units ourselves and know their fragment
  path.

diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c
index a28b27a3a00e1eeb38014f27e89ab5e1cc82a40f..4ebce83b2cea889352a2deaee05586e86f01f7a8 100644 (file)
--- a/src/analyze/analyze.c
+++ b/src/analyze/analyze.c
@@ -1568,7 +1568,7 @@ static int do_unit_files(int argc, char *argv[], void *userdata) {
         if (r < 0)
                 return log_error_errno(r, "lookup_paths_init() failed: %m");
 
-        r = unit_file_build_name_map(&lp, &unit_ids, &unit_names, NULL);
+        r = unit_file_build_name_map(&lp, NULL, &unit_ids, &unit_names, NULL);
         if (r < 0)
                 return log_error_errno(r, "unit_file_build_name_map() failed: %m");
 

diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 7521d599e9e604ef32258f5e1e0d006f95f50b1d..f34e424fbcb8ad20d1364e08ba1392a51ba54a3d 100644 (file)
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -4611,6 +4611,15 @@ int unit_load_fragment(Unit *u) {
                 return 0;
         }
 
+        /* Possibly rebuild the fragment map to catch new units */
+        r = unit_file_build_name_map(&u->manager->lookup_paths,
+                                     &u->manager->unit_cache_mtime,
+                                     &u->manager->unit_id_map,
+                                     &u->manager->unit_name_map,
+                                     &u->manager->unit_path_cache);
+        if (r < 0)
+                log_error_errno(r, "Failed to rebuild name map: %m");
+
         r = unit_file_find_fragment(u->manager->unit_id_map,
                                     u->manager->unit_name_map,
                                     u->id,

diff --git a/src/core/manager.c b/src/core/manager.c
index f8a7e7d64e43a55dac5dbd7e4433d60aa36cfe38..4a503a944ec11e38588761d1be9e0119e9e1c736 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -678,6 +678,7 @@ static void manager_free_unit_name_maps(Manager *m) {
         m->unit_id_map = hashmap_free(m->unit_id_map);
         m->unit_name_map = hashmap_free(m->unit_name_map);
         m->unit_path_cache = set_free_free(m->unit_path_cache);
+        m->unit_cache_mtime =  0;
 }
 
 static int manager_setup_run_queue(Manager *m) {
@@ -1626,11 +1627,6 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
         if (r < 0)
                 log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m");
 
-        manager_free_unit_name_maps(m);
-        r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache);
-        if (r < 0)
-                return log_error_errno(r, "Failed to build name map: %m");
-
         {
                 /* This block is (optionally) done with the reloading counter bumped */
                 _cleanup_(manager_reloading_stopp) Manager *reloading = NULL;
@@ -2847,10 +2843,6 @@ int manager_loop(Manager *m) {
         assert(m);
         assert(m->objective == MANAGER_OK); /* Ensure manager_startup() has been called */
 
-        /* Release the path and unit name caches */
-        manager_free_unit_name_maps(m);
-        // FIXME: once this happens, we cannot load any more units
-
         manager_check_finished(m);
 
         /* There might still be some zombies hanging around from before we were exec()'ed. Let's reap them. */
@@ -3526,10 +3518,8 @@ int manager_reload(Manager *m) {
         if (r < 0)
                 log_warning_errno(r, "Failed to reduce unit file paths, ignoring: %m");
 
+        /* We flushed out generated files, for which we don't watch mtime, so we should flush the old map. */
         manager_free_unit_name_maps(m);
-        r = unit_file_build_name_map(&m->lookup_paths, &m->unit_id_map, &m->unit_name_map, &m->unit_path_cache);
-        if (r < 0)
-                log_warning_errno(r, "Failed to build name map: %m");
 
         /* First, enumerate what we can from kernel and suchlike */
         manager_enumerate_perpetual(m);

diff --git a/src/core/manager.h b/src/core/manager.h
index 3cb37b3bf49f21e8161124febdfca7edf699e827..596220848f17fc738678cc5de4af02e7cb7daa2a 100644 (file)
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -224,6 +224,7 @@ struct Manager {
         Hashmap *unit_id_map;
         Hashmap *unit_name_map;
         Set *unit_path_cache;
+        usec_t unit_cache_mtime;
 
         char **transient_environment;  /* The environment, as determined from config files, kernel cmdline and environment generators */
         char **client_environment;     /* Environment variables created by clients through the bus API */

diff --git a/src/shared/unit-file.c b/src/shared/unit-file.c
index 90fa949d247e20abb546d24eee7c6d2239af2d03..8a09f3827f7948a0fc3f7864998b7228345ae030 100644 (file)
--- a/src/shared/unit-file.c
+++ b/src/shared/unit-file.c
@@ -152,8 +152,47 @@ static int unit_ids_map_get(
         return -ELOOP;
 }
 
+static bool lookup_paths_mtime_exclude(const LookupPaths *lp, const char *path) {
+        /* Paths that are under our exclusive control. Users shall not alter those directly. */
+
+        return streq_ptr(path, lp->generator) ||
+               streq_ptr(path, lp->generator_early) ||
+               streq_ptr(path, lp->generator_late) ||
+               streq_ptr(path, lp->transient) ||
+               streq_ptr(path, lp->persistent_control) ||
+               streq_ptr(path, lp->runtime_control);
+}
+
+static bool lookup_paths_mtime_good(const LookupPaths *lp, usec_t mtime) {
+        char **dir;
+
+        STRV_FOREACH(dir, (char**) lp->search_path) {
+                struct stat st;
+
+                if (lookup_paths_mtime_exclude(lp, *dir))
+                        continue;
+
+                /* Determine the latest lookup path modification time */
+                if (stat(*dir, &st) < 0) {
+                        if (errno == ENOENT)
+                                continue;
+
+                        log_debug_errno(errno, "Failed to stat %s, ignoring: %m", *dir);
+                        continue;
+                }
+
+                if (timespec_load(&st.st_mtim) > mtime) {
+                        log_debug_errno(errno, "Unit dir %s has changed, need to update cache.", *dir);
+                        return false;
+                }
+        }
+
+        return true;
+}
+
 int unit_file_build_name_map(
                 const LookupPaths *lp,
+                usec_t *cache_mtime,
                 Hashmap **ret_unit_ids_map,
                 Hashmap **ret_unit_names_map,
                 Set **ret_path_cache) {
@@ -171,6 +210,12 @@ int unit_file_build_name_map(
         _cleanup_set_free_free_ Set *paths = NULL;
         char **dir;
         int r;
+        usec_t mtime = 0;
+
+        /* Before doing anything, check if the mtime that was passed is still valid. If
+         * yes, do nothing. If *cache_time == 0, always build the cache. */
+        if (cache_mtime && *cache_mtime > 0 && lookup_paths_mtime_good(lp, *cache_mtime))
+                return 0;
 
         if (ret_path_cache) {
                 paths = set_new(&path_hash_ops);
@@ -181,6 +226,7 @@ int unit_file_build_name_map(
         STRV_FOREACH(dir, (char**) lp->search_path) {
                 struct dirent *de;
                 _cleanup_closedir_ DIR *d = NULL;
+                struct stat st;
 
                 d = opendir(*dir);
                 if (!d) {
@@ -189,6 +235,13 @@ int unit_file_build_name_map(
                         continue;
                 }
 
+                /* Determine the latest lookup path modification time */
+                if (fstat(dirfd(d), &st) < 0)
+                        return log_error_errno(errno, "Failed to fstat %s: %m", *dir);
+
+                if (!lookup_paths_mtime_exclude(lp, *dir))
+                        mtime = MAX(mtime, timespec_load(&st.st_mtim));
+
                 FOREACH_DIRENT(de, d, log_warning_errno(errno, "Failed to read \"%s\", ignoring: %m", *dir)) {
                         char *filename;
                         _cleanup_free_ char *_filename_free = NULL, *simplified = NULL;
@@ -316,12 +369,14 @@ int unit_file_build_name_map(
                                                  basename(dst), src);
         }
 
+        if (cache_mtime)
+                *cache_mtime = mtime;
         *ret_unit_ids_map = TAKE_PTR(ids);
         *ret_unit_names_map = TAKE_PTR(names);
         if (ret_path_cache)
                 *ret_path_cache = TAKE_PTR(paths);
 
-        return 0;
+        return 1;
 }
 
 int unit_file_find_fragment(

diff --git a/src/shared/unit-file.h b/src/shared/unit-file.h
index 52e17f7cfb12bef55a098ed2e8cd0d9e79e0fea7..54cc7876fef15b807d5194d1c295ea836ced9a0f 100644 (file)
--- a/src/shared/unit-file.h
+++ b/src/shared/unit-file.h
@@ -4,6 +4,7 @@
 #include <stdbool.h>
 
 #include "hashmap.h"
+#include "time-util.h"
 #include "unit-name.h"
 
 typedef enum UnitFileState UnitFileState;
@@ -42,6 +43,7 @@ int unit_validate_alias_symlink_and_warn(const char *filename, const char *targe
 
 int unit_file_build_name_map(
                 const LookupPaths *lp,
+                usec_t *ret_time,
                 Hashmap **ret_unit_ids_map,
                 Hashmap **ret_unit_names_map,
                 Set **ret_path_cache);

diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 31d364cefed787b58f09022fc255ca3120d9b1c4..1140deb0d102bf994f7fa9174e2544169c33f7e0 100644 (file)
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -2593,7 +2593,7 @@ static int unit_find_paths(
                 _cleanup_set_free_free_ Set *names = NULL;
 
                 if (!cached_name_map) {
-                        r = unit_file_build_name_map(lp, &cached_id_map, &cached_name_map, NULL);
+                        r = unit_file_build_name_map(lp, NULL, &cached_id_map, &cached_name_map, NULL);
                         if (r < 0)
                                 return r;
                 }

diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index c5144a1b7eaf00de2c389255b60b4eb9ca7921c8..0c0371375ad4b9ab8573521cdb538ddbb9ab1964 100644 (file)
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -31,10 +31,12 @@ static void test_unit_file_build_name_map(void) {
         Iterator i;
         const char *k, *dst;
         char **v;
+        usec_t mtime = 0;
+        int r;
 
         assert_se(lookup_paths_init(&lp, UNIT_FILE_SYSTEM, 0, NULL) >= 0);
 
-        assert_se(unit_file_build_name_map(&lp, &unit_ids, &unit_names, NULL) == 0);
+        assert_se(unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL) == 1);
 
         HASHMAP_FOREACH_KEY(dst, k, unit_ids, i)
                 log_info("ids: %s → %s", k, dst);
@@ -43,6 +45,14 @@ static void test_unit_file_build_name_map(void) {
                 _cleanup_free_ char *j = strv_join(v, ", ");
                 log_info("aliases: %s ← %s", k, j);
         }
+
+        char buf[FORMAT_TIMESTAMP_MAX];
+        log_debug("Last modification time: %s", format_timestamp(buf, sizeof buf, mtime));
+
+        r = unit_file_build_name_map(&lp, &mtime, &unit_ids, &unit_names, NULL);
+        assert_se(IN_SET(r, 0, 1));
+        if (r == 0)
+                log_debug("Cache rebuild skipped based on mtime.");
 }
 
 int main(int argc, char **argv) {