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: add lower_root config for lower layer #2177

Merged
merged 4 commits into from
Jan 31, 2023

Conversation

metanivek
Copy link
Member

This PR adds the config key for the upcoming lower layer, with validation during Repo.v.

I didn't want to add a new error message just for an invalid lower_root value, and I also wanted good log message if the error occurs. To accomplish both, I am reusing No_such_file_or_directory and have added a string value for the file path. I think this also improves this error generally.

Copy link
Member

@Firobe Firobe left a comment

Choose a reason for hiding this comment

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

Apart my only comment, the changes looks good to little me!

test/irmin-pack/common.ml Outdated Show resolved Hide resolved
@metanivek metanivek force-pushed the lower_layer_config branch 2 times, most recently from 4348639 to 99274c0 Compare January 25, 2023 19:17
@codecov-commenter
Copy link

Codecov Report

Merging #2177 (d56b0ae) into main (ff45092) will decrease coverage by 0.01%.
The diff coverage is 50.00%.

❗ Current head d56b0ae differs from pull request most recent head 99274c0. Consider uploading reports for the commit 99274c0 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    #2177      +/-   ##
==========================================
- Coverage   68.07%   68.07%   -0.01%     
==========================================
  Files         134      134              
  Lines       16161    16173      +12     
==========================================
+ Hits        11001    11009       +8     
- Misses       5160     5164       +4     
Impacted Files Coverage Δ
src/irmin-pack/inode.ml 78.96% <0.00%> (ø)
src/irmin-pack/unix/control_file_intf.ml 100.00% <ø> (ø)
src/irmin-pack/unix/errors.ml 28.88% <ø> (ø)
src/irmin-pack/unix/io_errors.ml 58.33% <ø> (ø)
src/irmin-pack/unix/mapping_file.ml 89.88% <0.00%> (ø)
src/irmin-pack/unix/store.ml 63.26% <37.50%> (-0.85%) ⬇️
src/irmin-pack/unix/async.ml 56.60% <40.00%> (+2.75%) ⬆️
src/irmin-pack/unix/file_manager.ml 84.89% <50.00%> (ø)
src/irmin-pack/conf.ml 87.50% <100.00%> (+0.96%) ⬆️
src/irmin-pack/unix/io.ml 65.21% <100.00%> (ø)
... and 4 more

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

@@ -38,7 +38,7 @@ type base_error =
[ `Double_close
| `File_exists of string
| `Invalid_parent_directory
| `No_such_file_or_directory
| `No_such_file_or_directory of string
Copy link
Contributor

Choose a reason for hiding this comment

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

This is great, thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd like to take a look at all of our errors to decide which ones really deserve some extra context info. But, for a future PR. 😄

@metanivek metanivek merged commit b6eb462 into mirage:main Jan 31, 2023
@metanivek metanivek deleted the lower_layer_config branch January 31, 2023 14:16
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

4 participants