mirage / irmin

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

irmin-pack: remove append-only external flush callback #2235

Closed art-w closed 1 year ago

art-w commented 1 year ago

I was trying to simplify the logic to update the control file in the file manager. The Append_only_file was using an external callback to signify the need to flush when its buffer was full, which would in turn update the control file (to make that portion of the file visible)... but then we could end up with a visible partially written commit, which isn't an issue in itself, but isn't how we think about the control file payload. As the flush threshold was fairly high, I don't think this "partial flush" was really stress tested either (?)

Anyway it's gone, as it should be fine to just write to the append-only files when their memory buffer is full, we only need to update the control file at the end to publish the writes.

While testing I discovered that I broke the Dict flushing its end_poff to the control file, so I refactored that a bit to match with the chunks/sparse file abstractions (inverting the File_manager dependency). A few breaking changes here:

codecov-commenter commented 1 year ago

Codecov Report

Merging #2235 (04e1790) into main (e6cb805) will decrease coverage by 0.01%. The diff coverage is 93.24%.

:exclamation: Current head 04e1790 differs from pull request most recent head 02fd19f. Consider uploading reports for the commit 02fd19f to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main    #2235      +/-   ##
==========================================
- Coverage   68.17%   68.17%   -0.01%     
==========================================
  Files         138      138              
  Lines       16739    16733       -6     
==========================================
- Hits        11412    11407       -5     
+ Misses       5327     5326       -1     
Files Changed Coverage Δ
src/irmin-pack/unix/gc_worker.ml 3.75% <0.00%> (ø)
src/irmin-pack/unix/pack_store.ml 78.84% <ø> (ø)
src/irmin-pack/unix/store.ml 65.55% <ø> (-0.23%) :arrow_down:
src/irmin-pack/unix/file_manager.ml 86.93% <91.17%> (+1.15%) :arrow_up:
src/irmin-pack/unix/append_only_file.ml 87.30% <93.75%> (+2.30%) :arrow_up:
src/irmin-pack/conf.ml 86.79% <100.00%> (-1.74%) :arrow_down:
src/irmin-pack/unix/chunked_suffix.ml 85.96% <100.00%> (-0.13%) :arrow_down:
src/irmin-pack/unix/dict.ml 95.65% <100.00%> (+1.20%) :arrow_up:
src/irmin-pack/unix/sparse_file.ml 84.49% <100.00%> (ø)

... and 3 files with indirect coverage changes

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