Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

irmin-pack: fast migration to lower by copy #2205

Merged
merged 1 commit into from
Mar 3, 2023

Conversation

art-w
Copy link
Contributor

@art-w art-w commented Mar 1, 2023

(builds on top of #2204)

A draft for the fast migration when enabling lower volumes from an existing store (which only had an upper suffix file, never garbage collected or chunk splitted). I went with a file copy rather than a move when initializing the lower volume to avoid any surprise if the operation fails midway... it should be as efficient as a rename when happening on the same hard drive anyway :) But the code is still missing some way of recovering and cleaning up the drive if the migration fails!

src/irmin-pack/unix/file_manager.ml Outdated Show resolved Hide resolved
src/irmin-pack/unix/file_manager.ml Outdated Show resolved Hide resolved
src/irmin-pack/unix/file_manager.ml Show resolved Hide resolved
src/irmin-pack/unix/file_manager.ml Outdated Show resolved Hide resolved
test/irmin-pack/test_lower.ml Show resolved Hide resolved
src/irmin-pack/unix/lower.ml Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

Merging #2205 (15e299e) into main (101dccf) will increase coverage by 0.05%.
The diff coverage is 76.36%.

❗ Current head 15e299e differs from pull request most recent head d67aaec. Consider uploading reports for the commit d67aaec to get more accurate results

📣 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    #2205      +/-   ##
==========================================
+ Coverage   67.95%   68.01%   +0.05%     
==========================================
  Files         135      135              
  Lines       16435    16472      +37     
==========================================
+ Hits        11168    11203      +35     
- Misses       5267     5269       +2     
Impacted Files Coverage Δ
src/irmin-pack/unix/control_file_intf.ml 82.14% <ø> (ø)
src/irmin-pack/unix/errors.ml 28.88% <ø> (ø)
src/irmin-pack/unix/io_errors.ml 58.33% <ø> (ø)
src/irmin-pack/unix/file_manager.ml 84.67% <75.75%> (-0.61%) ⬇️
src/irmin-pack/unix/lower.ml 63.69% <77.27%> (+5.48%) ⬆️
src/irmin-fs/unix/irmin_fs_unix.ml 67.74% <0.00%> (-0.65%) ⬇️
src/irmin-test/store.ml 94.89% <0.00%> (-0.06%) ⬇️
src/irmin-pack/unix/sparse_file.ml 77.98% <0.00%> (+1.83%) ⬆️
src/irmin-pack/unix/utils.ml 93.10% <0.00%> (+3.44%) ⬆️

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

@art-w art-w force-pushed the migrate-lower branch 2 times, most recently from 91b2601 to 9b84e35 Compare March 3, 2023 15:59
Copy link
Member

@metanivek metanivek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 🎉 Just had one question about a thing that popped out to me.

src/irmin-pack/unix/lower.ml Outdated Show resolved Hide resolved
@metanivek metanivek merged commit ce09bb7 into mirage:main Mar 3, 2023
metanivek added a commit to metanivek/opam-repository that referenced this pull request Apr 21, 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.0)

CHANGES:

### Added

- **irmin**
  - Add `Conf.pp` and `Conf.equal` to print and compare configuration values
    (mirage/irmin#2227, @samoht)
  - Add a `clear` optional arguments to all function that adds a new commit:
    `Commit.v`, `set`, `set_tree`, `remove`, `test_and_set`,
    `test_and_set_tree`, `test_set_and_get`, `test_set_and_get_tree`, `merge`,
    `merge_tree` and `with_tree`. This new argument allows to control whether
    the tree caches are cleared up after objects are exported to disk during
    the commit. (mirage/irmin#2225, @samoht)

- **irmin-pack**
  - Add configuration option, `lower_root`, to specify a path for archiving data
    during a GC. (mirage/irmin#2177, @metanivek)
  - Add `is_split_allowed` to check if a store allows split. (mirage/irmin#2175, @metanivek)
  - Add `add_volume` to allow creating new empty volume in lower layer. (mirage/irmin#2188,
    @metanivek)
  - Add a `behaviour` function to the GC to check wether the GC will archive or
    delete data. (mirage/irmin#2190, @Firobe)
  - Add a migration on `open_rw` to move the data to the `lower_root` if
    the configuration was enabled (mirage/irmin#2205, @art-w)

### Changed

- **irmin**
  - Expose type equality for `Schema.Info` to avoid defining the `info` function
    multiple times when using similar stores (mirage/irmin#2189, mirage/irmin#2193, @samoht)
- **irmin-pack**
  - GC now changes its behaviour depending on the presence of a lower layer.
    (mirage/irmin#2190, @Firobe)
  - Split now raises an exception if it is not allowed. It is not allowed on
    stores that do not allow GC. (mirage/irmin#2175, @metanivek)
  - GC now supports stores imported V1/V2 stores, in presence of a lower layer
    only. (mirage/irmin#2190, @art-w, @Firobe)
  - Upgrade on-disk format to version 5. (mirage/irmin#2184, @metanivek)
  - Archive to lower volume does not copy orphaned commits. (mirage/irmin#2215, @art-w)

### Fixed
- **irmin-pack**
  - Unhandled exceptions in GC worker process are now reported as a failure
    (mirage/irmin#2163, @metanivek)
  - Fix the silent mode for the integrity checks. (mirage/irmin#2179, @icristescu)
  - Fix file descriptor leak caused by `mmap`. (mirage/irmin#2232, @art-w)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants