mirage / irmin

Irmin is a distributed database that follows the same design principles as Git
https://irmin.org
ISC License
1.85k stars 157 forks source link

irmin-pack: introduce Sparse_file abstraction #2172

Closed art-w closed 1 year ago

art-w commented 1 year ago

This PR tries to abstract the mapping/data files coupling behind a Sparse_file module (that we could use to implement the lower volumes?). It's a pretty aggressive refactoring so I would understand if it is not welcomed! It notably gets rid of the dispatcher's accessors because I find it easier to hide the offset computations inside the Sparse/Chunked files blackboxes.

If there's no strong objection, my plan is to add some append-only functions to this Sparse_file that the GC will be able to use to transfer data to the lower. There's also still some refactoring to do in File_manager as it could delegate the opening/closing/etc of the mapping/data file to the new module.

codecov-commenter commented 1 year ago

Codecov Report

Merging #2172 (dd6f3f0) into main (ff45092) will decrease coverage by 0.09%. The diff coverage is 76.15%.

:exclamation: Current head dd6f3f0 differs from pull request most recent head a5ad27c. Consider uploading reports for the commit a5ad27c to get more accurate results

:mega: 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    #2172      +/-   ##
==========================================
- Coverage   68.07%   67.98%   -0.09%     
==========================================
  Files         134      134              
  Lines       16161    16107      -54     
==========================================
- Hits        11001    10950      -51     
+ Misses       5160     5157       -3     
Impacted Files Coverage Δ
src/irmin-pack/inode.ml 78.96% <0.00%> (ø)
src/irmin-pack/unix/gc_worker.ml 4.41% <0.00%> (+0.06%) :arrow_up:
src/irmin-pack/unix/import.ml 87.50% <ø> (+5.14%) :arrow_up:
src/irmin-pack/unix/store.ml 64.11% <ø> (ø)
src/irmin-pack/version.ml 44.44% <ø> (ø)
src/irmin-pack/unix/async.ml 56.60% <40.00%> (+2.75%) :arrow_up:
src/irmin-pack/unix/dispatcher.ml 58.73% <52.50%> (-17.30%) :arrow_down:
src/irmin-pack/unix/chunked_suffix.ml 85.58% <60.00%> (-0.27%) :arrow_down:
src/irmin-pack/unix/sparse_file.ml 82.92% <82.92%> (ø)
src/irmin-pack/unix/pack_store.ml 82.72% <83.33%> (ø)
... and 15 more

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

art-w commented 1 year ago

Thanks for the review! I've made some changes (to be squashed after approval) and simplified a bit the file manager (... I'm trying to expose less and less of the Mapping_file, although the GC worker still need this internal access until the sparse file provides "append" operations)