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 schema batch write v2 #3964

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ members = [
"commons/forkable-jellyfish-merkle",
"commons/time-service",
"commons/infallible",
"commons/starcoin-temppath",
"types",
"types/uint",
"genesis",
Expand All @@ -28,6 +29,7 @@ members = [
"state/service",
"config",
"storage",
"storage/schemadb",
"consensus",
"consensus/cryptonight-rs",
"testsuite",
Expand Down Expand Up @@ -131,6 +133,7 @@ default-members = [
"commons/accumulator",
"commons/forkable-jellyfish-merkle",
"commons/infallible",
"commons/starcoin-temppath",
"types",
"types/uint",
"genesis",
Expand All @@ -141,6 +144,7 @@ default-members = [
"state/service",
"config",
"storage",
"storage/schemadb",
"consensus",
"consensus/cryptonight-rs",
"testsuite",
Expand Down Expand Up @@ -475,6 +479,7 @@ starcoin-state-store-api = { path = "state/state-store-api" }
starcoin-state-tree = { path = "state/state-tree" }
starcoin-statedb = { path = "state/statedb" }
starcoin-storage = { path = "storage" }
starcoin-schemadb = { path = "storage/schemadb" }
starcoin-stratum = { path = "stratum" }
starcoin-sync = { path = "sync" }
starcoin-sync-api = { path = "sync/api" }
Expand All @@ -489,6 +494,7 @@ starcoin-types = { path = "types" }
starcoin-uint = { path = "types/uint" }
starcoin-vm-runtime = { path = "vm/vm-runtime" }
starcoin-vm-types = { path = "vm/types" }
starcoin-temppath = { path = "commons/starcoin-temppath" }
stdlib = { path = "vm/stdlib" }
stest = { path = "commons/stest" }
stest-macro = { path = "commons/stest/stest-macro" }
Expand Down
1 change: 1 addition & 0 deletions account/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ starcoin-decrypt = { workspace = true }
starcoin-logger = { workspace = true }
starcoin-storage = { workspace = true }
starcoin-types = { workspace = true }
starcoin-schemadb = { workspace = true }

[dev-dependencies]
hex = { workspace = true }
Expand Down
20 changes: 10 additions & 10 deletions account/src/account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
use crate::account_manager::gen_private_key;
use crate::account_storage::AccountStorage;
use anyhow::{format_err, Result};
use starcoin_account_api::error::AccountError;
use starcoin_account_api::{
AccountInfo, AccountPrivateKey, AccountPublicKey, AccountResult, Setting,
error::AccountError, AccountInfo, AccountPrivateKey, AccountPublicKey, AccountResult, Setting,
};
use starcoin_crypto::PrivateKey;
use starcoin_logger::prelude::*;
use starcoin_storage::storage::StorageInstance;
use starcoin_types::account_address;
use starcoin_types::account_address::AccountAddress;
use starcoin_types::genesis_config::ChainId;
use starcoin_types::sign_message::{SignedMessage, SigningMessage};
use starcoin_types::transaction::authenticator::AuthenticationKey;
use starcoin_types::transaction::{RawUserTransaction, SignedUserTransaction};
use starcoin_types::{
account_address,
account_address::AccountAddress,
genesis_config::ChainId,
sign_message::{SignedMessage, SigningMessage},
transaction::authenticator::AuthenticationKey,
transaction::{RawUserTransaction, SignedUserTransaction},
};

pub struct Account {
addr: AccountAddress,
Expand Down Expand Up @@ -172,7 +172,7 @@ impl Account {
let private_key = gen_private_key();
let public_key = private_key.public_key();
let address = account_address::from_public_key(&public_key);
let storage = AccountStorage::new(StorageInstance::new_cache_instance());
let storage = AccountStorage::mock();
Self::create(address, private_key.into(), "".to_string(), storage).map_err(|e| e.into())
}
}
5 changes: 4 additions & 1 deletion account/src/account_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ impl AccountManager {
None => Account::create_readonly(address, public_key, self.store.clone())?,
};

// todo: merge following single writes to one schema-batch-write: add_address + set_default
Copy link

Choose a reason for hiding this comment

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

Implement batch writes for performance optimization.

The TODO comment indicates the need to merge single writes into one schema-batch-write. This would significantly improve performance by reducing the number of I/O operations.

self.store.add_address(*account.address())?;

// if it's the first address, set it default.
Expand Down Expand Up @@ -195,7 +196,8 @@ impl AccountManager {
"Can not find account_info by address:{}, clear it from address list.",
account
);
self.store.remove_address(account)?;
// todo: merge single writes to one batch-write
Copy link

Choose a reason for hiding this comment

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

Consider implementing batch writes for address removal.

The TODO comment suggests merging single writes into one batch-write to optimize performance during the removal of addresses from the list.

self.store.remove_address_from_all(account)?;
}
}
Ok(res)
Expand Down Expand Up @@ -254,6 +256,7 @@ impl AccountManager {
}
}

// todo: merge single writes to one batch-write
Copy link

Choose a reason for hiding this comment

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

Implement batch writes for setting default account.

The TODO comment here also indicates a need for batch writes. Implementing this can reduce the number of transactions committed to the database, improving performance, especially during high load operations.

- // todo: merge single writes to one batch-write
+ self.store.set_default_account_batch(&address)?;

Committable suggestion was skipped due to low confidence.

pub fn set_default_account(&self, address: AccountAddress) -> AccountResult<AccountInfo> {
let mut account_info = self
.account_info(address)?
Expand Down
42 changes: 42 additions & 0 deletions account/src/account_schemadb/accepted_token.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use anyhow::Result;
use bcs_ext::BCSCodec;
use serde::{Deserialize, Serialize};
use starcoin_schemadb::{
define_schema,
schema::{KeyCodec, ValueCodec},
ColumnFamilyName,
};
use starcoin_types::account_address::AccountAddress;
use starcoin_types::account_config::token_code::TokenCode;

pub const ACCEPTED_TOKEN_PREFIX_NAME: ColumnFamilyName = "accepted_token";

define_schema!(
AcceptedToken,
AccountAddress,
AcceptedTokens,
ACCEPTED_TOKEN_PREFIX_NAME
);

impl KeyCodec<AcceptedToken> for AccountAddress {
fn encode_key(&self) -> Result<Vec<u8>> {
Ok(self.to_vec())
}

fn decode_key(data: &[u8]) -> Result<Self> {
AccountAddress::try_from(data).map_err(Into::into)
}
}

#[derive(Default, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
pub struct AcceptedTokens(pub Vec<TokenCode>);

impl ValueCodec<AcceptedToken> for AcceptedTokens {
fn encode_value(&self) -> Result<Vec<u8>> {
self.0.encode()
}

fn decode_value(data: &[u8]) -> Result<Self> {
<Vec<TokenCode>>::decode(data).map(AcceptedTokens)
}
}
50 changes: 50 additions & 0 deletions account/src/account_schemadb/global_setting.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use anyhow::Result;
use bcs_ext::BCSCodec;
use serde::{Deserialize, Serialize};
use starcoin_schemadb::{
define_schema,
schema::{KeyCodec, ValueCodec},
ColumnFamilyName,
};
use starcoin_types::account_address::AccountAddress;

pub const GLOBAL_PREFIX_NAME: ColumnFamilyName = "global";

define_schema!(
GlobalSetting,
GlobalSettingKey,
GlobalValue,
GLOBAL_PREFIX_NAME
);

#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
pub enum GlobalSettingKey {
DefaultAddress,
/// FIXME: once db support iter, remove this.
Copy link

Choose a reason for hiding this comment

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

Track the FIXME for future improvements.

The FIXME comment suggests that the AllAddresses key might be deprecated in the future. It's important to ensure this is tracked in an issue or a project management tool.

Would you like me to open a GitHub issue to track this FIXME?

AllAddresses,
}
Comment on lines +20 to +25
Copy link

Choose a reason for hiding this comment

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

Consider removing or refactoring the FIXME comment.

The FIXME comment on line 23 suggests future improvements when database iteration support is available. It's important to track this in the project's issue tracker for future reference.

-    /// FIXME: once db support iter, remove this.
+    // TODO: Remove AllAddresses once DB supports iteration. Tracked in issue #123
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
pub enum GlobalSettingKey {
DefaultAddress,
/// FIXME: once db support iter, remove this.
AllAddresses,
}
#[derive(Hash, Copy, Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
pub enum GlobalSettingKey {
DefaultAddress,
// TODO: Remove AllAddresses once DB supports iteration. Tracked in issue #123
AllAddresses,
}


#[derive(Default, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub struct GlobalValue {
pub(crate) addresses: Vec<AccountAddress>,
}

impl KeyCodec<GlobalSetting> for GlobalSettingKey {
fn encode_key(&self) -> Result<Vec<u8>> {
self.encode()
}

fn decode_key(data: &[u8]) -> Result<Self> {
GlobalSettingKey::decode(data)
}
}

impl ValueCodec<GlobalSetting> for GlobalValue {
fn encode_value(&self) -> Result<Vec<u8>> {
self.addresses.encode()
}

fn decode_value(data: &[u8]) -> Result<Self> {
<Vec<AccountAddress>>::decode(data).map(|addresses| GlobalValue { addresses })
}
}
78 changes: 78 additions & 0 deletions account/src/account_schemadb/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
use anyhow::Result;
use starcoin_schemadb::schema::Schema;
use starcoin_storage::cache_storage::GCacheStorage;
use std::sync::Arc;

mod accepted_token;
mod global_setting;
mod private_key;
mod public_key;
mod setting;

pub(crate) use accepted_token::*;
pub(crate) use global_setting::*;
pub(crate) use private_key::*;
pub(crate) use public_key::*;
pub(crate) use setting::*;
use starcoin_schemadb::{db::DBStorage as DB, SchemaBatch};

#[derive(Clone)]
pub(super) struct AccountStore<S: Schema> {
cache: Arc<GCacheStorage<S::Key, S::Value>>,
db: Option<Arc<DB>>,
}

impl<S: Schema> AccountStore<S> {
// create an memory-based store
pub fn new() -> Self {
Self {
cache: Arc::new(GCacheStorage::<S::Key, S::Value>::new(None)),
db: None,
}
}
pub fn new_with_db(db: &Arc<DB>) -> Self {
Self {
cache: Arc::new(GCacheStorage::<S::Key, S::Value>::new(None)),
db: Some(Arc::clone(db)),
}
}

pub fn get(&self, key: &S::Key) -> Result<Option<S::Value>> {
self.cache
.get_inner(key)
.map(|val| Ok(Some(val)))
.unwrap_or_else(|| {
self.db
.as_ref()
.map_or_else(|| Ok(None), |db| db.get::<S>(key))
})
}

pub fn put(&self, key: S::Key, value: S::Value) -> Result<()> {
self.db
.as_ref()
.map_or_else(|| Ok(()), |db| db.put::<S>(&key, &value))
.map(|_| {
self.cache.put_inner(key, value);
})
}

pub fn remove(&self, key: &S::Key) -> Result<()> {
self.db
.as_ref()
.map_or_else(|| Ok(()), |db| db.remove::<S>(key))
.map(|_| {
self.cache.remove_inner(key);
})
}

pub fn put_batch(&self, key: S::Key, value: S::Value, batch: &SchemaBatch) -> Result<()> {
batch.put::<S>(&key, &value)?;
self.put(key, value)
}

pub fn remove_batch(&self, key: &S::Key, batch: &SchemaBatch) -> Result<()> {
batch.delete::<S>(key)?;
self.remove(key)
}
}
44 changes: 44 additions & 0 deletions account/src/account_schemadb/private_key.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use anyhow::Result;
use starcoin_schemadb::{
define_schema,
schema::{KeyCodec, ValueCodec},
ColumnFamilyName,
};
use starcoin_types::account_address::AccountAddress;

pub const ENCRYPTED_PRIVATE_KEY_PREFIX_NAME: ColumnFamilyName = "encrypted_private_key";

define_schema!(
PrivateKey,
AccountAddress,
EncryptedPrivateKey,
ENCRYPTED_PRIVATE_KEY_PREFIX_NAME
);

impl KeyCodec<PrivateKey> for AccountAddress {
fn encode_key(&self) -> Result<Vec<u8>> {
Ok(self.to_vec())
}

fn decode_key(data: &[u8]) -> Result<Self> {
AccountAddress::try_from(data).map_err(Into::into)
}
}

#[derive(Default, Debug, Clone, PartialEq, Eq)]
pub struct EncryptedPrivateKey(pub Vec<u8>);
impl From<Vec<u8>> for EncryptedPrivateKey {
fn from(s: Vec<u8>) -> Self {
Self(s)
}
}

impl ValueCodec<PrivateKey> for EncryptedPrivateKey {
fn encode_value(&self) -> Result<Vec<u8>> {
Ok(self.0.clone())
}

fn decode_value(data: &[u8]) -> Result<Self> {
Ok(EncryptedPrivateKey(data.to_vec()))
}
}
Loading
Loading