From 7830b81c2a86ad7236bb5173c741b3ebbf4bc53f Mon Sep 17 00:00:00 2001 From: metanivek Date: Wed, 2 Nov 2022 14:03:41 -0400 Subject: [PATCH] fixup! irmin-pack: update GC for chunked suffix --- src/irmin-pack/unix/dispatcher.ml | 6 +++--- src/irmin-pack/unix/gc.ml | 13 +++++++++---- src/irmin-pack/unix/gc_worker.ml | 10 +++++----- src/irmin-pack/unix/gc_worker.mli | 4 ++-- test/irmin-pack/test_gc.ml | 20 ++++++++++---------- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/src/irmin-pack/unix/dispatcher.ml b/src/irmin-pack/unix/dispatcher.ml index f34442f03bc..510fe8e367d 100644 --- a/src/irmin-pack/unix/dispatcher.ml +++ b/src/irmin-pack/unix/dispatcher.ml @@ -85,18 +85,18 @@ module Make (Fm : File_manager.S with module Io = Io.Unix) : module Suffix_arithmetic = struct (* Adjust the read in suffix, as the global offset [off] is - [off] = [suffix_start_offset] + [suffix_offset]. *) + [off] = [suffix_start_offset] + [soff] - [suffix_dead_bytes]. *) let soff_of_off t off = let open Int63.Syntax in let suffix_start_offset = suffix_start_offset t in let suffix_dead_bytes = suffix_dead_bytes t in off - suffix_start_offset + suffix_dead_bytes - let off_of_soff t suffix_off = + let off_of_soff t soff = let open Int63.Syntax in let suffix_start_offset = suffix_start_offset t in let suffix_dead_bytes = suffix_dead_bytes t in - suffix_off - suffix_dead_bytes + suffix_start_offset + suffix_start_offset + soff - suffix_dead_bytes end let offset_of_soff = Suffix_arithmetic.off_of_soff diff --git a/src/irmin-pack/unix/gc.ml b/src/irmin-pack/unix/gc.ml index c6b710e23d3..8445fd275e0 100644 --- a/src/irmin-pack/unix/gc.ml +++ b/src/irmin-pack/unix/gc.ml @@ -108,7 +108,7 @@ module Make (Args : Gc_args.S) = struct latest_gc_target_offset; } - let swap_and_purge t removable_chunk_num suffix = + let swap_and_purge t removable_chunk_num suffix_params = let open Result_syntax in let { generation; latest_gc_target_offset; _ } = t in let Worker. @@ -117,13 +117,15 @@ module Make (Args : Gc_args.S) = struct chunk_start_idx; dead_bytes = suffix_dead_bytes; } = - suffix + suffix_params in (* Calculate chunk num in main process since more chunks could have been added while GC was running. GC process only tells us how many chunks are to be removed. *) let suffix = Fm.suffix t.fm in let chunk_num = Fm.Suffix.chunk_num suffix - removable_chunk_num in + (* Assert that we have at least one chunk (the appendable chunk), which + is guaranteed by the GC process. *) assert (chunk_num >= 1); let* () = @@ -233,13 +235,16 @@ module Make (Args : Gc_args.S) = struct let open Result_syntax in match (status, gc_output) with | ( `Success, - Ok { suffix; removable_chunk_idxs; stats = worker_stats } ) -> + Ok { suffix_params; removable_chunk_idxs; stats = worker_stats } + ) -> let partial_stats = Gc_stats.Main.finish_current_step partial_stats "swap and purge" in let* () = - swap_and_purge t (List.length removable_chunk_idxs) suffix + swap_and_purge t + (List.length removable_chunk_idxs) + suffix_params in let partial_stats = Gc_stats.Main.finish_current_step partial_stats "unlink" diff --git a/src/irmin-pack/unix/gc_worker.ml b/src/irmin-pack/unix/gc_worker.ml index 78453645e71..6d1c6c4cd58 100644 --- a/src/irmin-pack/unix/gc_worker.ml +++ b/src/irmin-pack/unix/gc_worker.ml @@ -118,7 +118,7 @@ module Make (Args : Gc_args.S) = struct stats := Gc_stats.Worker.add_file_size !stats "old_prefix" prefix_size; stats := Gc_stats.Worker.add_file_size !stats "old_mapping" mapping_size - type new_suffix = { + type suffix_params = { start_offset : int63; chunk_start_idx : int; dead_bytes : int63; @@ -126,7 +126,7 @@ module Make (Args : Gc_args.S) = struct [@@deriving irmin] type gc_results = { - suffix : new_suffix; + suffix_params : suffix_params; removable_chunk_idxs : int list; stats : Stats.Latest_gc.worker; } @@ -277,8 +277,8 @@ module Make (Args : Gc_args.S) = struct (Commit_value.parents commit) in - (* Step 6. Calculate new suffix values. *) - let new_suffix, removable_chunk_idxs = + (* Step 6. Calculate post-GC suffix parameters. *) + let suffix_params, removable_chunk_idxs = stats := Gc_stats.Worker.finish_current_step !stats "suffix: calculate new values"; @@ -344,7 +344,7 @@ module Make (Args : Gc_args.S) = struct (* Step 7. Finalise stats and return. *) let stats = Gc_stats.Worker.finalise !stats in - { suffix = new_suffix; removable_chunk_idxs; stats } + { suffix_params; removable_chunk_idxs; stats } let write_gc_output ~root ~generation output = let open Result_syntax in diff --git a/src/irmin-pack/unix/gc_worker.mli b/src/irmin-pack/unix/gc_worker.mli index 97b1d515435..d7e359502c7 100644 --- a/src/irmin-pack/unix/gc_worker.mli +++ b/src/irmin-pack/unix/gc_worker.mli @@ -25,7 +25,7 @@ module Make (Args : Gc_args.S) : sig val run_and_output_result : generation:int -> string -> Args.key -> int63 -> unit - type new_suffix = { + type suffix_params = { start_offset : int63; chunk_start_idx : int; dead_bytes : int63; @@ -33,7 +33,7 @@ module Make (Args : Gc_args.S) : sig [@@deriving irmin] type gc_results = { - suffix : new_suffix; + suffix_params : suffix_params; removable_chunk_idxs : int list; stats : Stats.Latest_gc.worker; } diff --git a/test/irmin-pack/test_gc.ml b/test/irmin-pack/test_gc.ml index 369979d5ada..1547cc196c2 100644 --- a/test/irmin-pack/test_gc.ml +++ b/test/irmin-pack/test_gc.ml @@ -923,20 +923,20 @@ module Split = struct let* () = S.Repo.close t.repo in S.Repo.close ro_t.repo - let c t s = - let h = - match Irmin.Type.(of_string Schema.Hash.t) s with + let load_commit t h = + let hash = + match Irmin.Type.(of_string Schema.Hash.t) h with | Error (`Msg s) -> Alcotest.failf "failed hash_of_string %s" s - | Ok h -> h + | Ok hash -> hash in - let+ c = S.Commit.of_hash t.repo h in - match c with - | None -> Alcotest.failf "Commit %s not found" s + let+ commit = S.Commit.of_hash t.repo hash in + match commit with + | None -> Alcotest.failf "Commit %s not found" h | Some commit -> commit let check_preexisting_commit t = - let s = "22e159de13b427226e5901defd17f0c14e744205" in - let* commit = c t s in + let h = "22e159de13b427226e5901defd17f0c14e744205" in + let* commit = load_commit t h in let tree = S.Commit.tree commit in let+ got = S.Tree.find tree [ "step-n01"; "step-b01" ] in Alcotest.(check (option string)) "find blob" (Some "b01") got @@ -944,7 +944,7 @@ module Split = struct let v3_migrated_store_splits_and_gc () = let root = create_test_env () in let* t = init ~readonly:false ~fresh:false ~root () in - let* c0 = c t "22e159de13b427226e5901defd17f0c14e744205" in + let* c0 = load_commit t "22e159de13b427226e5901defd17f0c14e744205" in let* t, c1 = commit_1 t in let () = S.split t.repo in let* t = checkout_exn t c1 in