irmin-pack-tool: Add a tool for last indexed accessible commits #2218
Conversation
| @@ -95,5 +96,37 @@ However, it might be useful to sort the json output using the offset. You can do | |||
| $ jq -s 'sort_by(.off)' -- index | |||
| ``` | |||
|
|
|||
| ## last-valid | |||
There was a problem hiding this comment.
| ## last-valid | |
| ## last-accessible |
| let main_cmd = | ||
| let doc = | ||
| "gives the last accessible commit informations stored in the index of an \ | ||
| upper store" |
There was a problem hiding this comment.
| upper store" | |
| store" |
| Arg.( | ||
| required | ||
| & pos 0 (some string) None | ||
| & info [] ~docv:"root folder" ~doc:"the path to the store") |
There was a problem hiding this comment.
| & info [] ~docv:"root folder" ~doc:"the path to the store") | |
| & info [] ~docv:"root folder" ~doc:"the path to the store root") |
| ``` | ||
|
|
||
| Note that this tool makes several assumptions about the state of the store: | ||
| - The store is an upper store. |
There was a problem hiding this comment.
| - The store is an upper store. | |
| - The path given to <path-to-store> is the store's root. |
Conceptually, the upper and (optional) lower layer are the store. It's a nit, but perhaps this point is that the path provided in <path-to-store> is the config root, not lower_root. I also make some suggestions below to attempt to make this consistent.
There was a problem hiding this comment.
Agreed, thx for the clarification
| @@ -0,0 +1,8 @@ | |||
| (executable | |||
| (public_name irmin-last_accessible) | |||
There was a problem hiding this comment.
Nit: I think it would look better to not mix dash and underscore 🥺
| let r = Upper_control.read_payload ~path:control_file in | ||
| match r with Error err -> Io_errors.raise_error err | Ok payload -> payload | ||
|
|
||
| let get_suffixes_sizes root |
There was a problem hiding this comment.
| let get_suffixes_sizes root | |
| let get_suffix_size root |
Since you're only returning a single size from this function.
| let r = ref 0 in | ||
| for i = payload.chunk_start_idx to payload.chunk_num do | ||
| let suffix = Irmin_pack.Layout.V5.suffix_chunk ~chunk_idx:i ~root in | ||
| let stats = Unix.stat suffix in |
There was a problem hiding this comment.
I'm not sure it matters too much but it might be nice to use the Io.size_of_path function.
| let payload = get_payload root_folder in | ||
| let sizes = get_suffixes_sizes root_folder payload in | ||
| let start_offset, dead_bytes = get_status_infos payload in | ||
| let off_max = sizes + start_offset - dead_bytes in |
There was a problem hiding this comment.
To be fully backwards compatible, this needs to take into account the dead header size (see this function) by subtracting it.
There was a problem hiding this comment.
Thx, I thought that maybe the From_v1_v2_post_upgrade would need something specific, but I didnt know what and was waiting for tests to tell me what !
| let r = ref { hash = "n/a"; off = Int63.zero; len = 0; kind = "n/a" } in | ||
| let off_max = Int63.of_int off_max in | ||
| let f k v = | ||
| let hash = Base64.encode_exn (Index.Key.encode k) in |
There was a problem hiding this comment.
I think we can use something like the following (haven't checked it compiles 😅) to give a string representation (since index's Key.t derives repr) that doesn't use base64.
| let hash = Base64.encode_exn (Index.Key.encode k) in | |
| let hash = (Irmin.Type.to_string Index.Key.t) k in |
|
Thanks! Do you mind addressing @metanivek comments and running |
This PR aims to help restoring a store to a previous state when a corruption occurs.
It uses the
indexof the store in order to find the latest commit indexed which is available in the suffix files.It should become useful in the case of a control file being updated, but the suffixes staying behind (i.e. power outage).
There might be some work to do in order to ensure that we do it correctly, but a good direction we discussed @art-w and I for this PR would be to re-generate a working
control filewhich a user could use in order to avoid deleting and reinstalling it's store.We might however have some issues with data registered in the
indexand such, but we still wanted to create a draft PR to open the discussion.One last point is that this PR currently makes several assumptions based on the control file and it's validity, and we might want to create additional checks which do not use it.