OTP24 - move away from lager in favour of logger, update rebar3 and CI.

Author: Martin Cox

.github/workflows/erlang.yml

@@ -1,15 +1,26 @@
 name: Erlang CI
 
-on: [push]
+on:
+  push:
+    branches: [ develop ]
+  pull_request:
+    branches: [ develop ]
+
 
 jobs:
+
   build:
+
     runs-on: ubuntu-latest
+
     container:
-      image: erlang:20.3.8
+      image: erlang:24.2.0.0
+
     steps:
-    - uses: actions/checkout@v1
-    - name: Compile
-      run: rebar3 compile
-    - name: Run tests
-      run: rebar3 do eunit, ct
+      - uses: actions/checkout@v2
+      - name: Compile
+        run: ./rebar3 compile
+      - name: Run xref and dialyzer
+        run: ./rebar3 do xref, dialyzer
+      - name: Run eunit
+        run: ./rebar3 eunit

rebar.config

@@ -31,14 +31,14 @@
    [
     {deps,
      [meck,
-     {cuttlefish, {git, "https://github.com/basho/cuttlefish.git", {branch, "develop-3.0"}}}
+     {cuttlefish, {git, "https://github.com/kyorai/cuttlefish.git", {tag, "v3.0.1"}}}
      ]},
     {eunit_opts, [verbose]}
    ]},
   {eqc,
    [
     {deps,
-     [meck, {faulterl, ".*", {git, "https://github.com/basho/faulterl", {branch, "master"}}}]},
+     [meck, {faulterl, {git, "https://github.com/basho/faulterl", {branch, "master"}}}]},
     {overrides,
      [{override, faulterl, [
                             {plugins, [pc]},

rebar3

@@ -45,6 +45,7 @@
 -export([has_pending_delete_bit/1]).                    % For EUnit tests
 
 -include_lib("kernel/include/file.hrl").
+-include_lib("kernel/include/logger.hrl").
 -include("bitcask.hrl").
 -include("stacktrace.hrl").
 
@@ -415,7 +416,7 @@ fold(State, Fun, Acc0, MaxAge, MaxPut, SeeTombstonesP) ->
                                              end,
                                          case {K, (TStamp < ExpiryTime)} of
                                              {{key_tx_error, TxErr}, _} ->
-                                                 error_logger:error_msg("Error converting key ~p: ~p", [K0, TxErr]),
+                                                 ?LOG_ERROR("Error converting key ~p: ~p", [K0, TxErr]),
                                                  Acc;
                                              {_, true} ->
                                                  Acc;
@@ -479,7 +480,7 @@ open_fold_files(Dirname, Keydir, Count) ->
 maybe_log_missing_file(Dirname, Keydir, ErrFile, enoent) ->
     case is_current_file(Dirname, Keydir, ErrFile) of
         true ->
-            error_logger:error_msg("Unexpectedly missing file ~s", [ErrFile]),
+            ?LOG_ERROR("Unexpectedly missing file ~s", [ErrFile]),
             FileId = bitcask_fileops:file_tstamp(ErrFile),
             %% Forget it to avoid retrying opening it
             _ = bitcask_nifs:keydir_trim_fstats(Keydir, [FileId]),
@@ -524,7 +525,7 @@ subfold(SubFun,[FD | Rest],Acc0) ->
                    Acc1
            catch
                throw:{fold_error, Error, _PartialAcc} ->
-                   error_logger:error_msg("subfold: skipping file ~s: ~p\n",
+                   ?LOG_ERROR("subfold: skipping file ~s: ~p\n",
                                           [FD#filestate.filename, Error]),
                    Acc0
            after
@@ -784,10 +785,10 @@ needs_merge(Ref, Opts) ->
         {ok, 0} ->
             ok;
         {ok, Warn} ->
-            error_logger:info_msg("Trimmed ~p non-existent fstat entries",
+            ?LOG_INFO("Trimmed ~p non-existent fstat entries",
                                   [Warn]);
         Err ->
-            error_logger:error_msg("Error trimming fstats entries: ~p",
+            ?LOG_ERROR("Error trimming fstats entries: ~p",
                                    [Err])
     end,
 
@@ -843,7 +844,7 @@ explicit_merge_files(Dirname) ->
         {error, ReadErr} ->
             case filelib:is_regular(MergeListFile) of
                 true ->
-                    error_logger:error_msg("Invalid merge input file ~s,"
+                    ?LOG_ERROR("Invalid merge input file ~s,"
                                            " deleting : ~p",
                                            [MergeListFile, ReadErr]),
                     _ = file:delete(MergeListFile),
@@ -948,7 +949,7 @@ run_merge_triggers(State, Summary) ->
             %%       recv this information and decide if they want it
             case get_opt(log_needs_merge, State#bc_state.opts) of
                 true ->
-                    error_logger:info_msg("~p needs_merge: ~p\n",
+                    ?LOG_INFO("~p needs_merge: ~p\n",
                                           [State#bc_state.dirname, MergableFiles]);
                 _ ->
                     ok
@@ -1195,7 +1196,7 @@ scan_key_files([Filename | Rest], KeyDir, Acc, CloseFile, KT) ->
                         K = try KT(K0) catch TxErr -> {key_tx_error, TxErr} end,
                         case K of
                             {key_tx_error, KeyTxErr} ->
-                                error_logger:error_msg("Invalid key on load ~p: ~p",
+                                ?LOG_ERROR("Invalid key on load ~p: ~p",
                                                        [K0, KeyTxErr]),
                                 ok;
                             _ ->
@@ -1206,7 +1207,7 @@ scan_key_files([Filename | Rest], KeyDir, Acc, CloseFile, KT) ->
                         K = try KT(K0) catch TxErr -> {key_tx_error, TxErr} end,
                         case K of
                             {key_tx_error, KeyTxErr} ->
-                                error_logger:error_msg("Invalid key on load ~p: ~p",
+                                ?LOG_ERROR("Invalid key on load ~p: ~p",
                                                        [K0, KeyTxErr]);
                             _ ->
                                 bitcask_nifs:keydir_put(KeyDir,
@@ -1388,7 +1389,7 @@ merge_files(#mstate {  dirname = Dirname,
                     end,
                 case K of
                     {key_tx_error, TxErr} ->
-                        error_logger:error_msg("Invalid key on merge ~p: ~p",
+                        ?LOG_ERROR("Invalid key on merge ~p: ~p",
                                                [K0, TxErr]),
                         State0;
                     _ ->
@@ -1400,7 +1401,7 @@ merge_files(#mstate {  dirname = Dirname,
                      State1#mstate{delete_files = [File|DelFiles]}
              catch
                  throw:{fold_error, Error, _PartialAcc} ->
-                     error_logger:error_msg(
+                     ?LOG_ERROR(
                        "merge_files: skipping file ~s in ~s: ~p\n",
                        [File#filestate.filename, Dirname, Error]),
                      State
@@ -1926,7 +1927,7 @@ purge_setuid_files(Dirname) ->
                 if StaleFs == [] ->
                         ok;
                    true ->
-                        error_logger:info_msg("Deleted ~p stale merge input "
+                        ?LOG_INFO("Deleted ~p stale merge input "
                                               "files from ~p\n",
                                               [length(StaleFs), Dirname])
                 end
@@ -1939,7 +1940,7 @@ purge_setuid_files(Dirname) ->
                 bitcask_lockops:release(WriteLock)
             end;
         Else ->
-            error_logger:info_msg("Lock failed trying deleting stale merge "
+            ?LOG_INFO("Lock failed trying deleting stale merge "
                                   "input files from ~p: ~p\n", [Dirname, Else])
     end.
 
@@ -1973,7 +1974,7 @@ expiry_merge([File | Files], LiveKeyDir, KT, Acc0) ->
                   K = try KT(K0) catch TxErr -> {key_tx_error, TxErr} end,
                   case K of
                       {key_tx_error, KeyTxErr} ->
-                          error_logger:error_msg("Invalid key on merge ~p: ~p",
+                          ?LOG_ERROR("Invalid key on merge ~p: ~p",
                                                  [K0, KeyTxErr]);
                       _ ->
                           bitcask_nifs:keydir_remove(LiveKeyDir, K, Tstamp,
@@ -1983,11 +1984,11 @@ expiry_merge([File | Files], LiveKeyDir, KT, Acc0) ->
         end,
     case bitcask_fileops:fold_keys(File, Fun, ok, default) of
         {error, Reason} ->
-            error_logger:error_msg("Error folding keys for ~p: ~p\n",
+            ?LOG_ERROR("Error folding keys for ~p: ~p\n",
                                    [File#filestate.filename,Reason]),
             Acc = Acc0;
         _ ->
-            error_logger:info_msg("All keys expired in: ~p scheduling "
+            ?LOG_INFO("All keys expired in: ~p scheduling "
                                   "file for deletion\n",
                                   [File#filestate.filename]),
             Acc = lists:append(Acc0, [File])
@@ -2005,7 +2006,7 @@ error_msg_perhaps(_Fmt, _Args) ->
     ok.
 -else. %TEST
 error_msg_perhaps(Fmt, Args) ->
-    error_logger:error_msg(Fmt, Args).
+    ?LOG_ERROR(Fmt, Args).
 -endif. %TEST
 
 %% ===================================================================
@@ -2043,7 +2044,7 @@ a0_test2() ->
     code:add_pathz("../ebin"),
     application:start(erlang),
     Mode = bitcask_io:determine_file_module(),
-    error_logger:info_msg("Bitcask IO mode is: ~p\n", [Mode]).
+    ?LOG_INFO("Bitcask IO mode is: ~p\n", [Mode]).
 
 roundtrip_test_() ->
     {timeout, 60, fun roundtrip_test2/0}.

src/bitcask.erl

@@ -44,6 +44,8 @@
 -record(state, {fd    :: file:fd() | undefined,
                 owner :: pid() | undefined}).
 
+-include_lib("kernel/include/logger.hrl").
+
 %%%===================================================================
 %%% API
 %%%===================================================================
@@ -141,15 +143,15 @@ handle_call({file_open, Owner, Filename, Opts}, _From, State) ->
                {_, true} ->
                    [read, write, exclusive, raw, binary]
            end,
-    _ = [error_logger:warning_msg("Bitcask file option '~p' not supported~n", [Opt])
+    _ = [?LOG_WARNING("Bitcask file option '~p' not supported~n", [Opt])
      || Opt <- [o_sync],
         proplists:get_bool(Opt, Opts)],
     case file:open(Filename, Mode) of
         {ok, Fd} ->
             State2 = State#state{fd=Fd, owner=Owner},
             {reply, ok, State2};
         Error = {error, Reason} ->
-            error_logger:warning_msg("Failed to open file ~p: ~p~n",
+            ?LOG_WARNING("Failed to open file ~p: ~p~n",
                                    [Filename, Reason]),
             {stop, {file_open_failed, Reason}, Error, State}
     end;

src/bitcask_file.erl

@@ -45,6 +45,7 @@
 -export([read_file_info/1, write_file_info/2, is_file/1]).
 
 -include_lib("kernel/include/file.hrl").
+-include_lib("kernel/include/logger.hrl").
 
 -include("bitcask.hrl").
 
@@ -426,15 +427,15 @@ fold_keys(State, Fun, Acc, recovery, _, true) ->
             Acc0;
         {error, Reason} ->
             HintFile = hintfile_name(State),
-            error_logger:warning_msg("Hintfile '~s' failed fold: ~p\n",
+            ?LOG_WARNING("Hintfile '~s' failed fold: ~p\n",
                                      [HintFile, Reason]),
             fold_keys_loop(State, 0, Fun, Acc);
         Acc1 ->
             Acc1
     end;
 fold_keys(State, Fun, Acc, recovery, _, false) ->
     HintFile = hintfile_name(State),
-    error_logger:warning_msg("Hintfile '~s' invalid\n",
+    ?LOG_WARNING("Hintfile '~s' invalid\n",
                              [HintFile]),
     fold_keys_loop(State, 0, Fun, Acc).
 
@@ -542,7 +543,7 @@ read_crc(Fd) ->
 %% ===================================================================
 
 fold_int_loop(_Bytes, _Fun, Acc, _Consumed, {Filename, _, Offset, 20}) ->
-    error_logger:error_msg("fold_loop: CRC error limit at file ~p offset ~p\n",
+    ?LOG_ERROR("fold_loop: CRC error limit at file ~p offset ~p\n",
                            [Filename, Offset]),
     {done, Acc};
 fold_int_loop(<<Crc32:?CRCSIZEFIELD, Tstamp:?TSTAMPFIELD,
@@ -560,7 +561,7 @@ fold_int_loop(<<Crc32:?CRCSIZEFIELD, Tstamp:?TSTAMPFIELD,
                           {Filename, FTStamp, Offset + TotalSz,
                            CrcSkipCount});
         _ ->
-            error_logger:error_msg("fold_loop: CRC error at file ~s offset ~p, "
+            ?LOG_ERROR("fold_loop: CRC error at file ~s offset ~p, "
                                    "skipping ~p bytes\n",
                                    [Filename, Offset, TotalSz]),
             fold_int_loop(Rest, Fun, Acc0, Consumed0 + TotalSz,
@@ -585,7 +586,7 @@ fold_keys_loop(#filestate{fd=Fd, filename=Filename, tstamp=FTStamp}, Offset,
     end.
 
 fold_keys_int_loop(_Bytes, _Fun, Acc, _Consumed, {Filename, _, Offset, 20}) ->
-    error_logger:error_msg("fold_loop: CRC error limit at file ~p offset ~p\n",
+    ?LOG_ERROR("fold_loop: CRC error limit at file ~p offset ~p\n",
                            [Filename, Offset]),
     {done, Acc};
 fold_keys_int_loop(<<Crc32:?CRCSIZEFIELD, Tstamp:?TSTAMPFIELD,
@@ -607,7 +608,7 @@ fold_keys_int_loop(<<Crc32:?CRCSIZEFIELD, Tstamp:?TSTAMPFIELD,
                                {Filename, FTStamp, Offset + TotalSz,
                                 CrcSkipCount});
         _ ->
-            error_logger:error_msg("fold_loop: CRC error at file ~s offset ~p, "
+            ?LOG_ERROR("fold_loop: CRC error at file ~s offset ~p, "
                                    "skipping ~p bytes\n",
                                    [Filename, Offset, TotalSz]),
             fold_keys_int_loop(Rest, Fun, Acc0, Consumed0 + TotalSz,
@@ -664,7 +665,7 @@ fold_hintfile_loop(<<Tstamp:?TSTAMPFIELD, KeySz:?KEYSIZEFIELD,
             Consumed = KeySz + ?HINT_RECORD_SZ + Consumed0,
             fold_hintfile_loop(Rest, Fun, Acc, Consumed, Args);
         false ->
-            error_logger:warning_msg("Hintfile '~s' contains pointer ~p ~p "
+            ?LOG_WARNING("Hintfile '~s' contains pointer ~p ~p "
                                      "that is greater than total data size ~p\n",
                                      [HintFile, Offset, TotalSz, DataSize]),
             {error, {trunc_hintfile, Acc0}}
@@ -865,7 +866,7 @@ get_efile_port() ->
                     put(Key, Port),
                     get_efile_port();
                 Err ->
-                    error_logger:error_msg("get_efile_port: ~p\n", [Err]),
+                    ?LOG_ERROR("get_efile_port: ~p\n", [Err]),
                     timer:sleep(1000),
                     get_efile_port()
             end;

src/bitcask_fileops.erl

@@ -27,6 +27,8 @@
          read_activefile/2,
          write_activefile/2]).
 
+-include_lib("kernel/include/logger.hrl").
+
 -ifdef(PULSE).
 -compile({parse_transform, pulse_instrument}).
 -include_lib("pulse_otp/include/pulse_otp.hrl").
@@ -147,7 +149,7 @@ delete_stale_lock(Filename) ->
                         end;
 
                     {error, Reason} ->
-                        error_logger:error_msg("Failed to read lock data from ~s: ~p\n",
+                        ?LOG_ERROR("Failed to read lock data from ~s: ~p\n",
                                                [Filename, Reason]),
                         not_stale
                 end
@@ -163,7 +165,7 @@ delete_stale_lock(Filename) ->
 
         {error, Reason} ->
             %% Failed to open the lock for reading due to other errors.
-            error_logger:error_msg("Failed to open lock file ~s: ~p\n",
+            ?LOG_ERROR("Failed to open lock file ~s: ~p\n",
                                    [Filename, Reason]),
             not_stale
     end.

src/bitcask_lockops.erl

@@ -48,6 +48,8 @@
 -record(state, { queue :: list(),
                 worker :: undefined | pid()}).
 
+-include_lib("kernel/include/logger.hrl").
+
 %% ====================================================================
 %% API
 %% ====================================================================
@@ -142,7 +144,7 @@ handle_info({'EXIT', _Pid, normal}, #state { queue = Q } = State) ->
     end;
 
 handle_info({'EXIT', Pid, Reason}, #state { worker = Pid } = State) ->
-    error_logger:error_msg("Merge worker PID exited: ~p\n", [Reason]),
+    ?LOG_ERROR("Merge worker PID exited: ~p\n", [Reason]),
     {stop, State}.
 
 terminate(_Reason, State) ->
@@ -199,10 +201,10 @@ do_merge(Args) ->
             [_,_,Args3] = Args,
             case Result of
                 ok ->
-                    error_logger:info_msg("Merged ~p in ~p seconds.\n",
+                    ?LOG_INFO("Merged ~p in ~p seconds.\n",
                                           [Args3, ElapsedSecs]);
                 {Error, Reason} when Error == error; Error == 'EXIT' ->
-                    error_logger:error_msg("Failed to merge ~p: ~p\n",
+                    ?LOG_ERROR("Failed to merge ~p: ~p\n",
                                            [Args3, Reason])
             end;
         false ->
@@ -219,7 +221,7 @@ merge_window() ->
                                         EndHour >= 0, EndHour =< 23 ->
             {StartHour, EndHour};
         Other ->
-            error_logger:error_msg("Invalid bitcask_merge window specified: ~p. "
+            ?LOG_ERROR("Invalid bitcask_merge window specified: ~p. "
                                    "Defaulting to 'always'.\n", [Other]),
             always
     end.