irmin-pack: fix v2 lower migration #2241
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #2241 +/- ##
==========================================
+ Coverage 67.93% 67.96% +0.02%
==========================================
Files 136 136
Lines 16603 16621 +18
==========================================
+ Hits 11279 11296 +17
- Misses 5324 5325 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
metanivek
changed the title
Previously we did not take the dead header of the dict into account when performing a lower layer migration for older stores. This commit fixes this issue by removing the dead header from the dict during the migration.
These stores migrate fine (these tests did not fail) but adding them for completeness as we add future stores. Also added a line that reads all contents in the store to ensure data moved correctly during the migration.
metanivek
force-pushed
the
fix_v2_migration
branch
from
a8dbc82
to
fffa054
Compare
art-w
approved these changes
May 3, 2023
There was a problem hiding this comment.
Tested and it works great, thanks!
edit: the CI reports a GC crash with flambda though? https://ocaml.ci.dev/github/mirage/irmin/commit/fffa054dc2de6acb750bde44d085dac1448564d3/variant/debian-11-4.14%2Bflambda_opam-2.1 (I'm relaunching it to see if it's flaky, copied the previous crash report below)
┌──────────────────────────────────────────────────────────────────────────────┐
│ [FAIL] upgrade 0 upgrade From_v3. │
└──────────────────────────────────────────────────────────────────────────────┘
...
+24066583us src/irmin-pack/unix/dict.ml:78 [DEBUG] [dict] find 0
+24066658us src/irmin-pack/unix/pack_store.ml:274 [DEBUG] key_of_offset: 164
+24066726us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:164 ~min_len:21 ~max_len:30
+24066797us src/irmin-pack/unix/pack_store.ml:274 [DEBUG] key_of_offset: 122
+24066856us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:122 ~min_len:21 ~max_len:30
+24066917us src/irmin-pack/unix/pack_store.ml:274 [DEBUG] key_of_offset: 54
+24066971us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:54 ~min_len:21 ~max_len:30
+24068851us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:54 ~min_len:34 ~max_len:34
+24068939us src/irmin-pack/unix/dict.ml:78 [DEBUG] [dict] find 0
+24068991us src/irmin-pack/unix/pack_store.ml:274 [DEBUG] key_of_offset: 29
+24069046us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:29 ~min_len:21 ~max_len:30
+24069848us src/irmin-pack/unix/gc_worker.ml:340 [DEBUG] GC: transfering to the new prefix
+24070334us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:29 ~min_len:1 ~max_len:59
+24070398us src/irmin-pack/unix/dispatcher.ml:90 [DEBUG] read_range_exn ~off:122 ~min_len:1 ~max_len:161
+24071143us index [DEBUG] [test-upgrade] close
+24071194us index [DEBUG] [test-upgrade] last open instance: closing the file descriptor
+24088199us src/irmin-pack/unix/file_manager.ml:291 [DEBUG] Remove residual file _build/test-upgrade/store.1.mapping
+24134027us src/irmin-pack/unix/file_manager.ml:291 [DEBUG] Remove residual file _build/test-upgrade/store.1.prefix
[exception] Pack_error: {"Gc_process_died_without_result_file":"success {\"No_such_file_or_directory\":\"_build/test-upgrade/store.1.out\"}"}
Raised at Irmin_pack_unix__Errors.Base.raise_error in file "src/irmin-pack/unix/errors.ml", line 123, characters 26-46
Called from Lwt.Sequential_composition.bind.create_result_promise_and_callback_if_deferred.callback in file "src/core/lwt.ml", line 1849, characters 23-26
Logs saved to `/src/_build/default/test/irmin-pack/_build/_tests/irmin-pack/upgrade.000.output'.
──────────────────────────────────────────────────────────────────────────────
Full test results in `/src/_build/default/test/irmin-pack/_build/_tests/irmin-pack'.
1 failure! in 134.178s. 239 tests run.
...
...
"/bin/bash" "-c" "opam exec -- dune build @install @check @runtest && rm -rf _build" failed with exit status 1
2023-05-02 18:31.32: Job failed: Failed: Build failed
2023-05-02 18:31.32: Log analysis:
2023-05-02 18:31.32: >>> [FAIL] upgrade 0 upgrade From_v3. (score = 30)
2023-05-02 18:31.32: >>> [FAIL] upgrade 0 upgrade From_v3. │ (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> Unix_error(Unix.ENOENT, "unlink", "_build/test-upgrade/store.1.out") (score = 30)
2023-05-02 18:31.32: >>> [exception] Pack_error: {"Gc_process_died_without_result_file":"success {\"No_such_file_or_directory\":\"_build/test-upgrade/store.1.out\"}"} (score = 35)
2023-05-02 18:31.32: Exception: Pack_error: {"Gc_process_died_without_result_file":"success {\"No_such_file_or_directory\":\"_build/test-upgrade/store.1.out\"}"}
metanivek
added a commit
to metanivek/opam-repository
that referenced
this pull request
May 3, 2023
…min-pack, irmin-pack-tools, irmin-mirage, irmin-mirage-graphql, irmin-mirage-git, irmin-http, irmin-graphql, irmin-git, irmin-fs, irmin-containers, irmin-cli, irmin-chunk and irmin-bench (3.7.1) CHANGES: ### Fixed - **irmin-pack** - Fix issue when migrating v2 stores to use lower layer (@metanivek, mirage/irmin#2241) - Fix issue when calling GC for a commit in the lower after migration (@metanivek, mirage/irmin#2242)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously we did not take the dead header of the dict into account when performing a lower layer migration for older stores. This commit fixes this issue by removing the dead header from the dict during the migration.
I also added some tests for v3 stores and updated migration tests to ensuring reading contents after a migration works. These tests all passed anyway, but thought it didn't hurt for completeness against potential future changes.