From 7714346663657716c61a34c6733d83728f147ba7 Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Tue, 13 Feb 2024 10:01:02 -0500
Subject: [PATCH] Rename ra_log snapshot_interval parameter to
 min_snapshot_interval

The snapshot interval is used as a minimum number of commands handled
before taking a snapshot rather than a regular fixed interval. We keep
the old `snapshot_interval` parameter around as a fallback so this
change isn't breaking.
---
 src/ra_log.erl              | 16 +++++++++++-----
 test/coordination_SUITE.erl |  2 +-
 test/ra_log_2_SUITE.erl     | 10 +++++-----
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/ra_log.erl b/src/ra_log.erl
index f7a85e41..8f559678 100644
--- a/src/ra_log.erl
+++ b/src/ra_log.erl
@@ -53,7 +53,7 @@
 -include("ra.hrl").
 
 -define(DEFAULT_RESEND_WINDOW_SEC, 20).
--define(SNAPSHOT_INTERVAL, 4096).
+-define(MIN_SNAPSHOT_INTERVAL, 4096).
 -define(MIN_CHECKPOINT_INTERVAL, 16384).
 -define(LOG_APPEND_TIMEOUT, 5000).
 
@@ -84,7 +84,7 @@
 -record(cfg, {uid :: ra_uid(),
               log_id :: unicode:chardata(),
               directory :: file:filename(),
-              snapshot_interval = ?SNAPSHOT_INTERVAL :: non_neg_integer(),
+              min_snapshot_interval = ?MIN_SNAPSHOT_INTERVAL :: non_neg_integer(),
               min_checkpoint_interval = ?MIN_CHECKPOINT_INTERVAL :: non_neg_integer(),
               snapshot_module :: module(),
               resend_window_seconds = ?DEFAULT_RESEND_WINDOW_SEC :: integer(),
@@ -115,7 +115,11 @@
 -type ra_log_init_args() :: #{uid := ra_uid(),
                               system_config => ra_system:config(),
                               log_id => unicode:chardata(),
+                              %% Deprecated in favor of `min_snapshot_interval'
+                              %% but this value is used as a fallback if
+                              %% `min_snapshot_interval' is not provided.
                               snapshot_interval => non_neg_integer(),
+                              min_snapshot_interval => non_neg_integer(),
                               min_checkpoint_interval => non_neg_integer(),
                               resend_window => integer(),
                               max_open_segments => non_neg_integer(),
@@ -155,7 +159,9 @@ init(#{uid := UId,
     %% this has to be patched by ra_server
     LogId = maps:get(log_id, Conf, UId),
     ResendWindow = maps:get(resend_window, Conf, ?DEFAULT_RESEND_WINDOW_SEC),
-    SnapInterval = maps:get(snapshot_interval, Conf, ?SNAPSHOT_INTERVAL),
+    SnapInterval = maps:get(min_snapshot_interval, Conf,
+                            maps:get(snapshot_interval, Conf,
+                                     ?MIN_SNAPSHOT_INTERVAL)),
     CPInterval = maps:get(min_checkpoint_interval, Conf,
                           ?MIN_CHECKPOINT_INTERVAL),
     MaxCheckpoints = maps:get(max_checkpoints, Conf, ?DEFAULT_MAX_CHECKPOINTS),
@@ -201,7 +207,7 @@ init(#{uid := UId,
     Cfg = #cfg{directory = Dir,
                uid = UId,
                log_id = LogId,
-               snapshot_interval = SnapInterval,
+               min_snapshot_interval = SnapInterval,
                min_checkpoint_interval = CPInterval,
                wal = Wal,
                segment_writer = SegWriter,
@@ -723,7 +729,7 @@ suggest_snapshot0(SnapKind, Idx, Cluster, MacVersion, MacState, State0) ->
     end.
 
 should_snapshot(snapshot, Idx,
-                #?MODULE{cfg = #cfg{snapshot_interval = SnapInter},
+                #?MODULE{cfg = #cfg{min_snapshot_interval = SnapInter},
                          reader = Reader,
                          snapshot_state = SnapState}) ->
     SnapLimit = case ra_snapshot:current(SnapState) of
diff --git a/test/coordination_SUITE.erl b/test/coordination_SUITE.erl
index 4a98a6ac..557233ec 100644
--- a/test/coordination_SUITE.erl
+++ b/test/coordination_SUITE.erl
@@ -723,7 +723,7 @@ recover_from_checkpoint(Config) ->
                      machine => {module, ?MODULE, #{}},
                      log_init_args => #{uid => UId,
                                         min_checkpoint_interval => 3,
-                                        snapshot_interval => 5}}
+                                        min_snapshot_interval => 5}}
                end || {Name, _Node} = NodeId <- ServerIds],
     {ok, Started, []} = ra:start_cluster(?SYS, Configs),
     {ok, _, Leader} = ra:members(hd(Started)),
diff --git a/test/ra_log_2_SUITE.erl b/test/ra_log_2_SUITE.erl
index 9a8cdd89..0369b11a 100644
--- a/test/ra_log_2_SUITE.erl
+++ b/test/ra_log_2_SUITE.erl
@@ -392,7 +392,7 @@ sparse_read(Config) ->
     ok.
 
 written_event_after_snapshot(Config) ->
-    Log0 = ra_log_init(Config, #{snapshot_interval => 1}),
+    Log0 = ra_log_init(Config, #{min_snapshot_interval => 1}),
     Log1 = ra_log:append({1, 1, <<"one">>}, Log0),
     Log1b = ra_log:append({2, 1, <<"two">>}, Log1),
     {Log2, _} = ra_log:update_release_cursor(2, #{}, 1,
@@ -429,7 +429,7 @@ updated_segment_can_be_read(Config) ->
     ra_counters:new(?FUNCTION_NAME, ?RA_COUNTER_FIELDS),
     Log0 = ra_log_init(Config,
                        #{counter => ra_counters:fetch(?FUNCTION_NAME),
-                         snapshot_interval => 1}),
+                         min_snapshot_interval => 1}),
     %% append a few entries
     Log2 = append_and_roll(1, 5, 1, Log0),
     % Log2 = deliver_all_log_events(Log1, 200),
@@ -695,7 +695,7 @@ detect_lost_written_range(Config) ->
 
 
 snapshot_written_after_installation(Config) ->
-    Log0 = ra_log_init(Config, #{snapshot_interval => 2}),
+    Log0 = ra_log_init(Config, #{min_snapshot_interval => 2}),
     %% log 1 .. 9, should create a single segment
     Log1 = write_and_roll(1, 10, 1, Log0),
     {Log2, _} = ra_log:update_release_cursor(5, #{}, 1,
@@ -734,7 +734,7 @@ snapshot_written_after_installation(Config) ->
     %% assert there is no pending snapshot
     ?assertEqual(undefined, ra_snapshot:pending(ra_log:snapshot_state(Log5))),
 
-    _ = ra_log:close(ra_log_init(Config, #{snapshot_interval => 2})),
+    _ = ra_log:close(ra_log_init(Config, #{min_snapshot_interval => 2})),
 
     ok.
 
@@ -931,7 +931,7 @@ update_release_cursor(Config) ->
 
 update_release_cursor_with_machine_version(Config) ->
     % ra_log should initiate shapshot if segments can be released
-    Log0 = ra_log_init(Config, #{snapshot_interval => 64}),
+    Log0 = ra_log_init(Config, #{min_snapshot_interval => 64}),
     % beyond 128 limit - should create two segments
     Log1 = append_and_roll(1, 150, 2, Log0),
     timer:sleep(300),