-
Notifications
You must be signed in to change notification settings - Fork 20
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
Handle sharing of Index instances explicitly #188
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0591251
to
cad0a23
Compare
icristescu
approved these changes
Jun 30, 2020
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! I have just a few minor comments.
Could you rebase this PR? |
bc46e2e
to
0bb2342
Compare
Done. |
samoht
approved these changes
Jul 13, 2020
Will merge once the CI passes. |
samoht
added a commit
to samoht/irmin
that referenced
this pull request
Jul 15, 2020
samoht
added a commit
to samoht/irmin
that referenced
this pull request
Jul 15, 2020
samoht
added a commit
to samoht/irmin
that referenced
this pull request
Jul 15, 2020
craigfe
added a commit
to craigfe/opam-repository
that referenced
this pull request
Oct 21, 2020
CHANGES: ## Added - Added `flush_callback` parameter to the creation of a store, to register a callback before a flush. This callback can be temporarily disabled by `~no_callback:()` to `flush`. (mirage/index#189, mirage/index#216) - Added `Stats.merge_durations` to list the duration of the last 10 merges. (mirage/index#193) - Added `is_merging` to detect if a merge is running. (mirage/index#192) - New `IO.Header.{get,set}` functions to read and write the file headers atomically (mirage/index#175, mirage/index#204, @icristescu, @craigfe, @samoht) - Added a `throttle` configuration option to select the strategy to use when the cache are full and an async merge is already in progress. The current behavior is the (default) [`Block_writes] strategy. The new [`Overcommit_memory] does not block but continue to fill the cache instead. (mirage/index#209, @samoht) - Add `IO.exists` obligation for IO implementations, to be used for lazy creation of IO instances. (mirage/index#233, @craigfe) - `Index.close` now takes an `~immediately:()` argument. When passed, this causes `close` to terminate any ongoing asynchronous merge operation, rather than waiting for it to finish. (mirage/index#185, mirage/index#234) - Added `Index.Checks.cli`, which provides offline integrity checking of Index stores. (mirage/index#236) ## Changed - `sync` has to be called by the read-only instance to synchronise with the files on disk. (mirage/index#175) - Caching of `Index` instances is now explicit: `Index.Make` requires a cache implementation, and `Index.v` may be passed a cache to be used for instance sharing. The default behaviour is _not_ to share instances. (mirage/index#188) ## Fixed - Added values after a clear are found by read-only instances. (mirage/index#168) - Fix a race between `merge` and `sync` (mirage/index#203, @samoht, @craigfe) - Fix a potential loss of data if a crash occurs at the end of a merge (mirage/index#232)
craigfe
added a commit
to craigfe/opam-repository
that referenced
this pull request
Jan 5, 2021
CHANGES: ## Added - Added `flush_callback` parameter to the creation of a store, to register a callback before a flush. This callback can be temporarily disabled by `~no_callback:()` to `flush`. (mirage/index#189, mirage/index#216) - Added `Stats.merge_durations` to list the duration of the last 10 merges. (mirage/index#193) - Added `is_merging` to detect if a merge is running. (mirage/index#192) - New `IO.Header.{get,set}` functions to read and write the file headers atomically (mirage/index#175, mirage/index#204, @icristescu, @craigfe, @samoht) - Added a `throttle` configuration option to select the strategy to use when the cache are full and an async merge is already in progress. The current behavior is the (default) `` `Block_writes`` strategy. The new `` `Overcommit_memory`` does not block but continue to fill the cache instead. (mirage/index#209, @samoht) - Add `IO.exists` obligation for IO implementations, to be used for lazy creation of IO instances. (mirage/index#233, @craigfe) - `Index.close` now takes an `~immediately:()` argument. When passed, this causes `close` to terminate any ongoing asynchronous merge operation, rather than waiting for it to finish. (mirage/index#185, mirage/index#234) - Added `Index.Checks.cli`, which provides offline integrity checking of Index stores. (mirage/index#236) - `Index.replace` now takes a `~overcommit` argument to postpone a merge. (mirage/index#253) - `Index.merge` is now part of the public API. (mirage/index#253) - `Index.try_merge` is now part of the public API. `try_merge' is a no-op if the number of entries in the write-ahead log is smaller than `log_size`, otherwise it's `merge'. (mirage/index#253 @samoht) ## Changed - `sync` has to be called by the read-only instance to synchronise with the files on disk. (mirage/index#175) - Caching of `Index` instances is now explicit: `Index.Make` requires a cache implementation, and `Index.v` may be passed a cache to be used for instance sharing. The default behaviour is _not_ to share instances. (mirage/index#188) ## Fixed - Added values after a clear are found by read-only instances. (mirage/index#168) - Fix a race between `merge` and `sync` (mirage/index#203, @samoht, @craigfe) - Fix a potential loss of data if a crash occurs at the end of a merge (mirage/index#232) - Fix `Index.iter` to only iterate once over elements persisted on the disk (mirage/index#260, @samoht, @icristescu)
kit-ty-kate
pushed a commit
to craigfe/opam-repository
that referenced
this pull request
Jan 6, 2021
CHANGES: ## Added - Added `flush_callback` parameter to the creation of a store, to register a callback before a flush. This callback can be temporarily disabled by `~no_callback:()` to `flush`. (mirage/index#189, mirage/index#216) - Added `Stats.merge_durations` to list the duration of the last 10 merges. (mirage/index#193) - Added `is_merging` to detect if a merge is running. (mirage/index#192) - New `IO.Header.{get,set}` functions to read and write the file headers atomically (mirage/index#175, mirage/index#204, @icristescu, @craigfe, @samoht) - Added a `throttle` configuration option to select the strategy to use when the cache are full and an async merge is already in progress. The current behavior is the (default) `` `Block_writes`` strategy. The new `` `Overcommit_memory`` does not block but continue to fill the cache instead. (mirage/index#209, @samoht) - Add `IO.exists` obligation for IO implementations, to be used for lazy creation of IO instances. (mirage/index#233, @craigfe) - `Index.close` now takes an `~immediately:()` argument. When passed, this causes `close` to terminate any ongoing asynchronous merge operation, rather than waiting for it to finish. (mirage/index#185, mirage/index#234) - Added `Index.Checks.cli`, which provides offline integrity checking of Index stores. (mirage/index#236) - `Index.replace` now takes a `~overcommit` argument to postpone a merge. (mirage/index#253) - `Index.merge` is now part of the public API. (mirage/index#253) - `Index.try_merge` is now part of the public API. `try_merge' is a no-op if the number of entries in the write-ahead log is smaller than `log_size`, otherwise it's `merge'. (mirage/index#253 @samoht) ## Changed - `sync` has to be called by the read-only instance to synchronise with the files on disk. (mirage/index#175) - Caching of `Index` instances is now explicit: `Index.Make` requires a cache implementation, and `Index.v` may be passed a cache to be used for instance sharing. The default behaviour is _not_ to share instances. (mirage/index#188) ## Fixed - Added values after a clear are found by read-only instances. (mirage/index#168) - Fix a race between `merge` and `sync` (mirage/index#203, @samoht, @craigfe) - Fix a potential loss of data if a crash occurs at the end of a merge (mirage/index#232) - Fix `Index.iter` to only iterate once over elements persisted on the disk (mirage/index#260, @samoht, @icristescu)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a new
Cache.S
signature with two implementations:Noop
andUnbounded
. A cache implementation must now be provided toIndex.Make
.The explicit instance sharing is Implemented with an optional
?cache
argument toIndex.v
, from which the instance may be picked (and to which any new instances will be added).cache
is perhaps not the best name for this argument: I picked it just because that's the necessary semantics for the underlying datastructure. (pool
?sharing_pool
? Ideas welcome.)