Merge !481: detect_time_jump: keep cache on suspend-resume

Closes #284

NEWS

+
+- detect_time_jump module: don't clear cache on suspend-resume (#284)
+
+
 Knot Resolver 2.0.0 (2018-01-31)
 ================================
 

daemon/bindings.c

@@ -731,20 +731,30 @@ static int cache_count(lua_State *L)
 	return 0;
 }
 
-/** Return time of last cache clear */
-static int cache_last_clear(lua_State *L)
+/** Return time of last checkpoint, or re-set it if passed `true`. */
+static int cache_checkpoint(lua_State *L)
 {
 	struct engine *engine = engine_luaget(L);
 	struct kr_cache *cache = &engine->resolver.cache;
-	lua_newtable(L);
-	lua_pushnumber(L, cache->last_clear_monotime);
-	lua_setfield(L, -2, "monotime");
-	lua_newtable(L);
-	lua_pushnumber(L, cache->last_clear_walltime.tv_sec);
-	lua_setfield(L, -2, "sec");
-	lua_pushnumber(L, cache->last_clear_walltime.tv_usec);
-	lua_setfield(L, -2, "usec");
-	lua_setfield(L, -2, "walltime");
+
+	if (lua_gettop(L) == 0) { /* Return the current value. */
+		lua_newtable(L);
+		lua_pushnumber(L, cache->checkpoint_monotime);
+		lua_setfield(L, -2, "monotime");
+		lua_newtable(L);
+		lua_pushnumber(L, cache->checkpoint_walltime.tv_sec);
+		lua_setfield(L, -2, "sec");
+		lua_pushnumber(L, cache->checkpoint_walltime.tv_usec);
+		lua_setfield(L, -2, "usec");
+		lua_setfield(L, -2, "walltime");
+		return 1;
+	}
+
+	if (lua_gettop(L) != 1 || !lua_isboolean(L, 1) || !lua_toboolean(L, 1)) {
+		format_error(L, "cache.checkpoint() takes no parameters or a true value");
+		lua_error(L);
+	}
+	kr_cache_make_checkpoint(cache);
 	return 1;
 }
 
@@ -1113,7 +1123,7 @@ int lib_cache(lua_State *L)
 		{ "backends", cache_backends },
 		{ "count",  cache_count },
 		{ "stats",  cache_stats },
-		{ "last_clear",  cache_last_clear },
+		{ "checkpoint", cache_checkpoint },
 		{ "open",   cache_open },
 		{ "close",  cache_close },
 		{ "prune",  cache_prune },

lib/cache/api.c

@@ -64,13 +64,6 @@ static inline int cache_clear(struct kr_cache *cache)
 	return cache_op(cache, clear);
 }
 
-/** @internal Set time when clearing cache. */
-static void reset_timestamps(struct kr_cache *cache)
-{
-	cache->last_clear_monotime = kr_now();
-	gettimeofday(&cache->last_clear_walltime, NULL);
-}
-
 /** @internal Open cache db transaction and check internal data version. */
 static int assert_right_version(struct kr_cache *cache)
 {
@@ -129,7 +122,7 @@ int kr_cache_open(struct kr_cache *cache, const struct kr_cdb_api *api, struct k
 	cache->ttl_min = KR_CACHE_DEFAULT_TTL_MIN;
 	cache->ttl_max = KR_CACHE_DEFAULT_TTL_MAX;
 	/* Check cache ABI version */
-	reset_timestamps(cache);
+	kr_cache_make_checkpoint(cache);
 	(void) assert_right_version(cache);
 	return 0;
 }
@@ -163,7 +156,7 @@ int kr_cache_clear(struct kr_cache *cache)
 	}
 	int ret = cache_clear(cache);
 	if (ret == 0) {
-		reset_timestamps(cache);
+		kr_cache_make_checkpoint(cache);
 		ret = assert_right_version(cache);
 	}
 	return ret;

lib/cache/api.h

@@ -48,8 +48,10 @@ struct kr_cache
 	} stats;
 
 	uint32_t ttl_min, ttl_max; /**< TTL limits */
-	struct timeval last_clear_walltime; /**< Time of last cache clear */
-	uint64_t last_clear_monotime;  /**< Last cache clear in monotonic milliseconds */
+
+	/* A pair of stamps for detection of real-time shifts during runtime. */
+	struct timeval checkpoint_walltime; /**< Wall time on the last check-point. */
+	uint64_t checkpoint_monotime; /**< Monotonic milliseconds on the last check-point. */
 };
 
 /**
@@ -83,6 +85,14 @@ static inline bool kr_cache_is_open(struct kr_cache *cache)
 	return cache->db != NULL;
 }
 
+/** (Re)set the time pair to the current values. */
+static inline void kr_cache_make_checkpoint(struct kr_cache *cache)
+{
+	cache->checkpoint_monotime = kr_now();
+	gettimeofday(&cache->checkpoint_walltime, NULL);
+}
+
+
 /**
  * Clear all items from the cache.
  * @param cache cache structure

modules/detect_time_jump/README.rst

@@ -4,11 +4,17 @@ Detect discontinuous jumps in the system time
 ---------------------------------------------
 
 This module detect discontinuous jumps in the system time when resolver
-is running. It clears cache when some time jumps occurs. 
+is running.  It clears cache when a significant backward time jumps occurs.
 
-Time jumps is ussualy created by NTP time change or by admin intervention.
-These change can affect cache records as they store timestamp and TTL in real 
+Time jumps are usualy created by NTP time change or by admin intervention.
+These change can affect cache records as they store timestamp and TTL in real
 time.
 
 If you want to preserve cache during time travel you should disable
 this module by ``modules.unload('detect_time_jump')``.
+
+Due to the way monotonic system time works on typical systems,
+suspend-resume cycles will be perceived as a foward time jumps,
+but this direction of shift does not have the risk of using records
+beyond their intended TTL, so forward jumps do not cause erasing the cache.
+

modules/detect_time_jump/detect_time_jump.lua

@@ -10,15 +10,18 @@ local event_id = nil
 -- time. In ideal case these differences should be almost same.
 -- If they differ more than mod.threshold value then clear cache.
 local function check_time()
-	local clear_time = cache.last_clear()
-	local cache_timeshift = clear_time.walltime.sec * 1000 - clear_time.monotime
+	local checkpoint = cache.checkpoint()
+	local cache_timeshift = checkpoint.walltime.sec * 1000 - checkpoint.monotime
 	local actual_timeshift = os.time() * 1000 - tonumber(ffi.C.kr_now())
-	local time_diff = math.abs(cache_timeshift - actual_timeshift)
-	if time_diff > mod.threshold then
-		log("Detected time change, clearing cache\n" ..
+	local jump_backward = cache_timeshift - actual_timeshift
+	if jump_backward > mod.threshold then
+		log("Detected backwards time jump, clearing cache.\n" ..
 		"But what does that mean? It means your future hasn't been written yet."
 		)
 		cache.clear()
+	elseif -jump_backward > mod.threshold then
+		log("Detected forward time jump.  (Suspend-resume, possibly.)")
+		cache.checkpoint(true)
 	end
 end