cosmos / iavl

Merkleized IAVL+ Tree implementation in Go
Apache License 2.0
417 stars 263 forks source link

feat: Uses BatchWithFlusher in iavl tree #807

Closed catShaark closed 1 year ago

catShaark commented 1 year ago

This PR replace the use of normal dbm.Batch with BatchWithFlusher in iavl tree It also hard code the value of flushthreshold to 100KB

catShaark commented 1 year ago

That's because I discussed with marko and we decide to hard code the value to be the best option based on benchmark result run on osmosis node

julienrbrt commented 1 year ago

+1 on @cool-develope comment.

Given that newNodeDB takes Options we could add an option in here: https://github.com/notional-labs/iavl/blob/a945f5f903800a214d30e55f87ca96cc6bc78051/options.go#L71-L89 and use 100000 in the DefaultOptions. If going this way, we should make them variadic so you don't end up overwriting that value to 0 by defining new options.

tac0turtle commented 1 year ago

@julienrbrt you fine with merging this and then i can make the changes.

julienrbrt commented 1 year ago

@julienrbrt you fine with merging this and then i can make the changes.

Works! We need changelogs as well to reflect some API breaks from here in the follow-up

julienrbrt commented 1 year ago

@mergifyio backport release/1.x.x

mergify[bot] commented 1 year ago

backport release/1.x.x

❌ No backport have been created

* Backport to branch `release/1.x.x` failed GitHub error: ```Branch not found```
julienrbrt commented 1 year ago

@Mergifyio backport release/v1.x.x

tac0turtle commented 1 year ago

@mergifyio backport release/v1.x.x

mergify[bot] commented 1 year ago

backport release/v1.x.x

✅ Backports have been created

* [#816 feat: Uses BatchWithFlusher in iavl tree (backport #807)](https://github.com/cosmos/iavl/pull/816) has been created for branch `release/v1.x.x`