Skip to content
This repository has been archived by the owner on Jan 11, 2024. It is now read-only.

FM-16: RocksDB #24

Merged
merged 8 commits into from
Feb 10, 2023
Merged

FM-16: RocksDB #24

merged 8 commits into from
Feb 10, 2023

Conversation

aakoshh
Copy link
Contributor

@aakoshh aakoshh commented Feb 9, 2023

Related to consensus-shipyard/ipc#287

Implements KVStore for RocksDB. Unfortunately I could not use the Forest RocksDB library any more, because even if I specified the multi-threaded-cf feature which is required for .transaction(), I had to create an OptimisticTransactionDB, not a DB, so I ended up copying a lot of the Forest code.

The PR changes App to use KVStore.

In this instance, after so much work, it doesn't make a difference because we don't modify the app state concurrently, nor do we perform multiple steps during writes/reads. Still, it felt right to put the data into a different column family, and at least this way we can use the same abstraction elsewhere in the application, like for relayers. Another area is where it can help is supporting queries by maintaining a list of the past N state roots. If it's slow, we can always go back.

I will add tests in a follow up PR.

@aakoshh aakoshh requested a review from adlrocha February 9, 2023 22:01
Base automatically changed from fm-16-storage-abstraction to master February 10, 2023 10:20
T: DeserializeOwned,
{
fn from_repr(repr: &Self::Repr) -> KVResult<T> {
fvm_ipld_encoding::from_slice(repr).map_err(|e| KVError::Codec(Box::new(e)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adlrocha I can't remember now if the restriction on String keys in maps only affects HAMT fields or everything you ever want to store.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's just the HAMT which serializes as CBOR but deserializes as IPLD. Just confusing that we are using fvm_ipld_encoding here.

@aakoshh aakoshh merged commit 97c7a98 into master Feb 10, 2023
@aakoshh aakoshh deleted the fm-16-rocksdb branch February 10, 2023 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant