gc: use *.snap st_mtime to schedule after restart

Introduce the `timestamp` field in `gc_checkpoint` so now `gc.{c,h}` are
aware of actual times of checkpoints, which is important since this
subsystem is responsible for scheduling. Now, one can track the unix time
of a new checkpoint with new `timestamp` argument of `gc_add_checkpoint`.
This change allows us to track previous checkpoints made before the
server restart and even `checkpoint_interval` value reconfiguring.

This approach was chosen instead of just scanning the `snap_dir` in
`gc.c` because it was engine-independent. One may also notice that even
if the actual time after the last snapshot before the restart is greater
than `2 * checkpoint_interval` we won't start checkpointing immediately
because that may cause high disk load in case of multiple instances. So
in this case we just schedule a checkpoint at a random moment in the
first `checkpoint_interval` seconds after the restart. It seems like
even with this scheduling strategy a snapshot will be eventually created
even during constant restarting.

Fixes tarantool#9820
NO_DOC=bugfix

Author: LevKats

changelogs/unreleased/gh-9820-checkpoint-interval-after-restart.md

@@ -0,0 +1,4 @@
+## bugfix/box
+
+* Fixed a bug when the timestamps of snapshots created before the server restart
+  were not taken into account with `checkpoint_interval` enabled (gh-9820).

src/box/checkpoint_schedule.c

@@ -29,17 +29,26 @@
  * SUCH DAMAGE.
  */
 #include "checkpoint_schedule.h"
+#include "trivia/util.h"
 
 #include <assert.h>
 #include <math.h>
 #include <stdlib.h>
 
 void
 checkpoint_schedule_cfg(struct checkpoint_schedule *sched,
-			double now, double interval)
+			double now, double interval,
+			double time_since_checkpoint)
 {
 	sched->interval = interval;
-	sched->start_time = now + interval;
+	/*
+	 * If the last checkpoint was long enough ago, we should not allow
+	 * the start_time to be before now in any case so possible parallel
+	 * snapshots on different instances will not result in too many
+	 * writes at once.
+	 */
+	sched->start_time = now + interval -
+			    MIN(time_since_checkpoint, interval);
 
 	/*
 	 * Add a random offset to the start time so as to avoid

src/box/checkpoint_schedule.h

@@ -53,10 +53,12 @@ struct checkpoint_schedule {
  *
  * @now is the current time.
  * @interval is the configured interval between checkpoints.
+ * @time_since_checkpoint is the time since the last checkpoint
  */
 void
 checkpoint_schedule_cfg(struct checkpoint_schedule *sched,
-			double now, double interval);
+			double now, double interval,
+			double time_since_checkpoint);
 
 /**
  * Reset a checkpoint schedule.

src/box/gc.c

@@ -135,7 +135,7 @@ gc_init(on_garbage_collection_f on_garbage_collection)
 	rlist_create(&gc.consumers);
 	gc_tree_new(&gc.active_consumers);
 	fiber_cond_create(&gc.cleanup_cond);
-	checkpoint_schedule_cfg(&gc.checkpoint_schedule, 0, 0);
+	checkpoint_schedule_cfg(&gc.checkpoint_schedule, 0, 0, 0);
 
 	gc.cleanup_fiber = fiber_new_system("gc", gc_cleanup_fiber_f);
 	if (gc.cleanup_fiber == NULL)
@@ -477,14 +477,19 @@ gc_set_checkpoint_interval(double interval)
 	 * if it's waiting for checkpointing to complete, because
 	 * checkpointing code doesn't tolerate spurious wakeups.
 	 */
+	struct gc_checkpoint *last = gc_last_checkpoint();
+	double time_since_checkpoint = 0;
+	if (last != NULL && !gc.checkpoint_is_in_progress)
+		time_since_checkpoint = ev_time() - last->timestamp;
 	checkpoint_schedule_cfg(&gc.checkpoint_schedule,
-				ev_monotonic_now(loop()), interval);
+				ev_monotonic_now(loop()), interval,
+				time_since_checkpoint);
 	if (!gc.checkpoint_is_in_progress)
 		fiber_wakeup(gc.checkpoint_fiber);
 }
 
 void
-gc_add_checkpoint(const struct vclock *vclock)
+gc_add_checkpoint(const struct vclock *vclock, double timestamp)
 {
 	struct gc_checkpoint *last_checkpoint = gc_last_checkpoint();
 	if (last_checkpoint != NULL &&
@@ -511,6 +516,7 @@ gc_add_checkpoint(const struct vclock *vclock)
 
 	rlist_create(&checkpoint->refs);
 	vclock_copy(&checkpoint->vclock, vclock);
+	checkpoint->timestamp = timestamp;
 	rlist_add_tail_entry(&gc.checkpoints, checkpoint, in_checkpoints);
 	gc.checkpoint_count++;
 
@@ -564,7 +570,7 @@ gc_do_checkpoint(bool is_scheduled)
 	 * Finally, track the newly created checkpoint in the garbage
 	 * collector state.
 	 */
-	gc_add_checkpoint(&checkpoint.vclock);
+	gc_add_checkpoint(&checkpoint.vclock, ev_time());
 out:
 	if (rc != 0)
 		engine_abort_checkpoint();
@@ -593,7 +599,8 @@ gc_checkpoint(void)
 	 */
 	checkpoint_schedule_cfg(&gc.checkpoint_schedule,
 				ev_monotonic_now(loop()),
-				gc.checkpoint_schedule.interval);
+				gc.checkpoint_schedule.interval,
+				0);
 	fiber_wakeup(gc.checkpoint_fiber);
 
 	if (gc_do_checkpoint(false) != 0)

src/box/gc.h

@@ -61,6 +61,8 @@ struct gc_checkpoint {
 	struct rlist in_checkpoints;
 	/** VClock of the checkpoint. */
 	struct vclock vclock;
+	/** Timestamp of the checkpoint. */
+	double timestamp;
 	/**
 	 * List of checkpoint references, linked by
 	 * gc_checkpoint_ref::in_refs.
@@ -329,7 +331,7 @@ gc_set_checkpoint_interval(double interval);
  * old checkpoints.
  */
 void
-gc_add_checkpoint(const struct vclock *vclock);
+gc_add_checkpoint(const struct vclock *vclock, double timestamp);
 
 /**
  * Make a *manual* checkpoint.

src/box/memtx_engine.cc

@@ -1703,7 +1703,18 @@ memtx_engine_new(const char *snap_dirname, bool force_recovery,
 	for (struct vclock *vclock = vclockset_first(&memtx->snap_dir.index);
 	     vclock != NULL;
 	     vclock = vclockset_next(&memtx->snap_dir.index, vclock)) {
-		gc_add_checkpoint(vclock);
+		const char *name = xdir_format_filename(&memtx->snap_dir,
+							vclock_sum(vclock),
+							NONE);
+		struct stat attr;
+		double timestamp = ev_time();
+		if (stat(name, &attr) != 0)
+			say_warn("failed to get modification time of %s, "
+				 "set it to the current time",
+				 name);
+		else
+			timestamp = (double)attr.st_mtime;
+		gc_add_checkpoint(vclock, timestamp);
 	}
 
 	stailq_create(&memtx->gc_queue);

test/box-luatest/gh_9820_checkpoint_interval_after_restart_test.lua

@@ -0,0 +1,68 @@
+local t = require('luatest')
+local server = require('luatest.server')
+
+local g = t.group()
+
+g.before_all(function(g)
+    g.server = server:new({box_cfg = {
+        checkpoint_interval = 5,
+        checkpoint_count = 1e9,
+    }})
+    g.server:start()
+    g.server:exec(function()
+        local s = box.schema.space.create('test')
+        s:create_index('pk', {sequence = true})
+    end)
+end)
+
+g.before_each(function(g)
+    g.server:exec(function()
+        box.space.test:truncate()
+        box.cfg{checkpoint_interval=5}
+        t.helpers.retrying({}, function()
+            t.assert_not(box.info.gc().checkpoint_is_in_progress)
+        end)
+    end)
+end)
+
+g.test_box_checkpoint_after_restart = function(g)
+    local fiber = require('fiber')
+    local checkpoint_count = g.server:exec(function()
+        local s = box.space.test
+        -- Insert something to make sure that box.snapshot() will do something.
+        s:insert({box.NULL})
+        -- Reschedule future snapshots.
+        box.snapshot()
+        local checkpoint_count = #box.info.gc().checkpoints
+        s:insert({box.NULL})
+        return checkpoint_count
+    end)
+    g.server:stop()
+    fiber.sleep(5)
+    g.server:start()
+    g.server:exec(function(checkpoint_count)
+        local fiber = require('fiber')
+        fiber.sleep(7)
+        t.assert_gt(#box.info.gc().checkpoints, checkpoint_count)
+    end, {checkpoint_count})
+end
+
+g.test_box_checkpoint_after_reconfigure = function(g)
+    g.server:exec(function()
+        box.cfg{checkpoint_interval = 25}
+        local fiber = require('fiber')
+        local s = box.space.test
+        s:insert({box.NULL})
+        box.snapshot()
+        local checkpoint_count = #box.info.gc().checkpoints
+        s:insert({box.NULL})
+        fiber.sleep(5)
+        box.cfg{checkpoint_interval = 5}
+        fiber.sleep(7)
+        t.assert_gt(#box.info.gc().checkpoints, checkpoint_count)
+    end)
+end
+
+g.after_all(function(g)
+    g.server:drop()
+end)

test/unit/checkpoint_schedule.c

@@ -25,7 +25,7 @@ main()
 	double now = rand();
 
 	struct checkpoint_schedule sched;
-	checkpoint_schedule_cfg(&sched, now, 0);
+	checkpoint_schedule_cfg(&sched, now, 0, 0);
 
 	is(checkpoint_schedule_timeout(&sched, now), 0,
 	   "checkpointing disabled - timeout after configuration");
@@ -43,15 +43,15 @@ main()
 	for (int i = 0; i < intervals_len; i++) {
 		double interval = intervals[i];
 
-		checkpoint_schedule_cfg(&sched, now, interval);
+		checkpoint_schedule_cfg(&sched, now, interval, 0);
 		double t = checkpoint_schedule_timeout(&sched, now);
 		ok(t >= interval && t <= interval * 2,
 		   "checkpoint interval %.0lf - timeout after configuration",
 		   interval);
 
 		double t0;
 		for (int j = 0; j < 100; j++) {
-			checkpoint_schedule_cfg(&sched, now, interval);
+			checkpoint_schedule_cfg(&sched, now, interval, 0);
 			t0 = checkpoint_schedule_timeout(&sched, now);
 			if (fabs(t - t0) > interval / 4)
 				break;