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

Feat: implement in-memory KVStorage #61

Merged
merged 7 commits into from
Sep 5, 2022

Conversation

fakedev9999
Copy link
Contributor

Closes #58

Copy link
Member

@junha1 junha1 left a comment

Choose a reason for hiding this comment

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

What happend to test_suite?

type DB = HashMap<Hash256, Vec<u8>>;

#[derive(Clone)]
pub struct IMDB {
Copy link
Member

Choose a reason for hiding this comment

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

Let's use MemoryDB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


type DB = HashMap<Hash256, Vec<u8>>;
Copy link
Member

Choose a reason for hiding this comment

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

Snapshot

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


#[derive(Clone)]
pub struct IMDB {
db: DB,
Copy link
Member

Choose a reason for hiding this comment

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

current_revision

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

impl KVStorage for IMDB {
async fn commit_checkpoint(&mut self) -> Result<(), Error> {
self.checkpoint = self.db.clone();

Copy link
Member

Choose a reason for hiding this comment

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

Please remove the new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

async fn get(&self, key: Hash256) -> Result<Vec<u8>, Error> {
match self.db.get(&key) {
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

@fakedev9999 fakedev9999 Sep 5, 2022

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

async fn contain(&self, key: Hash256) -> Result<bool, Error> {
match self.db.get(&key) {
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

@fakedev9999 fakedev9999 Sep 5, 2022

Choose a reason for hiding this comment

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

For the similar reason with async fn remove, cannot implement that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

}

async fn remove(&mut self, key: Hash256) -> Result<(), Error> {
match self.db.remove(&key) {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@fakedev9999 fakedev9999 Sep 5, 2022

Choose a reason for hiding this comment

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

Cannot implement using ok_or since HashMap::remove returns Option<Vec<u8>>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


async fn revert_to_latest_checkpoint(&mut self) -> Result<(), Error> {
self.db = self.checkpoint.clone();

Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.


async fn insert_or_update(&mut self, key: Hash256, value: &[u8]) -> Result<(), Error> {
self.db.insert(key, value.to_vec());

Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@junha1 junha1 merged commit d0b3876 into postech-dao:main Sep 5, 2022
@fakedev9999 fakedev9999 deleted the feature/inmemorykvstorage branch September 5, 2022 06:49
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.

Implement in-memory KVStorage
2 participants