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.

Sign up for GitHub

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

Jump to bottom

Add CachingShardStore for batching writes to a backend ShardStore #85

Merged
merged 3 commits into from
Jul 17, 2023
Merged

Add CachingShardStore for batching writes to a backend ShardStore #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2715a24
Add `CachingShardStore` for batching writes to a backend `ShardStore`
str4d Jul 14, 2023
f5160a4
shardtree: Pin `dashmap < 5.5.0` in dev-dependencies to keep MSRV
str4d Jul 14, 2023
7ec75fd
shardtree: Clarify documentation for `ShardStore::remove_checkpoint`
str4d Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions shardtree/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@ test-dependencies = ["proptest", "assert_matches"]

[target.'cfg(unix)'.dev-dependencies]
pprof = { version = "0.9", features = ["criterion", "flamegraph"] } # MSRV 1.56
dashmap = ">=5, <5.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the issue with 5.5.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I documented in the commit message, this was to keep MSRV. dashmap 5.5.0 bumped dependencies in a way that breaks the MSRV of this crate, but because we only use it as a dev-dependency we can just pin in this way. We've had this problem a lot with the criterion / pprof stack, and it's the standard way we solve it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok. I knew that zcashd has an MSRV of 1.69, the librustzcash crates' MSRVs are 1.65, and dashmap 5.5.0's MSRV is 1.64. I guess it's xacrimon/dashmap@d36311e that causes the problem? Is there a reason why we can't/shouldn't update the incrementalmerkletree repo's crates to match librustzcash?

In any case,

  • shardtree does not appear to declare its MSRV;
  • there isn't a comment in this file saying what the MSRV of dashmap is, unlike for pprof and inferno.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct that shardtree doesn't yet declare its MSRV; we should add that. The rest of the crates in this repo have MSRV 1.60, which is what CI is enforcing.

inferno = ">=0.11, <0.11.5" # MSRV 1.59
Loading
Loading