Skip to content

Commit

Permalink
[storage] small fixes (#4419)
Browse files Browse the repository at this point in the history
  • Loading branch information
msmouse authored Sep 22, 2022
1 parent 4c42944 commit 991ea85
Show file tree
Hide file tree
Showing 31 changed files with 85 additions and 126 deletions.
12 changes: 6 additions & 6 deletions consensus/src/liveness/leader_reputation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl AptosDBBackend {
fn refresh_db_result(
&self,
mut locked: MutexGuard<'_, (Vec<NewBlockEvent>, u64, bool)>,
lastest_db_version: u64,
latest_db_version: u64,
) -> Result<(Vec<NewBlockEvent>, u64, bool)> {
// assumes target round is not too far from latest commit
let limit = self.window_size + self.seek_len;
Expand All @@ -72,7 +72,7 @@ impl AptosDBBackend {
u64::max_value(),
Order::Descending,
limit as u64,
lastest_db_version,
latest_db_version,
)?;

let max_returned_version = events.first().map_or(0, |first| first.transaction_version);
Expand All @@ -86,7 +86,7 @@ impl AptosDBBackend {

let result = (
new_block_events,
std::cmp::max(lastest_db_version, max_returned_version),
std::cmp::max(latest_db_version, max_returned_version),
hit_end,
);
*locked = result.clone();
Expand Down Expand Up @@ -143,10 +143,10 @@ impl MetadataBackend for AptosDBBackend {
let has_larger = events.first().map_or(false, |e| {
(e.epoch(), e.round()) >= (target_epoch, target_round)
});
let lastest_db_version = self.aptos_db.get_latest_version().unwrap_or(0);
let latest_db_version = self.aptos_db.get_latest_version().unwrap_or(0);
// check if fresher data has potential to give us different result
if !has_larger && version < lastest_db_version {
let fresh_db_result = self.refresh_db_result(locked, lastest_db_version);
if !has_larger && version < latest_db_version {
let fresh_db_result = self.refresh_db_result(locked, latest_db_version);
match fresh_db_result {
Ok((events, _version, hit_end)) => {
self.get_from_db_result(target_epoch, target_round, &events, hit_end)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -355,15 +355,15 @@ The tree structure would look exactly the same as the one in [Overview and Archi
/// This module implements `SpeculationCache` that is an in-memory representation of this tree.
/// The tree is reprensented by a root block id,
/// all the children of root and a global block map. Each block is an Arc<Mutx<SpeculationBlock>>
/// with ref_count = 1. For the chidren of the root, the sole owner is `heads`. For the rest, the sole
/// with ref_count = 1. For the children of the root, the sole owner is `heads`. For the rest, the sole
/// owner is their parent block. So when a block is dropped, all its descendants will be dropped
/// recursively. In the meanwhile, wheir entries in the block map will be removed by each block's drop().
pub(crate) struct SpeculationCache {
synced_trees: ExecutedTrees,
committed_trees: ExecutedTrees,
// The id of root block.
committed_block_id: HashValue,
// The chidren of root block.
// The children of root block.
heads: Vec<Arc<Mutex<SpeculationBlock>>>,
// A pointer to the global block map keyed by id to achieve O(1) lookup time complexity.
// It is optional but an optimization.
Expand Down
5 changes: 0 additions & 5 deletions execution/executor-benchmark/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use aptos_config::config::{
};
use aptos_jellyfish_merkle::metrics::{
APTOS_JELLYFISH_INTERNAL_ENCODED_BYTES, APTOS_JELLYFISH_LEAF_ENCODED_BYTES,
APTOS_JELLYFISH_STORAGE_READS,
};
use aptosdb::AptosDB;

Expand Down Expand Up @@ -178,10 +177,6 @@ fn add_accounts_impl(
// Write metadata
generator.write_meta(&output_dir, num_new_accounts);

println!(
"Total reads from storage: {}",
APTOS_JELLYFISH_STORAGE_READS.get()
);
println!(
"Total written internal nodes value size: {} bytes",
APTOS_JELLYFISH_INTERNAL_ENCODED_BYTES.get()
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/aptosdb_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ fn test_get_latest_executed_trees() {
let tmp_dir = TempPath::new();
let db = AptosDB::new_for_test(&tmp_dir);

// entirely emtpy db
// entirely empty db
let empty = db.get_latest_executed_trees().unwrap();
assert!(empty.is_same_view(&ExecutedTrees::new_empty()));

Expand Down
17 changes: 8 additions & 9 deletions storage/aptosdb/src/event_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ impl EventStore {
let msg = if cur_seq == start_seq_num {
"First requested event is probably pruned."
} else {
"DB corruption: Sequence number not continous."
"DB corruption: Sequence number not continuous."
};
bail!("{} expected: {}, actual: {}", msg, cur_seq, seq);
}
Expand Down Expand Up @@ -322,13 +322,16 @@ impl EventStore {
batch.put::<EventByVersionSchema>(
&(*event.key(), version, event.sequence_number()),
&(idx as u64),
)?;
Ok(())
)
})?;

// EventAccumulatorSchema updates
let event_hashes: Vec<HashValue> = events.iter().map(ContractEvent::hash).collect();
let (root_hash, writes) = EmptyAccumulator::append(&EmptyReader, 0, &event_hashes)?;
let (root_hash, writes) = MerkleAccumulator::<EmptyReader, EventAccumulatorHasher>::append(
&EmptyReader,
0,
&event_hashes,
)?;
writes.into_iter().try_for_each(|(pos, hash)| {
batch.put::<EventAccumulatorSchema>(&(version, pos), &hash)
})?;
Expand Down Expand Up @@ -414,7 +417,7 @@ impl EventStore {
},
ledger_version,
)?.ok_or_else(|| format_err!(
"No new block found beyond timestmap {}, so can't determine the last version before it.",
"No new block found beyond timestamp {}, so can't determine the last version before it.",
timestamp,
))?;

Expand Down Expand Up @@ -528,8 +531,6 @@ impl EventStore {
}
}

type Accumulator<'a> = MerkleAccumulator<EventHashReader<'a>, EventAccumulatorHasher>;

struct EventHashReader<'a> {
store: &'a EventStore,
version: Version,
Expand All @@ -550,8 +551,6 @@ impl<'a> HashReader for EventHashReader<'a> {
}
}

type EmptyAccumulator = MerkleAccumulator<EmptyReader, EventAccumulatorHasher>;

struct EmptyReader;

// Asserts `get()` is never called.
Expand Down
3 changes: 2 additions & 1 deletion storage/aptosdb/src/ledger_store/ledger_info_test_utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Aptos
// SPDX-License-Identifier: Apache-2.0
use crate::AptosDB;
use anyhow::Result;
use aptos_types::{
ledger_info::LedgerInfoWithSignatures,
proptest_types::{AccountInfoUniverse, LedgerInfoWithSignaturesGen},
Expand Down Expand Up @@ -63,7 +64,7 @@ pub fn set_up(
ledger_infos_with_sigs
.iter()
.map(|info| store.put_ledger_info(info, &mut batch))
.collect::<anyhow::Result<Vec<_>>>()
.collect::<Result<Vec<_>>>()
.unwrap();
store.db.write_schemas(batch).unwrap();
store.set_latest_ledger_info(ledger_infos_with_sigs.last().unwrap().clone());
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/ledger_store/transaction_info_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ proptest! {
verify(store, &batch1, 0, ledger_version2, root_hash2);
verify(store, &batch2, batch1.len() as u64, ledger_version2, root_hash2);

// retrieve batch1 and verify against root_hash after batch1 was interted
// retrieve batch1 and verify against root_hash after batch1 was inserted
verify(store, &batch1, 0, ledger_version1, root_hash1);
}

Expand Down
27 changes: 6 additions & 21 deletions storage/aptosdb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ use aptos_types::{
TransactionOutput, TransactionOutputListWithProof, TransactionToCommit,
TransactionWithProof, Version,
},
write_set::WriteSet,
};
use aptos_vm::data_cache::AsMoveResolver;
use aptosdb_indexer::Indexer;
Expand Down Expand Up @@ -122,6 +121,8 @@ use storage_interface::{
pub const LEDGER_DB_NAME: &str = "ledger_db";
pub const STATE_MERKLE_DB_NAME: &str = "state_merkle_db";

// This is last line of defense against large queries slipping through external facing interfaces,
// like the API and State Sync, etc.
const MAX_LIMIT: u64 = 10000;

// TODO: Either implement an iteration API to allow a very old client to loop through a long history
Expand Down Expand Up @@ -433,7 +434,8 @@ impl AptosDB {
let state_merkle_db_secondary_path =
secondary_db_root_path.as_ref().join(STATE_MERKLE_DB_NAME);

// Secondary needs `max_open_files = -1` per https://github.com/facebook/rocksdb/wiki/Secondary-instance
// Secondary needs `max_open_files = -1` per
// https://github.com/facebook/rocksdb/wiki/Read-only-and-Secondary-instances
rocksdb_configs.ledger_db_config.max_open_files = -1;
rocksdb_configs.state_merkle_db_config.max_open_files = -1;

Expand Down Expand Up @@ -933,7 +935,7 @@ impl DbReader for AptosDB {
})
}

/// This API is best-effort in that it CANNOT provide absense proof.
/// This API is best-effort in that it CANNOT provide absence proof.
fn get_transaction_by_hash(
&self,
hash: HashValue,
Expand Down Expand Up @@ -1129,23 +1131,6 @@ impl DbReader for AptosDB {
})
}

/// Returns write sets for range [begin_version, end_version).
///
/// Used by the executor to build in memory state after a state checkpoint.
/// Any missing write set in the entire range results in an error.
fn get_write_sets(
&self,
begin_version: Version,
end_version: Version,
) -> Result<Vec<WriteSet>> {
gauged_api("get_write_sets", || {
self.error_if_ledger_pruned("Write set", begin_version)?;

self.transaction_store
.get_write_sets(begin_version, end_version)
})
}

fn get_events(
&self,
event_key: &EventKey,
Expand Down Expand Up @@ -1489,7 +1474,7 @@ impl DbWriter for AptosDB {
/// `first_version` is the version of the first transaction in `txns_to_commit`.
/// When `ledger_info_with_sigs` is provided, verify that the transaction accumulator root hash
/// it carries is generated after the `txns_to_commit` are applied.
/// Note that even if `txns_to_commit` is empty, `frist_version` is checked to be
/// Note that even if `txns_to_commit` is empty, `first_version` is checked to be
/// `ledger_info_with_sigs.ledger_info.version + 1` if `ledger_info_with_sigs` is not `None`.
fn save_transactions(
&self,
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/pruner/db_pruner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub trait DBPruner: Send + Sync {

/// Returns the target version for the current pruning round - this might be different from the
/// target_version() because we need to keep max_version in account.
fn get_currrent_batch_target(&self, max_versions: Version) -> Version {
fn get_current_batch_target(&self, max_versions: Version) -> Version {
// Current target version might be less than the target version to ensure we don't prune
// more than max_version in one go.
min(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use crate::{
},
EventStore, StateStore, TransactionStore,
};

use aptos_logger::warn;
use aptos_types::transaction::{AtomicVersion, Version};
use schemadb::{ReadOptions, SchemaBatch, DB};
Expand Down Expand Up @@ -173,7 +172,7 @@ impl LedgerPruner {

// Current target version might be less than the target version to ensure we don't prune
// more than max_version in one go.
let current_target_version = self.get_currrent_batch_target(max_versions as Version);
let current_target_version = self.get_current_batch_target(max_versions as Version);

self.transaction_store_pruner.prune(
db_batch,
Expand Down
2 changes: 0 additions & 2 deletions storage/aptosdb/src/schema/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,12 @@ pub const EVENT_ACCUMULATOR_CF_NAME: ColumnFamilyName = "event_accumulator";
pub const EVENT_BY_KEY_CF_NAME: ColumnFamilyName = "event_by_key";
pub const EVENT_BY_VERSION_CF_NAME: ColumnFamilyName = "event_by_version";
pub const EVENT_CF_NAME: ColumnFamilyName = "event";
pub const INDEXER_METADATA_CF_NAME: ColumnFamilyName = "indexer_metadata";
pub const JELLYFISH_MERKLE_NODE_CF_NAME: ColumnFamilyName = "jellyfish_merkle_node";
pub const LEDGER_INFO_CF_NAME: ColumnFamilyName = "ledger_info";
pub const STALE_NODE_INDEX_CF_NAME: ColumnFamilyName = "stale_node_index";
pub const STALE_NODE_INDEX_CROSS_EPOCH_CF_NAME: ColumnFamilyName = "stale_node_index_cross_epoch";
pub const STALE_STATE_VALUE_INDEX_CF_NAME: ColumnFamilyName = "stale_state_value_index";
pub const STATE_VALUE_CF_NAME: ColumnFamilyName = "state_value";
pub const TABLE_INFO_CF_NAME: ColumnFamilyName = "table_info";
pub const TRANSACTION_CF_NAME: ColumnFamilyName = "transaction";
pub const TRANSACTION_ACCUMULATOR_CF_NAME: ColumnFamilyName = "transaction_accumulator";
pub const TRANSACTION_BY_ACCOUNT_CF_NAME: ColumnFamilyName = "transaction_by_account";
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/schema/stale_node_index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//!
//! ```text
//! |<--------------key-------------->|
//! | stale_since_vesrion | node_key |
//! | stale_since_version | node_key |
//! ```
//!
//! `stale_since_version` is serialized in big endian so that records in RocksDB will be in order of
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/schema/stale_state_value_index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//!
//! ```text
//! |<-------------------key------------------->|
//! | stale_since_vesrion | version | state_key |
//! | stale_since_version | version | state_key |
//! ```
//!
//! `stale_since_version` is serialized in big endian so that records in RocksDB will be in order of
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/schema/state_value/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//! An Index Key in this data set has 2 pieces of information:
//! 1. The state key
//! 2. The version associated with the key
//! The value associated with the key is the the serialized State Value.
//! The value associated with the key is the serialized State Value.
//!
//! ```text
//! |<-------- key -------->|<--- value --->|
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/schema/write_set/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Aptos
// SPDX-License-Identifier: Apache-2.0

//! This module defines physical storage schema for write set emited by each transaction
//! This module defines physical storage schema for write set emitted by each transaction
//! saved to storage.
//!
//! Serialized signed transaction bytes identified by version.
Expand Down
4 changes: 2 additions & 2 deletions storage/aptosdb/src/state_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ impl Deref for StateStore {
}

// "using an Arc<dyn DbReader> as an Arc<dyn StateReader>" is not allowed in stable Rust. Actually we
// want another trait, `StateReader`, which is a subset of `DbReaer` here but Rust does not support trait
// upcasting coercion for now. Should change it to a different trait once upcasting is stablized.
// want another trait, `StateReader`, which is a subset of `DbReader` here but Rust does not support trait
// upcasting coercion for now. Should change it to a different trait once upcasting is stabilized.
// ref: https://github.com/rust-lang/rust/issues/65991
impl DbReader for StateDb {
/// Returns the latest state snapshot strictly before `next_version` if any.
Expand Down
2 changes: 1 addition & 1 deletion storage/aptosdb/src/state_store/state_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ proptest! {
let store2 = &db2.state_store;

let mut restore =
StateSnapshotRestore::new(&store2.state_merkle_db, store2, version, expected_root_hash, false, /* async_commit */).unwrap();
StateSnapshotRestore::new(&store2.state_merkle_db, store2, version, expected_root_hash, true, /* async_commit */).unwrap();

let mut ordered_input: Vec<_> = input
.into_iter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl EpochEndingBackupController {

pub async fn run(self) -> Result<FileHandle> {
info!(
"Epoch ending backup started, starting from epoch {}, unill epoch {} (excluded).",
"Epoch ending backup started, starting from epoch {}, until epoch {} (excluded).",
start_epoch = self.start_epoch,
end_epoch = self.end_epoch,
);
Expand Down
2 changes: 1 addition & 1 deletion storage/backup/backup-cli/src/metadata/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl MetadataView {
}
ensure!(
backup.first_version == next_ver,
"Transactioon backup ranges not continuous, expecting version {}, got {}.",
"Transaction backup ranges not continuous, expecting version {}, got {}.",
next_ver,
backup.first_version,
);
Expand Down
2 changes: 1 addition & 1 deletion storage/indexer/src/schema/indexer_metadata/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Aptos
// SPDX-License-Identifier: Apache-2.0

//! This module defines physical storage schema storing medadata for the internal indexer
//! This module defines physical storage schema storing metadata for the internal indexer
//!
use crate::metadata::{MetadataKey, MetadataValue};
Expand Down
2 changes: 1 addition & 1 deletion storage/jellyfish-merkle/src/iterator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ struct NodeVisitInfo {

/// This integer always has exactly one 1-bit. The position of the 1-bit (from LSB) indicates
/// the next child to visit in the iteration process. All the ones on the left have already
/// been visited. All the chilren on the right (including this one) have not been visited yet.
/// been visited. All the children on the right (including this one) have not been visited yet.
next_child_to_visit: u16,
}

Expand Down
8 changes: 0 additions & 8 deletions storage/jellyfish-merkle/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,6 @@ pub static APTOS_JELLYFISH_INTERNAL_ENCODED_BYTES: Lazy<IntCounter> = Lazy::new(
.unwrap()
});

pub static APTOS_JELLYFISH_STORAGE_READS: Lazy<IntCounter> = Lazy::new(|| {
register_int_counter!(
"aptos_jellyfish_storage_reads",
"Aptos jellyfish reads from storage"
)
.unwrap()
});

pub static APTOS_JELLYFISH_LEAF_COUNT: Lazy<IntGauge> = Lazy::new(|| {
register_int_gauge!(
"aptos_jellyfish_leaf_count",
Expand Down
Loading

0 comments on commit 991ea85

Please sign in to comment.