From 0b5bc2fcc313c8f16592f65d2e883415a6cfac32 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Fri, 24 Feb 2023 09:22:45 +0100 Subject: [PATCH 1/5] Store: improve v3_0_0 upgrade error --- src/lib_shell_services/store_errors.ml | 20 ++++++++++++++++++++ src/lib_store/unix/store.ml | 5 +++++ 2 files changed, 25 insertions(+) diff --git a/src/lib_shell_services/store_errors.ml b/src/lib_shell_services/store_errors.ml index f1fc6538f355..f985017bd32e 100644 --- a/src/lib_shell_services/store_errors.ml +++ b/src/lib_shell_services/store_errors.ml @@ -1296,3 +1296,23 @@ let () = Data_encoding.(obj1 (req "kind" corruption_kind_encoding)) (function Corrupted_store k -> Some k | _ -> None) (fun k -> Corrupted_store k) + +(* Store upgrade errors *) +type error += Cannot_find_chain_dir of string + +let () = + Error_monad.register_error_kind + `Permanent + ~id:"store.cannot_find_chain_dir" + ~title:"Cannot find chain dir" + ~description:"Cannot find chain dir while upgrading storage" + ~pp:(fun ppf path -> + Format.fprintf + ppf + "Failed to upgrade the storage. The chain directory %s cannot be \ + found. Make sure that the data directory contains data of the \ + expected network." + path) + Data_encoding.(obj1 (req "path" string)) + (function Cannot_find_chain_dir p -> Some p | _ -> None) + (fun p -> Cannot_find_chain_dir p) diff --git a/src/lib_store/unix/store.ml b/src/lib_store/unix/store.ml index 7d2a51e2f00a..8ca1d011bd56 100644 --- a/src/lib_store/unix/store.ml +++ b/src/lib_store/unix/store.ml @@ -2985,6 +2985,11 @@ let v_3_0_upgrade ~store_dir genesis = let*! () = List.iter_s (fun f -> f ()) !cleanups in Lwt.return_error err) (fun () -> + let* () = + let chain_dir_path = Naming.dir_path chain_dir in + let*! chain_dir_exists = Lwt_unix.file_exists chain_dir_path in + fail_unless chain_dir_exists (Cannot_find_chain_dir chain_dir_path) + in let* () = upgrade_protocol_levels ~chain_dir ~cleanups ~finalizers in let* () = Block_store.v_3_0_upgrade chain_dir ~cleanups ~finalizers in let*! () = List.iter_s (fun f -> f ()) !finalizers in -- GitLab From 45a38315ebd0ae6925d4b1c72d742564a1fb0afe Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Fri, 24 Feb 2023 09:37:46 +0100 Subject: [PATCH 2/5] Lib_node_config: improve and use read_data_dir --- src/bin_node/node_replay_command.ml | 8 ++- src/bin_node/node_snapshot_command.ml | 60 ++--------------- src/bin_node/node_storage_command.ml | 38 ++--------- src/lib_node_config/shared_arg.ml | 92 ++++++++++++++++++++------- src/lib_node_config/shared_arg.mli | 11 +++- 5 files changed, 92 insertions(+), 117 deletions(-) diff --git a/src/bin_node/node_replay_command.ml b/src/bin_node/node_replay_command.ml index c46e45608b66..a24beef3d263 100644 --- a/src/bin_node/node_replay_command.ml +++ b/src/bin_node/node_replay_command.ml @@ -491,9 +491,13 @@ let process verbosity singleprocess strict blocks args = in let run = let open Lwt_result_syntax in - let* data_dir = Shared_arg.read_data_dir args in + let* data_dir, config = + Shared_arg.resolve_data_dir_and_config_file + ?data_dir:args.Shared_arg.data_dir + ?config_file:(Some args.config_file) + () + in let* () = check_data_dir data_dir in - let* config = Shared_arg.read_and_patch_config_file args in run ?verbosity ~singleprocess ~strict config blocks in match Lwt_main.run run with diff --git a/src/bin_node/node_snapshot_command.ml b/src/bin_node/node_snapshot_command.ml index c4e32fb5a58d..e56f24681504 100644 --- a/src/bin_node/node_snapshot_command.ml +++ b/src/bin_node/node_snapshot_command.ml @@ -108,16 +108,6 @@ module Event = struct last checkpoint as the default value" ~level:Notice () - - let overriding_config_file_arg = - declare_1 - ~section - ~name:"overriding_config_file_arg" - ~msg: - "the data directory from the --config-file argument was overridden by \ - the given --data-dir path: {path}" - ~level:Warning - ("path", Data_encoding.string) end module Term = struct @@ -134,33 +124,12 @@ module Term = struct let run = let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in - let actual_data_dir = - Option.value ~default:Config_file.default_data_dir data_dir - in - let config_file = - Option.value - ~default: - Filename.Infix.( - actual_data_dir // Data_version.default_config_file_name) - config_file + let* data_dir, node_config = + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () in - let* node_config = Config_file.read config_file in let ({genesis; chain_name; _} : Config_file.blockchain_network) = node_config.blockchain_network in - let*! data_dir = - (* The --data-dir argument overrides the potentially given - configuration file. *) - match data_dir with - | Some data_dir -> - let*! () = - if not (String.equal data_dir node_config.data_dir) then - Event.(emit overriding_config_file_arg) data_dir - else Lwt.return_unit - in - Lwt.return data_dir - | None -> Lwt.return node_config.data_dir - in let* () = Data_version.ensure_data_dir genesis data_dir in let context_dir = Data_version.context_dir data_dir in let store_dir = Data_version.store_dir data_dir in @@ -192,29 +161,8 @@ module Term = struct let run = let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in - let actual_data_dir = - Option.value ~default:Config_file.default_data_dir data_dir - in - let config_file = - Option.value - ~default: - Filename.Infix.( - actual_data_dir // Data_version.default_config_file_name) - config_file - in - let* node_config = Config_file.read config_file in - let*! data_dir = - (* The --data-dir argument overrides the potentially given - configuration file. *) - match data_dir with - | Some data_dir -> - let*! () = - if not (String.equal data_dir node_config.data_dir) then - Event.(emit overriding_config_file_arg) data_dir - else Lwt.return_unit - in - Lwt.return data_dir - | None -> Lwt.return node_config.data_dir + let* data_dir, node_config = + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () in let*! existing_data_dir = Lwt_unix.file_exists data_dir in let ({genesis; _} : Config_file.blockchain_network) = diff --git a/src/bin_node/node_storage_command.ml b/src/bin_node/node_storage_command.ml index 49c96f627c77..572c2764cbdd 100644 --- a/src/bin_node/node_storage_command.ml +++ b/src/bin_node/node_storage_command.ml @@ -179,28 +179,13 @@ module Term = struct Shared_arg.process_command (let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in - let actual_data_dir = - Option.value ~default:Config_file.default_data_dir data_dir + let* data_dir, node_config = + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () in - let config_file = - Option.value - ~default: - Filename.Infix.( - actual_data_dir // Data_version.default_config_file_name) - config_file - in - let* node_config = Config_file.read config_file in let ({genesis; _} : Config_file.blockchain_network) = node_config.blockchain_network in let chain_id = Chain_id.of_block_hash genesis.block in - let data_dir = - (* The --data-dir argument overrides the potentially given - configuration file. *) - match data_dir with - | Some data_dir -> data_dir - | None -> node_config.data_dir - in let* () = Data_version.ensure_data_dir genesis data_dir in let context_dir = Data_version.context_dir data_dir in let store_dir = Data_version.store_dir data_dir in @@ -239,28 +224,13 @@ module Term = struct Shared_arg.process_command (let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in - let actual_data_dir = - Option.value ~default:Config_file.default_data_dir data_dir + let* data_dir, node_config = + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () in - let config_file = - Option.value - ~default: - Filename.Infix.( - actual_data_dir // Data_version.default_config_file_name) - config_file - in - let* node_config = Config_file.read config_file in let ({genesis; _} : Config_file.blockchain_network) = node_config.blockchain_network in let chain_id = Chain_id.of_block_hash genesis.block in - let data_dir = - (* The --data-dir argument overrides the potentially given - configuration file. *) - match data_dir with - | Some data_dir -> data_dir - | None -> node_config.data_dir - in let* () = Data_version.ensure_data_dir genesis data_dir in let context_dir = Data_version.context_dir data_dir in let store_dir = Data_version.store_dir data_dir in diff --git a/src/lib_node_config/shared_arg.ml b/src/lib_node_config/shared_arg.ml index c69be2980698..d367bd64c97f 100644 --- a/src/lib_node_config/shared_arg.ml +++ b/src/lib_node_config/shared_arg.ml @@ -109,6 +109,37 @@ let () = | Network_http_error (status, body) -> Some (status, body) | _ -> None) (fun (status, body) -> Network_http_error (status, body)) +module Event = struct + include Internal_event.Simple + + let section = ["node"; "main"] + + let disabled_bootstrap_peers = + Internal_event.Simple.declare_0 + ~section + ~name:"disabled_bootstrap_peers" + ~msg:"disabled bootstrap peers" + () + + let testchain_is_deprecated = + Internal_event.Simple.declare_0 + ~section + ~level:Warning + ~name:"enable_testchain_is_deprecated" + ~msg:"The command-line option `--enable-testchain` is deprecated." + () + + let overriding_config_file_arg = + declare_1 + ~section + ~name:"overriding_config_file_arg" + ~msg: + "the data directory from the --config-file argument was overridden by \ + the given --data-dir path: {path}" + ~level:Warning + ("path", Data_encoding.string) +end + let decode_net_config source json = let open Result_syntax in match @@ -691,12 +722,44 @@ let read_config_file args = if Sys.file_exists args.config_file then Config_file.read args.config_file else return Config_file.default_config -let read_data_dir args = +let resolve_data_dir_and_config_file ?data_dir ?config_file () = let open Lwt_result_syntax in - let* cfg = read_config_file args in - let {data_dir; _} = args in - let data_dir = Option.value ~default:cfg.data_dir data_dir in - return data_dir + let config_file_arg = Option.is_some config_file in + let actual_data_dir = + Option.value ~default:Config_file.default_data_dir data_dir + in + let config_file = + Option.value + ~default: + Filename.Infix.( + actual_data_dir // Data_version.default_config_file_name) + config_file + in + let* node_config = Config_file.read config_file in + (* Returns true if the given paths are equal, after removing the + potential trailing slash. *) + let paths_equals p1 p2 = + let f x = + List.filter (fun s -> s <> String.empty) (String.split_on_char '/' x) + in + f p1 = f p2 + in + let*! data_dir = + (* The --data-dir argument overrides the potentially given + configuration file. *) + match data_dir with + | Some data_dir -> + let*! () = + if + (not (paths_equals data_dir node_config.data_dir)) + && config_file_arg + then Event.(emit overriding_config_file_arg) data_dir + else Lwt.return_unit + in + Lwt.return data_dir + | None -> Lwt.return node_config.data_dir + in + return (data_dir, node_config) type error += | Network_configuration_mismatch of { @@ -755,25 +818,6 @@ let () = (function Invalid_command_line_arguments x -> Some x | _ -> None) (fun explanation -> Invalid_command_line_arguments explanation) -module Event = struct - include Internal_event.Simple - - let disabled_bootstrap_peers = - Internal_event.Simple.declare_0 - ~section:["node"; "main"] - ~name:"disabled_bootstrap_peers" - ~msg:"disabled bootstrap peers" - () - - let testchain_is_deprecated = - Internal_event.Simple.declare_0 - ~section:["node"; "main"] - ~level:Warning - ~name:"enable_testchain_is_deprecated" - ~msg:"The command-line option `--enable-testchain` is deprecated." - () -end - let patch_network ?(cfg = Config_file.default_config) blockchain_network = let open Lwt_result_syntax in return {cfg with blockchain_network} diff --git a/src/lib_node_config/shared_arg.mli b/src/lib_node_config/shared_arg.mli index 752ac8170299..9c3771cc75f9 100644 --- a/src/lib_node_config/shared_arg.mli +++ b/src/lib_node_config/shared_arg.mli @@ -99,7 +99,16 @@ end val read_config_file : t -> Config_file.t tzresult Lwt.t -val read_data_dir : t -> string tzresult Lwt.t +(* Returns the [data_dir] and [config_file] from either [config_file] + (configuration file value) or [data_dir] (command line value), if + any. Otherwise, returns the default values. If both the + [config_file] and [data_dir] are given, the [data_dir] overrides + the value from the [config_file]. *) +val resolve_data_dir_and_config_file : + ?data_dir:string -> + ?config_file:string -> + unit -> + (string * Config_file.t, tztrace) result Lwt.t (** Modify a node's network configuration. -- GitLab From 139d65bf9aec3db19d4504b12a13d6d95bdca615 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Fri, 24 Feb 2023 10:14:56 +0100 Subject: [PATCH 3/5] Bin_node: remove all node arguments from various commands --- src/bin_node/node_reconstruct_command.ml | 11 +++++++---- src/bin_node/node_replay_command.ml | 9 +++------ src/bin_node/node_upgrade_command.ml | 17 +++++++++-------- 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/bin_node/node_reconstruct_command.ml b/src/bin_node/node_reconstruct_command.ml index de9bb4f572f4..e58accadf0c6 100644 --- a/src/bin_node/node_reconstruct_command.ml +++ b/src/bin_node/node_reconstruct_command.ml @@ -44,12 +44,13 @@ let () = (** Main *) module Term = struct - let process args sandbox_file progress_display_mode = + let process data_dir config_file sandbox_file progress_display_mode = let run = let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in - let* node_config = Shared_arg.read_and_patch_config_file args in - let data_dir = node_config.data_dir in + let* data_dir, node_config = + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () + in let ({genesis; _} : Config_file.blockchain_network) = node_config.blockchain_network in @@ -132,7 +133,9 @@ module Term = struct let term = let open Cmdliner.Term in - ret (const process $ Shared_arg.Term.args $ sandbox $ progress_display_mode) + ret + (const process $ Shared_arg.Term.data_dir $ Shared_arg.Term.config_file + $ sandbox $ progress_display_mode) end module Manpage = struct diff --git a/src/bin_node/node_replay_command.ml b/src/bin_node/node_replay_command.ml index a24beef3d263..294b2e7bc94f 100644 --- a/src/bin_node/node_replay_command.ml +++ b/src/bin_node/node_replay_command.ml @@ -484,7 +484,7 @@ let check_data_dir dir = msg = Some (Format.sprintf "directory '%s' does not exists" dir); }) -let process verbosity singleprocess strict blocks args = +let process verbosity singleprocess strict blocks data_dir config_file = let verbosity = let open Internal_event in match verbosity with [] -> None | [_] -> Some Info | _ -> Some Debug @@ -492,10 +492,7 @@ let process verbosity singleprocess strict blocks args = let run = let open Lwt_result_syntax in let* data_dir, config = - Shared_arg.resolve_data_dir_and_config_file - ?data_dir:args.Shared_arg.data_dir - ?config_file:(Some args.config_file) - () + Shared_arg.resolve_data_dir_and_config_file ?data_dir ?config_file () in let* () = check_data_dir data_dir in run ?verbosity ~singleprocess ~strict config blocks @@ -579,7 +576,7 @@ module Term = struct Cmdliner.Term.( ret (const process $ verbosity $ singleprocess $ strict $ blocks - $ Shared_arg.Term.args)) + $ Shared_arg.Term.data_dir $ Shared_arg.Term.config_file)) end module Manpage = struct diff --git a/src/bin_node/node_upgrade_command.ml b/src/bin_node/node_upgrade_command.ml index 030584291df4..ee77847db7fa 100644 --- a/src/bin_node/node_upgrade_command.ml +++ b/src/bin_node/node_upgrade_command.ml @@ -57,17 +57,18 @@ module Term = struct & pos 0 (some (parser, printer)) None & info [] ~docv:"UPGRADE" ~doc - let process subcommand args status sandbox_file = + let process subcommand data_dir config_file status sandbox_file = let run = let open Lwt_result_syntax in let*! () = Log_config.init_internal_events_with_defaults () in match subcommand with | Storage -> ( - let* config = Config_file.read args.Shared_arg.config_file in - (* Use the command-line argument data-dir if present: the - configuration data-dir may be inconsistent if the - directory was moved. *) - let data_dir = Option.value ~default:config.data_dir args.data_dir in + let* data_dir, config = + Shared_arg.resolve_data_dir_and_config_file + ?data_dir + ?config_file + () + in let*! r = Lwt_lock_file.try_with_lock ~when_locked:(fun () -> @@ -141,8 +142,8 @@ module Term = struct let term = Cmdliner.Term.( ret - (const process $ subcommand_arg $ Shared_arg.Term.args $ status - $ sandbox)) + (const process $ subcommand_arg $ Shared_arg.Term.data_dir + $ Shared_arg.Term.config_file $ status $ sandbox)) end module Manpage = struct -- GitLab From db2c81f2de830e44635faf4c4552d61e70f1644a Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Thu, 2 Mar 2023 08:47:49 +0100 Subject: [PATCH 4/5] Node_config: improve error when empty dirs are encountered --- src/lib_node_config/data_version.ml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/lib_node_config/data_version.ml b/src/lib_node_config/data_version.ml index 8e7316e6dee7..eaa3e3027206 100644 --- a/src/lib_node_config/data_version.ml +++ b/src/lib_node_config/data_version.ml @@ -228,7 +228,8 @@ let () = ~pp:(fun ppf path -> Format.fprintf ppf - "Tried to read version file at '%s', but the file could not be parsed." + "Tried to read version file at '%s', but the file could not be found \ + or parsed." path) (function Could_not_read_data_dir_version path -> Some path | _ -> None) (fun path -> Could_not_read_data_dir_version path) ; @@ -373,13 +374,11 @@ let read_version_file version_file = try return (Data_encoding.Json.destruct Version.encoding json) with _ -> tzfail (Could_not_read_data_dir_version version_file) -let check_data_dir_version files data_dir = +let check_data_dir_version data_dir = let open Lwt_result_syntax in let version_file = version_file data_dir in let*! file_exists = Lwt_unix.file_exists version_file in - if not file_exists then - let msg = Some (clean_directory files) in - tzfail (Invalid_data_dir {data_dir; msg}) + if not file_exists then tzfail (Could_not_read_data_dir_version version_file) else let* version = read_version_file version_file in if Version.(equal version current_version) then return_none @@ -421,7 +420,7 @@ let ensure_data_dir ~mode data_dir = | files, Is_bare -> let msg = Some (clean_directory files) in tzfail (Invalid_data_dir {data_dir; msg}) - | files, Is_compatible -> check_data_dir_version files data_dir + | _, Is_compatible -> check_data_dir_version data_dir | _files, Exists -> return_none else let*! () = Lwt_utils_unix.create_dir ~perm:0o700 data_dir in -- GitLab From 618daa494e5140708453dabbb4f710ebd8837426 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Tue, 7 Mar 2023 15:20:12 +0100 Subject: [PATCH 5/5] Node: cleaner trace when storage upgrade fails --- src/lib_node_config/data_version.ml | 9 ++++----- src/lib_store/shared/store_events.ml | 7 +++---- src/lib_store/unix/store.ml | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/lib_node_config/data_version.ml b/src/lib_node_config/data_version.ml index eaa3e3027206..e92bf36212ce 100644 --- a/src/lib_node_config/data_version.ml +++ b/src/lib_node_config/data_version.ml @@ -318,13 +318,12 @@ module Events = struct () let aborting_upgrade = - declare_1 + declare_0 ~section ~level:Notice ~name:"aborting_upgrade" - ~msg:"failed to upgrade storage: {error}" - ~pp1:Error_monad.pp_print_trace - ("error", Error_monad.trace_encoding) + ~msg:"failed to upgrade storage" + () let upgrade_status = declare_2 @@ -445,7 +444,7 @@ let upgrade_data_dir ~data_dir genesis ~chain_name ~sandbox_parameters = let*! () = Events.(emit update_success ()) in return_unit | Error e -> - let*! () = Events.(emit aborting_upgrade e) in + let*! () = Events.(emit aborting_upgrade) () in Lwt.return (Error e)) let ensure_data_dir ?(mode = Is_compatible) genesis data_dir = diff --git a/src/lib_store/shared/store_events.ml b/src/lib_store/shared/store_events.ml index 4584fa9c53e5..6a458b084809 100644 --- a/src/lib_store/shared/store_events.ml +++ b/src/lib_store/shared/store_events.ml @@ -476,13 +476,12 @@ let notify_merge_error = ("errs", Error_monad.trace_encoding) let upgrade_store_failed = - declare_1 + declare_0 ~section ~level:Error ~name:"upgrade_store_failed" - ~msg:"store upgrade failed, cleaning up temporary files: {errs}" - ~pp1:(fun ppf -> Format.fprintf ppf "%a" Error_monad.pp_print_trace) - ("errs", Error_monad.trace_encoding) + ~msg:"store upgrade failed, cleaning up temporary files" + () let upgrade_store_started = declare_0 diff --git a/src/lib_store/unix/store.ml b/src/lib_store/unix/store.ml index 8ca1d011bd56..d1a05a5a039a 100644 --- a/src/lib_store/unix/store.ml +++ b/src/lib_store/unix/store.ml @@ -2981,7 +2981,7 @@ let v_3_0_upgrade ~store_dir genesis = in protect ~on_error:(fun err -> - let*! () = Store_events.(emit upgrade_store_failed) err in + let*! () = Store_events.(emit upgrade_store_failed) () in let*! () = List.iter_s (fun f -> f ()) !cleanups in Lwt.return_error err) (fun () -> -- GitLab