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

1/3 Implement improved API implementation for database #227

Merged
merged 11 commits into from
Sep 28, 2023
42 changes: 40 additions & 2 deletions firewood/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ use crate::{
CachedSpace, MemStoreR, SpaceWrite, StoreConfig, StoreDelta, StoreRevMut, StoreRevShared,
ZeroStore, PAGE_SIZE_NBIT,
},
v2::api::Proof,
v2::api::{self, Proof},
};
use async_trait::async_trait;
use bytemuck::{cast_slice, AnyBitPattern};
use metered::{metered, HitCount};
use parking_lot::{Mutex, RwLock};
Expand All @@ -28,7 +29,7 @@ use std::{
collections::VecDeque,
error::Error,
fmt,
io::{Cursor, Write},
io::{Cursor, ErrorKind, Write},
mem::size_of,
num::NonZeroUsize,
os::fd::{AsFd, BorrowedFd},
Expand Down Expand Up @@ -272,6 +273,43 @@ pub struct DbRev<S> {
merkle: Merkle<S>,
}

#[async_trait]
impl<S: ShaleStore<Node> + Send + Sync> api::DbView for DbRev<S> {
async fn root_hash(&self) -> Result<api::HashKey, api::Error> {
self.merkle
.root_hash(self.header.kv_root)
.map(|h| *h)
.map_err(|e| api::Error::IO(std::io::Error::new(ErrorKind::Other, e)))
}

async fn val<K: api::KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, api::Error> {
Copy link
Contributor

@richardpringle richardpringle Sep 28, 2023

Choose a reason for hiding this comment

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

It's infuriating that you had to change this.

When we complete the following, we will be able to put it back:
#290

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed 😢

let obj_ref = self.merkle.get(key, self.header.kv_root);
match obj_ref {
Err(e) => Err(api::Error::IO(std::io::Error::new(ErrorKind::Other, e))),
Ok(obj) => match obj {
None => Ok(None),
Some(inner) => Ok(Some(inner.as_ref().to_owned())),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Naming is hard.., we discussed to no longer use inner as it didn't bring any clarification?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to stop using the inner pattern. Naming something inner when we've simply added a wrapper type is fine IMO

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. This is one of those sweep tasks we can assign as a good first issue. Anyone want to create one?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's trivial to unravel all the SomeType { inner: SomeInnerType } especially with all the misused trait-bounds

},
}
}

async fn single_key_proof<K: api::KeyType, N: AsRef<[u8]> + Send>(
&self,
_key: K,
) -> Result<Option<Proof<N>>, api::Error> {
todo!()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If these are todo maybe we can do it in a follow up with real implementations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's why this is 1/3 :)


async fn range_proof<K: api::KeyType, V, N>(
&self,
_first_key: Option<K>,
_last_key: Option<K>,
_limit: usize,
) -> Result<Option<api::RangeProof<K, V, N>>, api::Error> {
todo!()
}
}

impl<S: ShaleStore<Node> + Send + Sync> DbRev<S> {
fn flush_dirty(&mut self) -> Option<()> {
self.header.flush_dirty();
Expand Down
2 changes: 1 addition & 1 deletion firewood/src/v2/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub trait DbView {
async fn root_hash(&self) -> Result<HashKey, Error>;

/// Get the value of a specific key
async fn val<K: KeyType>(&self, key: K) -> Result<Option<&[u8]>, Error>;
async fn val<K: KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, Error>;

/// Obtain a proof for a single key
async fn single_key_proof<K: KeyType, V: ValueType>(
Expand Down
2 changes: 1 addition & 1 deletion firewood/src/v2/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl api::DbView for DbView {
todo!()
}

async fn val<K: KeyType>(&self, _key: K) -> Result<Option<&[u8]>, api::Error> {
async fn val<K: KeyType>(&self, _key: K) -> Result<Option<Vec<u8>>, api::Error> {
todo!()
}

Expand Down
2 changes: 1 addition & 1 deletion firewood/src/v2/emptydb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl DbView for HistoricalImpl {
Ok(ROOT_HASH)
}

async fn val<K: KeyType>(&self, _key: K) -> Result<Option<&[u8]>, Error> {
async fn val<K: KeyType>(&self, _key: K) -> Result<Option<Vec<u8>>, Error> {
Ok(None)
}

Expand Down
4 changes: 2 additions & 2 deletions firewood/src/v2/propose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,12 @@ impl<T: api::DbView + Send + Sync> api::DbView for Proposal<T> {
todo!()
}

async fn val<K: KeyType>(&self, key: K) -> Result<Option<&[u8]>, api::Error> {
async fn val<K: KeyType>(&self, key: K) -> Result<Option<Vec<u8>>, api::Error> {
// see if this key is in this proposal
match self.delta.get(key.as_ref()) {
Some(change) => match change {
// key in proposal, check for Put or Delete
KeyOp::Put(val) => Ok(Some(val)),
KeyOp::Put(val) => Ok(Some(val.to_owned())),
KeyOp::Delete => Ok(None), // key was deleted in this proposal
},
None => match &self.base {
Expand Down