-
Notifications
You must be signed in to change notification settings - Fork 971
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
node: caching for IPFS blockstore #108
Comments
Hi, @Wondertan.
But is DefaultCacheOpts enough in this case or they should be configured in some particular way? I would be happy to help you with this issue if you don't mind. |
@Wondertan is currently on vacation so let me briefly chime in here. Yes, you are welcome to take that issue! It's not the most urgent as it is an optimization but it's still cool to tackle it now! Please bare with us if we are a bit slow on reviewing this as we are pushing hard towards a first devnet.
I think the options should be well understood and unless there is any obvious thing that needs to change, we can go with the default options (looking at those default options they seem fine to me for now but I'm also not deeply familiar with the internals here). It would be good if the PR contained a brief rationale in the comments for why the default options are fine (or in a follow-up PR). I also think it is somewhat important to look at the existing benchmarks for ARC and make sure that we don't accidentally slow things down in our codebase. It would be good to understand the implications of using the cache (do we consume much more memory/CPU time without substantial gains?), especially in interplay with the actual underlying DB (badger as far as I remember) that probably uses a cache too. Also, another question to clarify: should the cache-store be the only supported store, or should it be optional? If the cached one has any (even minor or just potential) downsides, it should be optional. I defer to @Wondertan's expertise to give a recommendation but as mentioned above it might take a while till we find the time to focus on this too. P.S. For those following along and not sure what ARC means in this context: https://github.com/ipfs/go-ipfs-blockstore/blob/03acccfc7eae2b977b3ec49663b6e23b6a1bdf9e/arc_cache.go#L16-L20 |
Hi @liamsi. Thanks for the opportunity to help you. Am I understand you correctly that besides the implementation I should prepare a small explanation regarding CacheOpts and check memory consumption within the PR? |
Yeah, a brief explanation for the cacheopts would definitely be great. Benchmarks comparing current performance, CPU and mem-consumption with and without the cache would also be great but could potentially be done in a separate PR. |
Context
We use Bitswap to serve Shares over the network. Bitswap heavily relies on ipfs-blockstore and produces lots of IO accesses. To optimize things a bit and prevent unnecessary IO, we can reuse caching which is already implemented for us. It includes BloomFilter and ARC. Caching is especially helpful for us, as the expected data production/propagation rate and thus IO is much higher than for IPFS.
Implementation
Simply wrap blockstore instance in node building with code linked above
The text was updated successfully, but these errors were encountered: