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: allow to keep tree caches after a commit #2225

Merged
merged 2 commits into from Mar 28, 2023

Conversation

samoht
Copy link
Member

@samoht samoht commented Mar 24, 2023

Expose the clear flag threaded through to Tree.export.

With this, usingS.set_tree_exn ~clear:fase in https://gitlab.com/tezos/tezos/-/merge_requests/8169 seems to fix the issue.

It's also unclear what the default should be - There's an explicit Tree.clear for those who really want to clear the caches, so maybe by default, we should have clear=false?

@samoht samoht changed the title irmin: allow to keep tree cache after a commit irmin: allow to keep tree caches after a commit Mar 24, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #2225 (d6644ee) into main (8eeffae) will decrease coverage by 0.01%.
The diff coverage is 66.66%.

❗ Current head d6644ee differs from pull request most recent head 9d04fc6. Consider uploading reports for the commit 9d04fc6 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    #2225      +/-   ##
==========================================
- Coverage   68.09%   68.09%   -0.01%     
==========================================
  Files         135      135              
  Lines       16480    16484       +4     
==========================================
+ Hits        11222    11224       +2     
- Misses       5258     5260       +2     
Impacted Files Coverage Δ
src/irmin/store.ml 66.23% <65.71%> (+<0.01%) ⬆️
src/irmin/tree.ml 81.07% <100.00%> (-0.21%) ⬇️

... and 2 files with indirect coverage changes

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

@samoht
Copy link
Member Author

samoht commented Mar 27, 2023

I've added some documentation and tests. It needs #2227 to be able to compare repository configurations in a finer grain as before (as previously we would always cleanup these fields after an export)

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.

Changes look good. I don't have a strong opinion on changing the default behavior. Exposing it at least now gives choice to a client that they did not have before.

src/irmin/store_intf.ml Outdated Show resolved Hide resolved
@samoht samoht force-pushed the expose-clear branch 2 times, most recently from 1858792 to 73dee9e Compare March 28, 2023 11:56
Expose the `clear` flag that is threaded through to `Tree.export`.
@samoht samoht merged commit 47cffd1 into mirage:main Mar 28, 2023
3 of 4 checks passed
@samoht samoht deleted the expose-clear branch March 28, 2023 19:27
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