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

Parity db migration #786

Merged
merged 8 commits into from Aug 31, 2022
Merged

Parity db migration #786

merged 8 commits into from Aug 31, 2022

Conversation

i1i1
Copy link
Contributor

@i1i1 i1i1 commented Aug 25, 2022

Fixes #567

This pr migrates to parity db from rocksdb. I decided to migrate only piece index hash db (copying data over to parity db) and just remove all commitments (as we can regenerate them, and it shouldn't take much time).

Code contributor checklist:

@i1i1
Copy link
Contributor Author

i1i1 commented Aug 27, 2022

Here is the issue related to the release of parity-db

@i1i1
Copy link
Contributor Author

i1i1 commented Aug 29, 2022

Benchmarked changes, and it seems that performance has decreased:

Before:

$ cargo run --release -p subspace-farmer -- --base-path ... bench --plot-size 3G --write-pieces-size 6G
Finished benchmarking.

3.13G allocated for farming
2.79G actual space pledged (which is 89.25%)
344.44M of overhead (which is 10.75%)
1m 26s plotting time
34.69M/s average plotting throughput
Recommitment took 1.239412406s

After:

$ cargo run --release -p subspace-farmer -- --base-path ... bench --plot-size 3G --write-pieces-size 6G
Finished benchmarking.

3.20G allocated for farming
2.79G actual space pledged (which is 87.31%)
415.85M of overhead (which is 12.69%)
2m 24s plotting time
20.72M/s average plotting throughput
Recommitment took 1.071120161s

Copy link
Member

@nazar-pc nazar-pc left a comment

Choose a reason for hiding this comment

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

Hm... looks like it is using a bit more space, slower to write, but faster to read 🤔

Overall makes sense to me.

Cargo.toml Outdated Show resolved Hide resolved
crates/subspace-farmer/src/commitments/databases.rs Outdated Show resolved Hide resolved
crates/subspace-farmer/src/commitments/databases.rs Outdated Show resolved Hide resolved
@i1i1 i1i1 requested a review from nazar-pc August 30, 2022 07:02
@i1i1
Copy link
Contributor Author

i1i1 commented Aug 30, 2022

@nazar-pc reverted last commit with that abstraction for commitments database. Despite that history is the same

@i1i1 i1i1 requested a review from nazar-pc August 30, 2022 13:41
nazar-pc
nazar-pc previously approved these changes Aug 30, 2022
@nazar-pc
Copy link
Member

I'd wait for upstream PR to be accepted though or at least address their concerns as we can't afford have stack overflows.

@i1i1 i1i1 merged commit 7dc6ffd into main Aug 31, 2022
@i1i1 i1i1 deleted the parity-db-migration branch August 31, 2022 19:45
@nazar-pc nazar-pc mentioned this pull request Sep 2, 2022
1 task
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.

Remove RocksDB from subspace-farmer
2 participants