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

zebra-state: replace sled with rocksdb #1325

Merged
merged 18 commits into from
Nov 19, 2020
Merged

zebra-state: replace sled with rocksdb #1325

merged 18 commits into from
Nov 19, 2020

Conversation

yaahc
Copy link
Contributor

@yaahc yaahc commented Nov 18, 2020

Motivation

Prior to this PR we've been using sled as our database for storing persistent chain data on the disk between boots. We picked sled over rocksdb to minimize our c++ dependencies despite it being a less mature codebase. The theory was if it worked well enough we'd prefer to have a pure rust codebase, but if we ever ran into problems we knew we could easily swap it out with rocksdb.

Well, we ran into problems. Sled's memory usage was particularly high, and it seemed to be leaking memory. On top of all that, the performance for writes was pretty poor, causing us to become bottle-necked on sled instead of the network.

Solution

This PR replaces sled with rocksdb. We've seen a 10x improvement in memory usage out of the box, no more leaking, and much better write performance. With this change writing chain data to disk is no longer a limiting factor in how quickly we can sync the chain.

The code in this pull request has:

  • Documentation Comments
  • Unit Tests and Property Tests

Review

@hdevalence

@yaahc yaahc marked this pull request as draft November 18, 2020 23:23
@dconnolly
Copy link
Contributor

😬 on debian-buster
image

@yaahc
Copy link
Contributor Author

yaahc commented Nov 18, 2020

grimacing on debian-buster
image

oh joy, more c++ dependency management

@yaahc yaahc requested a review from hdevalence November 19, 2020 00:51
@yaahc yaahc marked this pull request as ready for review November 19, 2020 00:51
Copy link
Contributor

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

LGTM!

One thing we'll want to do later is fiddle with the RocksDB options; using the defaults as in this PR is a good start until we do so.

Also, we previously dropped the async state accesses because of sled issues, but we should consider bringing that design back at a later point (this is a performance optimization, so let's aim for functional completeness first).

@yaahc yaahc merged commit 4c9bb87 into main Nov 19, 2020
@yaahc yaahc deleted the jane/rocksdb branch November 19, 2020 02:05
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.

3 participants