Introduce the KVDB to phala runtime #749
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…tate storage into db
kvinwang
reviewed
May 9, 2022
| @@ -52,6 +51,7 @@ members = [ | |||
| "crates/phala-async-executor", | |||
| "crates/phala-allocator", | |||
| "crates/pink", | |||
| "crates/pkvdb", | |||
| @@ -5,4 +5,4 @@ edition = "2018" | |||
| description = "Platform abstraction layer for Phactory Runtime" | |||
|
|
|||
| [dependencies] | |||
| anyhow = "1.0.43" | |||
| anyhow = "1.0.43" | |||
| @@ -2,7 +2,7 @@ use super::*; | |||
|
|
|||
| // For bin_api | |||
| impl<Platform: pal::Platform + Serialize + DeserializeOwned> Phactory<Platform> { | |||
| fn get_info_json(&self) -> Result<Value, Value> { | |||
| fn get_info_json(&mut self) -> Result<Value, Value> { | |||
There was a problem hiding this comment.
Looks not good to turn this top level state into mut.
| use std::{io::Write, marker::PhantomData}; | ||
| use std::{path::Path, str}; | ||
| use std::{marker::PhantomData}; | ||
| use std::{ str}; |
| @@ -74,7 +74,7 @@ mod types; | |||
|
|
|||
| // TODO: Completely remove the reference to Phala/Khala runtime. Instead we can create a minimal | |||
| // runtime definition locally. | |||
| type RuntimeHasher = <chain::Runtime as frame_system::Config>::Hashing; | |||
| pub(crate) type RuntimeHasher = <chain::Runtime as frame_system::Config>::Hashing; | |||
There was a problem hiding this comment.
Doesn't it available in sub mods without the pub(crate)?
|
|
||
| impl<H: Hasher> Default for PhalaTrieStorage<H> { | ||
| fn default() -> Self { | ||
| Self::Empty |
There was a problem hiding this comment.
Where the Empty is used? Is it possible to remove this variant?
| { | ||
| fn from(v: Vec<u8>) -> Self { | ||
| let (rc, val) = v.split_at(4); | ||
| let value = T::from(val); |
| db: Arc<Mutex<DB>>, | ||
| pink_overlay: Arc<Mutex<MemoryDB<H>>>, | ||
| sync_overlay: Arc<Mutex<MemoryDB<H>>>, | ||
| raw_map: BTreeMap<Vec<u8>, Vec<u8>>, |
There was a problem hiding this comment.
Only one overlay(the raw_map) here should be enough. pink/sync are user logic.
| sync_overlay: Arc<Mutex<MemoryDB<H>>>, | ||
| raw_map: BTreeMap<Vec<u8>, Vec<u8>>, | ||
| pub pink_root: Option<H::Out>, | ||
| pub sync_root: Option<H::Out>, |
There was a problem hiding this comment.
The root should be tracked by user, don't put them in this low level DB layer.
| } | ||
| } | ||
|
|
||
| impl<H: Hasher> HashDB<H, DBValue> for TrieEssenceStorage<H> |
There was a problem hiding this comment.
So many impl HashDB for .... Can we reduce some of them?
|
Closed. The work is continued in #928 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is targetd to replace phactory and pink MemoryDB into underlying KVDB.
Currenly, Pink and Phactory both use the MemoryDB, Morever after a specificed interval, Phactory will take a checkpoint not only over it's RuntimeState but the Pink storage. Unfortunatly, just replace the Pink storage could cause the whole system into inconsistence. For example:
Assume that, Phactory is now sync
100thblock from pherry and Pink is working on99thblock, if we just simply commit the transaction in Pink into the underlying storage, when the phactory crash now, Phactory checkpoint maybe at59thblock. When phala restart, from59thblock to99block storage changes is duplicated,Implementation Details
firstly we should introduce the KVDB as a crate to support base semantics used for both
PinkandPhactory.Pinkis just need aTrieBackend. So simplly wraps the underlying storage asTrieBackendStoragemakes thing sound. And forPhactoryit just needs theRawDBinterface notHashDB. Due to this, we should introduce another interface over pkvdb to supply theRawDBinterface.And then, commit all transaction both
Pinkchanges andPhactorycheckpoint per block makes thing sound.