pid1: use the cache mtime not clock to "mark" load attempts

We really only care if the cache has been reloaded between the time when we
last attempted to load this unit and now. So instead of recording the actual
time we try to load the unit, just store the timestamp of the cache. This has
the advantage that we'll notice if the cache mtime jumps forward or backward.

Also rename fragment_loadtime to fragment_not_found_time. It only gets set when
we failed to load the unit and the old name was suggesting it is always set.

In https://bugzilla.redhat.com/show_bug.cgi?id=1871327
(and most likely https://bugzilla.redhat.com/show_bug.cgi?id=1867930
and most likely https://bugzilla.redhat.com/show_bug.cgi?id=1872068) we try
to load a non-existent unit over and over from transaction_add_job_and_dependencies().
My understanding is that the clock was in the future during inital boot,
so cache_mtime is always in the future (since we don't touch the fs after initial boot),
so no matter how many times we try to load the unit and set
fragment_loadtime / fragment_not_found_time, it is always higher than cache_mtime,
so manager_unit_cache_should_retry_load() always returns true.

(cherry picked from commit c149d2b49128700a2ae361f43b9065b51c174838)

diff --git a/src/core/manager.c b/src/core/manager.c
index 01eb31e1299415b03a57577d4ee288fb401621bb..1a70634cb9cede1d4529284d71d268f28f7924a6 100644 (file)
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1951,7 +1951,7 @@ bool manager_unit_cache_should_retry_load(Unit *u) {
 
         /* The cache has been updated since the last time we tried to load the unit. There might be new
          * fragment paths to read. */
-        if (u->manager->unit_cache_mtime > u->fragment_loadtime)
+        if (u->manager->unit_cache_mtime != u->fragment_not_found_time)
                 return true;
 
         /* The cache needs to be updated because there are modifications on disk. */

diff --git a/src/core/unit.c b/src/core/unit.c
index d5968f36f8b6ff14a805f55a44e30b4e07c37697..ea445e61f302fcb912629c7a6dcadacf68e2be80 100644 (file)
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -1682,10 +1682,10 @@ fail:
                                                      UNIT_ERROR;
         u->load_error = r;
 
-        /* Record the last time we tried to load the unit, so that if the cache gets updated between now
-         * and the next time an attempt is made to load this unit, we know we need to check again. */
+        /* Record the timestamp on the cache, so that if the cache gets updated between now and the next time
+         * an attempt is made to load this unit, we know we need to check again. */
         if (u->load_state == UNIT_NOT_FOUND)
-                u->fragment_loadtime = now(CLOCK_REALTIME);
+                u->fragment_not_found_time = u->manager->unit_cache_mtime;
 
         unit_add_to_dbus_queue(u);
         unit_add_to_gc_queue(u);

diff --git a/src/core/unit.h b/src/core/unit.h
index d5e4c65989e400922724efb333714b91bfa06e67..d348df7d47ae9ff7fb9338045750f814f6b898f3 100644 (file)
--- a/src/core/unit.h
+++ b/src/core/unit.h
@@ -136,7 +136,7 @@ typedef struct Unit {
         char *source_path; /* if converted, the source file */
         char **dropin_paths;
 
-        usec_t fragment_loadtime;
+        usec_t fragment_not_found_time;
         usec_t fragment_mtime;
         usec_t source_mtime;
         usec_t dropin_mtime;