Default to a safe last_gc height (#4134)

aeternity · May 15, 2023 · a2d501e · a2d501e
1 parent b56c072
commit a2d501e
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 5 deletions.
diff --git a/apps/aecore/src/aec_db.erl b/apps/aecore/src/aec_db.erl
@@ -84,6 +84,7 @@
         , secondary_state_tab/1
         , write_last_gc_switch/1
         , read_last_gc_switch/0
+        , read_last_gc_switch/1
         , write_last_gc_scan/1
         , read_last_gc_scan/0 ]).
 
@@ -956,9 +957,12 @@ write_last_gc_switch(Height) ->
     ?t(write(#aec_chain_state{key = last_gc_switch, value = Height})).
 
 read_last_gc_switch() ->
+    read_last_gc_switch(0).
+
+read_last_gc_switch(Default) ->
     R = ?t(case read(aec_chain_state, last_gc_switch) of
                [] ->
-                   0;
+                   Default;
                [#aec_chain_state{value = Height}] ->
                    Height
            end),
@@ -976,7 +980,7 @@ read_last_gc_scan() ->
                [#aec_chain_state{value = Height}] ->
                    Height
            end),
-    lager:debug("<-- last GC Height: ~p", [R]),
+    lager:debug("<-- last height of complete GC scan: ~p", [R]),
     R.
 
 -spec make_primary_state_tab(tree_name(), table_name()) -> ok.

diff --git a/apps/aecore/src/aec_db_gc.erl b/apps/aecore/src/aec_db_gc.erl
@@ -180,7 +180,7 @@ init(#{ <<"enabled">>     := Enabled
                 aec_db:ensure_activity(
                   async_dirty,
                   fun() ->
-                          {aec_db:read_last_gc_switch(),
+                          {last_gc_switch(),
                            aec_db:read_last_gc_scan()}
                   end);
             false ->
@@ -201,6 +201,17 @@ init(#{ <<"enabled">>     := Enabled
                synced       = false},
     {ok, Data}.
 
+last_gc_switch() ->
+    case aec_db:read_last_gc_switch(undefined) of
+        undefined ->
+            case aec_chain:top_height() of
+                undefined -> 0;
+                TopHeight -> TopHeight
+            end;
+        Height ->
+            Height
+    end.
+
 maybe_restart_scanners(LastSwitch, LastScan, Trees) ->
     if LastSwitch > 0, LastScan < LastSwitch ->
             [start_scanner(T, LastSwitch) || T <- Trees];

diff --git a/apps/aecore/test/aec_db_gc_SUITE.erl b/apps/aecore/test/aec_db_gc_SUITE.erl
@@ -8,7 +8,8 @@
 
 %% test case exports
 -export([main_test/1,
-         calls_test/1]).
+         calls_test/1,
+         last_switch_test/1 ]).
 
 -include_lib("common_test/include/ct.hrl").
 
@@ -28,7 +29,8 @@ groups() ->
      {two_nodes, [sequence],
       [main_test]},
      {one_node, [sequence],
-      [calls_test]}].
+      [ calls_test
+      , last_switch_test ]}].
 
 suite() ->
     [].
@@ -361,6 +363,23 @@ calls_test(_Config) ->
     aecore_suite_utils:unsubscribe(N1, gc),
     ok.
 
+%% Simulate the case where we've been GC:ing with an older version, where the height of
+%% the last switch wasn't recorded persistently. The important thing is that it doesn't
+%% default to a lower height (say, zero), tricking the GC to start sweeping prematurely.
+%% The safe bet is therefore to default to the top height.
+%%
+last_switch_test(Config) ->
+    N1 = aecore_suite_utils:node_name(dev1),
+    ok = rpc:call(N1, mnesia, dirty_delete, [aec_chain_state, last_gc_switch]),
+    ok = aecore_suite_utils:stop_node(dev1, Config),
+    aecore_suite_utils:start_node(dev1, Config),
+    aecore_suite_utils:connect(N1),
+    Top = rpc:call(N1, aec_chain, top_height, []),
+    #{last_gc := LastGC} = rpc:call(N1, aec_db_gc, info, [[last_gc]]),
+    ct:log("Top = ~p, LastGC = ~p", [Top, LastGC]),
+    Top = LastGC,
+    ok.
+
 latest_sophia_abi() ->
     aect_test_utils:latest_sophia_abi_version().