From 151bd929ba5bb91726378e0870ff7a0a7410ce6f Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Wed, 6 Nov 2024 15:51:56 +0100 Subject: [PATCH 1/3] Store/block_store: minor code/doc improvements --- src/lib_store/unix/block_store.ml | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/lib_store/unix/block_store.ml b/src/lib_store/unix/block_store.ml index 433a6d097f09..c2f044924229 100644 --- a/src/lib_store/unix/block_store.ml +++ b/src/lib_store/unix/block_store.ml @@ -988,8 +988,9 @@ let update_floating_stores block_store ~history_mode ~ro_store ~rw_store {verbosity = Debug; metadata = [("prometheus", "")]} "copy all lpbl predecessors"]) in - (* 2. Retrieve ALL cycles (potentially more than one) *) - (* 2.1. We write back to the new store all the blocks from + (* 2. Retrieve ALL cycles (potentially more than one) + + 2.1. We write back to the new store all the blocks from [lpbl_block] to the end of the file(s). 2.2 At the same time, retrieve the list of cycle bounds: i.e. the @@ -1259,9 +1260,13 @@ let compute_lowest_bound_to_preserve_in_floating block_store ~new_head (Int32.of_int (match Block_repr.metadata lpbl_block with | None -> - (* FIXME: this is not valid but it is a good - approximation of the max_op_ttl of a block where the - metadata is missing. *) + (* This is not valid but it is a good approximation of + the max_op_ttl of a block where the metadata is + missing. This is harmless if the max_op_ttl of the + head is bigger: we only store more block data than + expected. If the max_op_ttl has decreased in the + head, this might generate a storage with a broken + invariant, but which is not breaking. *) Block_repr.max_operations_ttl new_head_metadata | Some metadata -> Block_repr.max_operations_ttl metadata))) @@ -1537,7 +1542,6 @@ let merge_stores ?(cycle_size_limit = default_cycle_size_limit) block_store (* Lock the block store to avoid RO instances to open the state while the file descriptors are being updated. *) let*! () = lock block_store.lockfile in - (instantiate_temporary_floating_store block_store [@profiler.record_s -- GitLab From e0b2d883f322ac79a12d97e8a09d2f2a51bee4ed Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Wed, 6 Nov 2024 16:46:53 +0100 Subject: [PATCH 2/3] Store/cemented_store: minor code/doc improvements --- src/lib_store/unix/cemented_block_store.ml | 3 ++ src/lib_store/unix/cemented_block_store.mli | 46 ++++++++++----------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/lib_store/unix/cemented_block_store.ml b/src/lib_store/unix/cemented_block_store.ml index 9ae42db3f5ff..15c17abf0da2 100644 --- a/src/lib_store/unix/cemented_block_store.ml +++ b/src/lib_store/unix/cemented_block_store.ml @@ -115,6 +115,9 @@ let may_synchronize_indexes `log_size * log_entry` where a `log_entry` is roughly 56 bytes. *) let default_index_log_size = 10_000 +(* Default Zip compression level. This is an integer between 0 and 9, + with 0 meaning no compression (store as is), 1 lowest compression, + 9 highest compression. *) let default_compression_level = 9 (* Defines the maximum number of file descriptors that are cached to diff --git a/src/lib_store/unix/cemented_block_store.mli b/src/lib_store/unix/cemented_block_store.mli index 322bb8de8994..4723328e099b 100644 --- a/src/lib_store/unix/cemented_block_store.mli +++ b/src/lib_store/unix/cemented_block_store.mli @@ -120,11 +120,11 @@ type cemented_blocks_file = { file : [`Cemented_blocks_file] Naming.file; } -(** [init ?log_size ~cemented_blocks_dir ~readonly] creates or loads - an existing cemented block store at path - [cemented_blocks_dir]. [cemented_blocks_dir] will be created if it - does not exists. If [readonly] is true, cementing blocks will - result in an error. [log_size] determines the index cache size. *) +(** [init ?log_size chain_dir ~readonly] creates or loads an existing + cemented block store at path [chain_dir]. [chain_dir] will be + created if it does not exists. If [~readonly] is [true], cementing + blocks will result in an error. [?log_size] determines the index + cache size. *) val init : ?log_size:int -> [< `Chain_dir] Naming.directory -> @@ -167,22 +167,22 @@ val cemented_block_hash_index : t -> Cemented_block_hash_index.t no effect on RW instances. *) val may_synchronize_indexes : t -> unit -(** [load_table ~cemented_blocks_dir] reads the [cemented_blocks_dir] +(** [load_table cemented_blocks_dir] returns the [cemented_blocks_files] in [cemented_blocks_dir] directory and instantiate the cemented blocks chunks files. *) val load_table : [`Cemented_blocks_dir] Naming.directory -> cemented_blocks_file array option tzresult Lwt.t -(** [load_metadata_table ~cemented_blocks_dir] similar to - [load_table], but for the cemented metadata files. *) +(** [load_metadata_table cemented_blocks_dir] similar to [load_table], + but for the cemented metadata files. *) val load_metadata_table : [`Cemented_blocks_dir] Naming.directory -> cemented_metadata_file array option tzresult Lwt.t (** [find_block_file cemented_store block_level] lookups the - [cemented_store] to find the cemented block chunk file that - contains the block at level [block_level]. Returns [None] if the - block cannot be found.*) + [cemented_store] to find the cemented block chunk file that + contains the block at level [block_level]. Returns [None] if the + block cannot be found.*) val find_block_file : t -> int32 -> cemented_blocks_file option (** [is_cemented cemented_store block_hash] checks if the [block_hash] @@ -232,9 +232,10 @@ val get_highest_cemented_level : t -> int32 option val get_cemented_block_by_level : t -> read_metadata:bool -> int32 -> Block_repr.block option tzresult Lwt.t -(** [get_cemented_block_by_hash cemented_store hash] reads the cemented - block of [hash] in [cemented_store], if it exists. It also - retrieves the metadata depending on [read_metadata]. *) +(** [get_cemented_block_by_hash ~read_metadata cemented_store hash] + reads the cemented block of [hash] in [cemented_store], if it + exists. It also retrieves the metadata depending on + [~read_metadata]. *) val get_cemented_block_by_hash : read_metadata:bool -> t -> @@ -281,12 +282,11 @@ val cement_blocks : the higher the offset is, the longest the GC phase will last. *) val trigger_gc : t -> History_mode.t -> unit Lwt.t -(** [iter_cemented_file ~cemented_block_dir f block_file] reads from - the cemented [block_file] located in [cemented_block_dir] and - applies [f] on every block. +(** [iter_cemented_file f block_file] reads from the cemented + [block_file] and applies [f] on every block. - {b Warning}: in this version, exceptions are caught. Use [raw_iter_cemented_file] - for manual exception management. *) + {b Warning}: in this version, exceptions are caught. Use + [raw_iter_cemented_file] for manual exception management. *) val iter_cemented_file : (Block_repr.block -> unit Lwt.t) -> cemented_blocks_file -> @@ -297,11 +297,11 @@ val iter_cemented_file : val raw_iter_cemented_file : (Block_repr.block -> unit Lwt.t) -> cemented_blocks_file -> unit Lwt.t -(** [check_indexes_consistency ?post_step ?genesis_hash cemented_store - history_mode] iterates over a partially initialized +(** [check_indexes_consistency ?post_step ?genesis_hash + cemented_store] iterates over a partially initialized [cemented_store] that contains both chunks of blocks and indexes - then check the consistency of each block: (hashes, predecessors and - levels). The hash is not checked for [genesis_hash] and + then check the consistency of each block: (hashes, predecessors + and levels). The hash is not checked for [genesis_hash] and [post_step] is called after each treated chunk. This is used for snapshot imports. *) val check_indexes_consistency : -- GitLab From 9ca1d2e0fed29e676b67865240296d534b5031d1 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Thu, 7 Nov 2024 17:00:51 +0100 Subject: [PATCH 3/3] Store/consistency: minor code/doc improvements --- src/lib_store/unix/consistency.ml | 38 ++++++++++++++---------------- src/lib_store/unix/consistency.mli | 8 +++---- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/lib_store/unix/consistency.ml b/src/lib_store/unix/consistency.ml index d1e63dc5e9d0..77a92806db8c 100644 --- a/src/lib_store/unix/consistency.ml +++ b/src/lib_store/unix/consistency.ml @@ -29,9 +29,10 @@ open Store_errors (* A non-empty store is considered consistent if the following invariants hold: - - genesis, caboose, savepoint, checkpoint, current_head associated - files exists, are decodable and the blocks they point to may be - read in the block store and are consistent with their definition; + - genesis, caboose, savepoint, checkpoint and current_head + associated files exists, are decodable and the blocks they are + pointing to can be read in the block store and are consistent with + their definition; - genesis ≤ caboose ≤ savepoint ≤ [cementing_highwatermark] ≤ checkpoint ≤ current_head @@ -42,9 +43,8 @@ open Store_errors store. *) -(* [check_cementing_highwatermark ~chain_dir block_store] checks that - the cementing_highwatermark is consistent with the cemented - store. *) +(* Checks that [cementing_highwatermark] is consistent with the + cemented store. *) let check_cementing_highwatermark ~cementing_highwatermark block_store = let open Lwt_result_syntax in let cemented_store = Block_store.cemented_block_store block_store in @@ -160,9 +160,9 @@ let create_lockfile path chain_dir = in return_ok fd) -(* [check_consistency ~store_dir genesis] aims to provide a quick - check (in terms of execution time) which checks that files may be - read and they are consistent w.r.t to the given invariant. +(* [check_consistency chain_dir genesis] aims to provide a quick check + (in terms of execution time) which checks that files can be read + and they are consistent w.r.t to the given invariant. Hypothesis: an existing store is provided. *) let check_consistency chain_dir genesis = @@ -207,19 +207,18 @@ let check_consistency chain_dir genesis = Stored_data.load (Naming.forked_chains_file chain_dir) in let* _target_data = Stored_data.load (Naming.target_file chain_dir) in - (* Open the store and try to read the blocks *) - (* [~readonly:false] to recover from a potential interrupted merge *) + (* Open the store and try to read the blocks. [readonly] is set to + false to avoid the Block_store.load to automatically recover from + a potential interrupted merge. *) let* block_store = Block_store.load chain_dir ~genesis_block ~readonly:true in Lwt.finalize (fun () -> - (* TODO should we check context as well? *) let genesis_descr = Block_repr.descriptor genesis_block in let expected_blocks = [ (genesis_descr, false, "genesis"); (caboose, false, "caboose"); (savepoint, true, "savepoint"); - (* is this really true? *) (checkpoint, true, "checkpoint"); (current_head, true, "current_head"); ] @@ -265,6 +264,10 @@ let check_consistency chain_dir genesis = let*! () = Lwt_unix.close lockfile_fd in Block_store.close block_store) +(* [fix_floating_stores chain_dir] fixes the floating store by + removing the potential leftover from an interrupted store + merge. In addition to that, it fixes the integrity of the existing + floating stores. *) let fix_floating_stores chain_dir = let open Lwt_result_syntax in let store_kinds = [Floating_block_store.RO; RW; RW_TMP; RO_TMP] in @@ -347,10 +350,6 @@ let fix_head chain_dir block_store genesis_block = highest_cemented_level in WithExceptions.Option.get ~loc:__LOC__ o - (* If the highest of the floating blocks is genesis and there is - at least one cemented file, then it means that the floating - blocks were truncated. The head is then chosen as the highest - cemented block known. *) else return floating_head in (* Make sure that the inferred head have metadata *) @@ -383,9 +382,8 @@ let fix_head chain_dir block_store genesis_block = in return inferred_head -(* Search for the lowest block with metadata (for savepoint) and the - lowest block (for caboose) from the cemented store. - We assume that the given [cemented_block_files] list is sorted in +(* Returns the lowest block of the given cemented cycle. As a + store invariant, the cemented cycle is expected to be sorted in an ascending order (lowest block files comes first). *) let lowest_cemented_block cemented_block_files = match cemented_block_files with diff --git a/src/lib_store/unix/consistency.mli b/src/lib_store/unix/consistency.mli index 02f205f1864b..bfba05dfe308 100644 --- a/src/lib_store/unix/consistency.mli +++ b/src/lib_store/unix/consistency.mli @@ -23,14 +23,14 @@ (* *) (*****************************************************************************) -(** [check_consistency store_dir genesis] checks that +(** [check_consistency chain_dir genesis] checks that the store is in a consistent state. *) val check_consistency : [`Chain_dir] Naming.directory -> Genesis.t -> unit tzresult Lwt.t -(** [fix_consistency ?history_mode store_dir context_index genesis] - aims to fix a store if it is in an inconsistent state. To do so, it - will inspect the stored data and try infer all the missing or +(** [fix_consistency ?history_mode chain_dir context_index genesis] + aims to fix a store if it is in an inconsistent state. To do so, + it will inspect the stored data and try infer all the missing or corrupted parts. *) val fix_consistency : ?history_mode:History_mode.t -> -- GitLab