From 850860e4f7867caab58b036dc09d38306ee6827d Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Tue, 17 Nov 2020 13:24:45 -0800 Subject: [PATCH 01/18] maybe working version --- Cargo.lock | 23 ++ zebra-state/Cargo.toml | 1 + zebra-state/src/config.rs | 25 +- zebra-state/src/service/finalized_state.rs | 218 ++++++++---------- .../service/finalized_state/disk_format.rs | 124 +++------- 5 files changed, 166 insertions(+), 225 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 94d2590117a..c1851f7834a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1476,6 +1476,18 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "librocksdb-sys" +version = "6.11.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb5b56f651c204634b936be2f92dbb42c36867e00ff7fe2405591f3b9fa66f09" +dependencies = [ + "bindgen", + "cc", + "glob", + "libc", +] + [[package]] name = "linked-hash-map" version = "0.5.3" @@ -2367,6 +2379,16 @@ dependencies = [ "opaque-debug 0.2.3", ] +[[package]] +name = "rocksdb" +version = "0.15.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "23d83c02c429044d58474eaf5ae31e062d0de894e21125b47437ec0edc1397e6" +dependencies = [ + "libc", + "librocksdb-sys", +] + [[package]] name = "rust-argon2" version = "0.8.2" @@ -3480,6 +3502,7 @@ dependencies = [ "once_cell", "primitive-types", "proptest", + "rocksdb", "serde", "sled", "spandoc", diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index ab5bf229521..724b1d36a59 100644 --- a/zebra-state/Cargo.toml +++ b/zebra-state/Cargo.toml @@ -25,6 +25,7 @@ tracing-error = "0.1.2" thiserror = "1.0.22" tokio = { version = "0.2.22", features = ["sync"] } displaydoc = "0.1.7" +rocksdb = "0.15.0" [dev-dependencies] zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] } diff --git a/zebra-state/src/config.rs b/zebra-state/src/config.rs index f0aed0a14fe..65ddfb8069f 100644 --- a/zebra-state/src/config.rs +++ b/zebra-state/src/config.rs @@ -55,20 +55,29 @@ pub struct Config { } impl Config { - /// Generate the appropriate `sled::Config` for `network`, based on the - /// provided `zebra_state::Config`. - pub(crate) fn sled_config(&self, network: Network) -> sled::Config { + pub(crate) fn open_db(&self, network: Network) -> rocksdb::DB { let net_dir = match network { Network::Mainnet => "mainnet", Network::Testnet => "testnet", }; - let config = sled::Config::default() - .cache_capacity(self.memory_cache_bytes) - .mode(sled::Mode::LowSpace); + let mut opts = rocksdb::Options::default(); + + let cfs = vec![ + rocksdb::ColumnFamilyDescriptor::new("hash_by_height", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("height_by_hash", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("block_by_height", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("tx_by_hash", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("utxo_by_outpoint", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("sprout_nullifiers", opts.clone()), + rocksdb::ColumnFamilyDescriptor::new("sapling_nullifiers", opts.clone()), + ]; + + opts.create_if_missing(true); + opts.create_missing_column_families(true); if self.ephemeral { - config.temporary(self.ephemeral) + todo!(); } else { let path = self .cache_dir @@ -76,7 +85,7 @@ impl Config { .join(format!("v{}", crate::constants::SLED_FORMAT_VERSION)) .join(net_dir); - config.path(path) + rocksdb::DB::open_cf_descriptors(&opts, path, cfs).unwrap() } } diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index a4158d3dcc3..3a3506972f4 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -17,11 +17,11 @@ use self::disk_format::{DiskDeserialize, DiskSerialize, FromDisk, IntoDisk, Tran use super::QueuedBlock; -/// The finalized part of the chain state, stored in sled. +/// The finalized part of the chain state, stored in the db. /// /// This structure has two categories of methods: /// -/// - *synchronous* methods that perform writes to the sled state; +/// - *synchronous* methods that perform writes to the db state; /// - *asynchronous* methods that perform reads. /// /// For more on this distinction, see RFC5. The synchronous methods are @@ -39,33 +39,19 @@ pub struct FinalizedState { queued_by_prev_hash: HashMap, max_queued_height: i64, - hash_by_height: sled::Tree, - height_by_hash: sled::Tree, - block_by_height: sled::Tree, - tx_by_hash: sled::Tree, - utxo_by_outpoint: sled::Tree, - sprout_nullifiers: sled::Tree, - sapling_nullifiers: sled::Tree, - // sprout_anchors: sled::Tree, - // sapling_anchors: sled::Tree, + db: rocksdb::DB, /// Commit blocks to the finalized state up to this height, then exit Zebra. debug_stop_at_height: Option, } impl FinalizedState { pub fn new(config: &Config, network: Network) -> Self { - let db = config.sled_config(network).open().unwrap(); + let db = config.open_db(network); let new_state = Self { queued_by_prev_hash: HashMap::new(), max_queued_height: -1, - hash_by_height: db.open_tree(b"hash_by_height").unwrap(), - height_by_hash: db.open_tree(b"height_by_hash").unwrap(), - block_by_height: db.open_tree(b"block_by_height").unwrap(), - tx_by_hash: db.open_tree(b"tx_by_hash").unwrap(), - utxo_by_outpoint: db.open_tree(b"utxo_by_outpoint").unwrap(), - sprout_nullifiers: db.open_tree(b"sprout_nullifiers").unwrap(), - sapling_nullifiers: db.open_tree(b"sapling_nullifiers").unwrap(), + db, debug_stop_at_height: config.debug_stop_at_height.map(block::Height), }; @@ -106,17 +92,7 @@ impl FinalizedState { /// Returns the number of bytes flushed during this call. /// See sled's `Tree.flush` for more details. pub fn flush(&self) -> sled::Result { - let mut total_flushed = 0; - - total_flushed += self.hash_by_height.flush()?; - total_flushed += self.height_by_hash.flush()?; - total_flushed += self.block_by_height.flush()?; - total_flushed += self.tx_by_hash.flush()?; - total_flushed += self.utxo_by_outpoint.flush()?; - total_flushed += self.sprout_nullifiers.flush()?; - total_flushed += self.sapling_nullifiers.flush()?; - - Ok(total_flushed) + todo!() } /// If `block_height` is greater than or equal to the configured stop height, @@ -181,19 +157,34 @@ impl FinalizedState { self.tip().map(|(height, _)| height) } + fn is_empty(&self, cf: &rocksdb::ColumnFamily) -> bool { + // use iterator to check if it's empty + !self + .db + .iterator_cf(cf, rocksdb::IteratorMode::Start) + .valid() + } + /// Immediately commit `block` to the finalized state. pub fn commit_finalized_direct(&mut self, block: Arc) -> Result { - use sled::Transactional; - let height = block .coinbase_height() .expect("finalized blocks are valid and have a coinbase height"); let hash = block.hash(); block_precommit_metrics(&hash, height, &block); + tracing::trace!(?height, ?hash, "Finalized block"); + + let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); + let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); + let block_by_height = self.db.cf_handle("block_by_height").unwrap(); + let tx_by_hash = self.db.cf_handle("tx_by_hash").unwrap(); + let utxo_by_outpoint = self.db.cf_handle("utxo_by_outpoint").unwrap(); + let sprout_nullifiers = self.db.cf_handle("sprout_nullifiers").unwrap(); + let sapling_nullifiers = self.db.cf_handle("sapling_nullifiers").unwrap(); // Assert that callers (including unit tests) get the chain order correct - if self.block_by_height.is_empty() { + if self.is_empty(hash_by_height) { assert_eq!( block::Hash([0; 32]), block.header.previous_block_hash, @@ -220,83 +211,64 @@ impl FinalizedState { ); } - let result = ( - &self.hash_by_height, - &self.height_by_hash, - &self.block_by_height, - &self.utxo_by_outpoint, - &self.tx_by_hash, - &self.sprout_nullifiers, - &self.sapling_nullifiers, - ) - .transaction( - move |( - hash_by_height, - height_by_hash, - block_by_height, - utxo_by_outpoint, - tx_by_hash, - sprout_nullifiers, - sapling_nullifiers, - )| { - // Index the block - hash_by_height.zs_insert(height, hash)?; - height_by_hash.zs_insert(hash, height)?; - block_by_height.zs_insert(height, &block)?; - - // TODO: sprout and sapling anchors (per block) - - // Consensus-critical bug in zcashd: transactions in the - // genesis block are ignored. - if block.header.previous_block_hash == block::Hash([0; 32]) { - return Ok(hash); - } + let mut batch = rocksdb::WriteBatch::default(); + + // Index the block + batch.zs_insert(hash_by_height, height, hash); + batch.zs_insert(height_by_hash, hash, height); + batch.zs_insert(block_by_height, height, &block); + + // TODO: sprout and sapling anchors (per block) + + // Consensus-critical bug in zcashd: transactions in the + // genesis block are ignored. + if block.header.previous_block_hash == block::Hash([0; 32]) { + let result = self.db.write(batch).map(|()| hash); + return result.map_err(Into::into); + } - // Index each transaction - for (transaction_index, transaction) in block.transactions.iter().enumerate() { - let transaction_hash = transaction.hash(); - let transaction_location = TransactionLocation { - height, - index: transaction_index - .try_into() - .expect("no more than 4 billion transactions per block"), - }; - tx_by_hash.zs_insert(transaction_hash, transaction_location)?; - - // Mark all transparent inputs as spent - for input in transaction.inputs() { - match input { - transparent::Input::PrevOut { outpoint, .. } => { - utxo_by_outpoint.remove(outpoint.as_bytes())?; - } - // Coinbase inputs represent new coins, - // so there are no UTXOs to mark as spent. - transparent::Input::Coinbase { .. } => {} - } - } - - // Index all new transparent outputs - for (index, output) in transaction.outputs().iter().enumerate() { - let outpoint = transparent::OutPoint { - hash: transaction_hash, - index: index as _, - }; - utxo_by_outpoint.zs_insert(outpoint, output)?; - } - - // Mark sprout and sapling nullifiers as spent - for sprout_nullifier in transaction.sprout_nullifiers() { - sprout_nullifiers.zs_insert(sprout_nullifier, ())?; - } - for sapling_nullifier in transaction.sapling_nullifiers() { - sapling_nullifiers.zs_insert(sapling_nullifier, ())?; - } + // Index each transaction + for (transaction_index, transaction) in block.transactions.iter().enumerate() { + let transaction_hash = transaction.hash(); + let transaction_location = TransactionLocation { + height, + index: transaction_index + .try_into() + .expect("no more than 4 billion transactions per block"), + }; + batch.zs_insert(tx_by_hash, transaction_hash, transaction_location); + + // Mark all transparent inputs as spent + for input in transaction.inputs() { + match input { + transparent::Input::PrevOut { outpoint, .. } => { + batch.delete_cf(utxo_by_outpoint, outpoint.as_bytes()); } + // Coinbase inputs represent new coins, + // so there are no UTXOs to mark as spent. + transparent::Input::Coinbase { .. } => {} + } + } - // for some reason type inference fails here - Ok::<_, sled::transaction::ConflictableTransactionError>(hash) - }, - ); + // Index all new transparent outputs + for (index, output) in transaction.outputs().iter().enumerate() { + let outpoint = transparent::OutPoint { + hash: transaction_hash, + index: index as _, + }; + batch.zs_insert(utxo_by_outpoint, outpoint, output); + } + + // Mark sprout and sapling nullifiers as spent + for sprout_nullifier in transaction.sprout_nullifiers() { + batch.zs_insert(sprout_nullifiers, sprout_nullifier, ()); + } + for sapling_nullifier in transaction.sapling_nullifiers() { + batch.zs_insert(sapling_nullifiers, sapling_nullifier, ()); + } + } + + let result = self.db.write(batch).map(|()| hash); if result.is_ok() && self.is_at_stop_height(height) { if let Err(e) = self.flush() { @@ -330,15 +302,13 @@ impl FinalizedState { /// Returns the tip height and hash if there is one. pub fn tip(&self) -> Option<(block::Height, block::Hash)> { - self.hash_by_height - .iter() - .rev() + let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); + self.db + .iterator_cf(hash_by_height, rocksdb::IteratorMode::End) .next() - .transpose() - .expect("expected that sled errors would not occur") .map(|(height_bytes, hash_bytes)| { - let height = block::Height::from_ivec(height_bytes); - let hash = block::Hash::from_ivec(hash_bytes); + let height = block::Height::from_bytes(height_bytes); + let hash = block::Hash::from_bytes(hash_bytes); (height, hash) }) @@ -346,31 +316,37 @@ impl FinalizedState { /// Returns the height of the given block if it exists. pub fn height(&self, hash: block::Hash) -> Option { - self.height_by_hash.zs_get(&hash) + let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); + self.db.zs_get(&height_by_hash, &hash) } /// Returns the given block if it exists. pub fn block(&self, hash_or_height: HashOrHeight) -> Option> { - let height = hash_or_height.height_or_else(|hash| self.height_by_hash.zs_get(&hash))?; + let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); + let block_by_height = self.db.cf_handle("block_by_height").unwrap(); + let height = hash_or_height.height_or_else(|hash| self.db.zs_get(height_by_hash, &hash))?; - self.block_by_height.zs_get(&height) + self.db.zs_get(block_by_height, &height) } /// Returns the `transparent::Output` pointed to by the given /// `transparent::OutPoint` if it is present. pub fn utxo(&self, outpoint: &transparent::OutPoint) -> Option { - self.utxo_by_outpoint.zs_get(outpoint) + let utxo_by_outpoint = self.db.cf_handle("utxo_by_outpoint").unwrap(); + self.db.zs_get(utxo_by_outpoint, outpoint) } /// Returns the finalized hash for a given `block::Height` if it is present. pub fn hash(&self, height: block::Height) -> Option { - self.hash_by_height.zs_get(&height) + let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); + self.db.zs_get(hash_by_height, &height) } /// Returns the given transaction if it exists. pub fn transaction(&self, hash: transaction::Hash) -> Option> { - self.tx_by_hash - .zs_get(&hash) + let tx_by_hash = self.db.cf_handle("tx_by_hash").unwrap(); + self.db + .zs_get(tx_by_hash, &hash) .map(|TransactionLocation { index, height }| { let block = self .block(height.into()) diff --git a/zebra-state/src/service/finalized_state/disk_format.rs b/zebra-state/src/service/finalized_state/disk_format.rs index 77e9a4f33b5..4917b9fee2b 100644 --- a/zebra-state/src/service/finalized_state/disk_format.rs +++ b/zebra-state/src/service/finalized_state/disk_format.rs @@ -25,9 +25,6 @@ pub trait IntoDisk { // function to convert the current type to its disk format in `zs_get()` // without necessarily allocating a new IVec fn as_bytes(&self) -> Self::Bytes; - - // function to convert the current type into its disk format - fn into_ivec(&self) -> sled::IVec; } impl<'a, T> IntoDisk for &'a T @@ -39,10 +36,6 @@ where fn as_bytes(&self) -> Self::Bytes { T::as_bytes(*self) } - - fn into_ivec(&self) -> sled::IVec { - T::into_ivec(*self) - } } impl IntoDisk for Arc @@ -54,10 +47,6 @@ where fn as_bytes(&self) -> Self::Bytes { T::as_bytes(&*self) } - - fn into_ivec(&self) -> sled::IVec { - T::into_ivec(&*self) - } } /// Helper type for retrieving types from the disk with the correct format. @@ -69,15 +58,15 @@ pub trait FromDisk: Sized { /// # Panics /// /// - if the input data doesn't deserialize correctly - fn from_ivec(bytes: sled::IVec) -> Self; + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self; } impl FromDisk for Arc where T: FromDisk, { - fn from_ivec(bytes: sled::IVec) -> Self { - Arc::new(T::from_ivec(bytes)) + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { + Arc::new(T::from_bytes(bytes)) } } @@ -88,14 +77,10 @@ impl IntoDisk for Block { self.zcash_serialize_to_vec() .expect("serialization to vec doesn't fail") } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().into() - } } impl FromDisk for Block { - fn from_ivec(bytes: sled::IVec) -> Self { + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { Block::zcash_deserialize(bytes.as_ref()) .expect("deserialization format should match the serialization format used by IntoSled") } @@ -115,14 +100,11 @@ impl IntoDisk for TransactionLocation { bytes } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl FromDisk for TransactionLocation { - fn from_ivec(sled_bytes: sled::IVec) -> Self { + fn from_bytes(sled_bytes: impl AsRef<[u8]>) -> Self { + let sled_bytes = sled_bytes.as_ref(); let height = { let mut bytes = [0; 4]; bytes.copy_from_slice(&sled_bytes[0..4]); @@ -146,10 +128,6 @@ impl IntoDisk for transaction::Hash { fn as_bytes(&self) -> Self::Bytes { self.0 } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl IntoDisk for block::Hash { @@ -158,13 +136,10 @@ impl IntoDisk for block::Hash { fn as_bytes(&self) -> Self::Bytes { self.0 } - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl FromDisk for block::Hash { - fn from_ivec(bytes: sled::IVec) -> Self { + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { let array = bytes.as_ref().try_into().unwrap(); Self(array) } @@ -176,10 +151,6 @@ impl IntoDisk for sprout::Nullifier { fn as_bytes(&self) -> Self::Bytes { self.0 } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl IntoDisk for sapling::Nullifier { @@ -188,10 +159,6 @@ impl IntoDisk for sapling::Nullifier { fn as_bytes(&self) -> Self::Bytes { self.0 } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl IntoDisk for () { @@ -200,10 +167,6 @@ impl IntoDisk for () { fn as_bytes(&self) -> Self::Bytes { [] } - - fn into_ivec(&self) -> sled::IVec { - sled::IVec::default() - } } impl IntoDisk for block::Height { @@ -212,13 +175,10 @@ impl IntoDisk for block::Height { fn as_bytes(&self) -> Self::Bytes { self.0.to_be_bytes() } - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().as_ref().into() - } } impl FromDisk for block::Height { - fn from_ivec(bytes: sled::IVec) -> Self { + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { let array = bytes.as_ref().try_into().unwrap(); block::Height(u32::from_be_bytes(array)) } @@ -231,15 +191,11 @@ impl IntoDisk for transparent::Output { self.zcash_serialize_to_vec() .expect("serialization to vec doesn't fail") } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().into() - } } impl FromDisk for transparent::Output { - fn from_ivec(bytes: sled::IVec) -> Self { - Self::zcash_deserialize(&*bytes) + fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { + Self::zcash_deserialize(bytes.as_ref()) .expect("deserialization format should match the serialization format used by IntoSled") } } @@ -251,51 +207,27 @@ impl IntoDisk for transparent::OutPoint { self.zcash_serialize_to_vec() .expect("serialization to vec doesn't fail") } - - fn into_ivec(&self) -> sled::IVec { - self.as_bytes().into() - } } /// Helper trait for inserting (Key, Value) pairs into sled with a consistently /// defined format pub trait DiskSerialize { /// Serialize and insert the given key and value into a sled tree. - fn zs_insert( - &self, - key: K, - value: V, - ) -> Result<(), sled::transaction::UnabortableTransactionError> + fn zs_insert(&mut self, cf: &rocksdb::ColumnFamily, key: K, value: V) where K: IntoDisk + Debug, V: IntoDisk; } -impl DiskSerialize for sled::transaction::TransactionalTree { - fn zs_insert( - &self, - key: K, - value: V, - ) -> Result<(), sled::transaction::UnabortableTransactionError> +impl DiskSerialize for rocksdb::WriteBatch { + fn zs_insert(&mut self, cf: &rocksdb::ColumnFamily, key: K, value: V) where K: IntoDisk + Debug, V: IntoDisk, { - use std::any::type_name; - - let key_bytes = key.into_ivec(); - let value_bytes = value.into_ivec(); - let previous = self.insert(key_bytes, value_bytes)?; - - assert!( - previous.is_none(), - "duplicate key: previous value for key {:?} was not none when inserting into ({}, {}) sled Tree", - key, - type_name::(), - type_name::() - ); - - Ok(()) + let key_bytes = key.as_bytes(); + let value_bytes = value.as_bytes(); + self.put_cf(cf, key_bytes, value_bytes); } } @@ -304,14 +236,14 @@ impl DiskSerialize for sled::transaction::TransactionalTree { pub trait DiskDeserialize { /// Serialize the given key and use that to get and deserialize the /// corresponding value from a sled tree, if it is present. - fn zs_get(&self, key: &K) -> Option + fn zs_get(&self, cf: &rocksdb::ColumnFamily, key: &K) -> Option where K: IntoDisk, V: FromDisk; } -impl DiskDeserialize for sled::Tree { - fn zs_get(&self, key: &K) -> Option +impl DiskDeserialize for rocksdb::DB { + fn zs_get(&self, cf: &rocksdb::ColumnFamily, key: &K) -> Option where K: IntoDisk, V: FromDisk, @@ -319,10 +251,10 @@ impl DiskDeserialize for sled::Tree { let key_bytes = key.as_bytes(); let value_bytes = self - .get(key_bytes) + .get_pinned_cf(cf, key_bytes) .expect("expected that sled errors would not occur"); - value_bytes.map(V::from_ivec) + value_bytes.map(V::from_bytes) } } @@ -347,8 +279,8 @@ mod tests { where T: IntoDisk + FromDisk, { - let bytes = input.into_ivec(); - T::from_ivec(bytes) + let bytes = input.as_bytes(); + T::from_bytes(bytes) } fn assert_round_trip(input: T) @@ -364,8 +296,8 @@ mod tests { where T: IntoDisk + FromDisk, { - let bytes = input.into_ivec(); - T::from_ivec(bytes) + let bytes = input.as_bytes(); + T::from_bytes(bytes) } fn assert_round_trip_ref(input: &T) @@ -381,8 +313,8 @@ mod tests { where T: IntoDisk + FromDisk, { - let bytes = input.into_ivec(); - T::from_ivec(bytes) + let bytes = input.as_bytes(); + T::from_bytes(bytes) } fn assert_round_trip_arc(input: Arc) @@ -417,7 +349,7 @@ mod tests { T: IntoDisk + Clone, { let before = input.clone(); - let ivec = input.into_ivec(); + let ivec = input.as_bytes(); assert_eq!(before.as_bytes().as_ref(), ivec.as_ref()); } From fe65f54a279fad09d44234786902fe15b47d8d11 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 15:19:13 -0800 Subject: [PATCH 02/18] start fixing tests --- zebra-state/src/config.rs | 47 ++++++++++++++++++---- zebra-state/src/service/finalized_state.rs | 41 ++++++++----------- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/zebra-state/src/config.rs b/zebra-state/src/config.rs index 65ddfb8069f..ad29dd90077 100644 --- a/zebra-state/src/config.rs +++ b/zebra-state/src/config.rs @@ -1,5 +1,8 @@ use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::{ + path::PathBuf, + sync::atomic::{AtomicUsize, Ordering}, +}; use zebra_chain::parameters::Network; /// Configuration for the state service. @@ -54,6 +57,35 @@ pub struct Config { pub debug_stop_at_height: Option, } +fn gen_temp_path() -> PathBuf { + use std::time::SystemTime; + + static SALT_COUNTER: AtomicUsize = AtomicUsize::new(0); + + let seed = SALT_COUNTER.fetch_add(1, Ordering::SeqCst) as u128; + + let now = SystemTime::now() + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_nanos() + << 48; + + #[cfg(not(miri))] + let pid = u128::from(std::process::id()); + + #[cfg(miri)] + let pid = 0; + + let salt = (pid << 16) + now + seed; + + if cfg!(target_os = "linux") { + // use shared memory for temporary linux files + format!("/dev/shm/pagecache.tmp.{}", salt).into() + } else { + std::env::temp_dir().join(format!("pagecache.tmp.{}", salt)) + } +} + impl Config { pub(crate) fn open_db(&self, network: Network) -> rocksdb::DB { let net_dir = match network { @@ -76,17 +108,16 @@ impl Config { opts.create_if_missing(true); opts.create_missing_column_families(true); - if self.ephemeral { - todo!(); + let path = if self.ephemeral { + gen_temp_path() } else { - let path = self - .cache_dir + self.cache_dir .join("state") .join(format!("v{}", crate::constants::SLED_FORMAT_VERSION)) - .join(net_dir); + .join(net_dir) + }; - rocksdb::DB::open_cf_descriptors(&opts, path, cfs).unwrap() - } + rocksdb::DB::open_cf_descriptors(&opts, path, cfs).unwrap() } /// Construct a config for an ephemeral in memory database diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 3a3506972f4..e7c10558d64 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -1,4 +1,4 @@ -//! The primary implementation of the `zebra_state::Service` built upon sled +//! The primary implementation of the `zebra_state::Service` built upon rocksdb mod disk_format; @@ -28,7 +28,7 @@ use super::QueuedBlock; /// implemented as ordinary methods on the [`FinalizedState`]. The asynchronous /// methods are not implemented using `async fn`, but using normal methods that /// return `impl Future`. This allows them to move data (e.g., -/// clones of handles for [`sled::Tree`]s) into the futures they return. +/// clones of handles for the database) into the futures they return. /// /// This means that the returned futures have a `'static` lifetime and don't /// borrow any resources from the [`FinalizedState`], and the actual database work is @@ -40,6 +40,7 @@ pub struct FinalizedState { max_queued_height: i64, db: rocksdb::DB, + temporary: bool, /// Commit blocks to the finalized state up to this height, then exit Zebra. debug_stop_at_height: Option, } @@ -52,6 +53,7 @@ impl FinalizedState { queued_by_prev_hash: HashMap::new(), max_queued_height: -1, db, + temporary: config.ephemeral, debug_stop_at_height: config.debug_stop_at_height.map(block::Height), }; @@ -87,20 +89,8 @@ impl FinalizedState { new_state } - /// Synchronously flushes all dirty IO buffers and calls fsync. - /// - /// Returns the number of bytes flushed during this call. - /// See sled's `Tree.flush` for more details. - pub fn flush(&self) -> sled::Result { - todo!() - } - - /// If `block_height` is greater than or equal to the configured stop height, - /// stop the process. - /// - /// Flushes sled trees before exiting. - /// - /// `called_from` and `block_hash` are used for assertions and logging. + /// Stop the process if `block_height` is greater than or equal to the + /// configured stop height. fn is_at_stop_height(&self, block_height: block::Height) -> bool { let debug_stop_at_height = match self.debug_stop_at_height { Some(debug_stop_at_height) => debug_stop_at_height, @@ -271,15 +261,6 @@ impl FinalizedState { let result = self.db.write(batch).map(|()| hash); if result.is_ok() && self.is_at_stop_height(height) { - if let Err(e) = self.flush() { - tracing::error!( - ?e, - ?height, - ?hash, - "error flushing sled state before stopping" - ); - } - tracing::info!(?height, ?hash, "stopping at configured height"); std::process::exit(0); @@ -357,6 +338,16 @@ impl FinalizedState { } } +impl Drop for FinalizedState { + fn drop(&mut self) { + if self.temporary { + let path = self.db.path(); + tracing::debug!("removing temporary database files {:?}", path); + let _res = std::fs::remove_dir_all(path); + } + } +} + fn block_precommit_metrics(hash: &block::Hash, height: block::Height, block: &Block) { let transaction_count = block.transactions.len(); let transparent_prevout_count = block From a2acaa7f3c60847b477bd516901a76fc703f08a0 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 15:42:58 -0800 Subject: [PATCH 03/18] fix the commit and stop logic --- zebra-state/src/service/finalized_state.rs | 101 +++++++++++---------- zebrad/tests/acceptance.rs | 2 +- 2 files changed, 54 insertions(+), 49 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index e7c10558d64..d50acbf7342 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -163,7 +163,7 @@ impl FinalizedState { let hash = block.hash(); block_precommit_metrics(&hash, height, &block); - tracing::trace!(?height, ?hash, "Finalized block"); + tracing::info!(?height, ?hash, "Finalized block"); let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); @@ -201,62 +201,67 @@ impl FinalizedState { ); } - let mut batch = rocksdb::WriteBatch::default(); + let prepare_commit = || -> rocksdb::WriteBatch { + let mut batch = rocksdb::WriteBatch::default(); - // Index the block - batch.zs_insert(hash_by_height, height, hash); - batch.zs_insert(height_by_hash, hash, height); - batch.zs_insert(block_by_height, height, &block); + // Index the block + batch.zs_insert(hash_by_height, height, hash); + batch.zs_insert(height_by_hash, hash, height); + batch.zs_insert(block_by_height, height, &block); - // TODO: sprout and sapling anchors (per block) + // TODO: sprout and sapling anchors (per block) - // Consensus-critical bug in zcashd: transactions in the - // genesis block are ignored. - if block.header.previous_block_hash == block::Hash([0; 32]) { - let result = self.db.write(batch).map(|()| hash); - return result.map_err(Into::into); - } + // Consensus-critical bug in zcashd: transactions in the + // genesis block are ignored. + if block.header.previous_block_hash == block::Hash([0; 32]) { + return batch; + } - // Index each transaction - for (transaction_index, transaction) in block.transactions.iter().enumerate() { - let transaction_hash = transaction.hash(); - let transaction_location = TransactionLocation { - height, - index: transaction_index - .try_into() - .expect("no more than 4 billion transactions per block"), - }; - batch.zs_insert(tx_by_hash, transaction_hash, transaction_location); - - // Mark all transparent inputs as spent - for input in transaction.inputs() { - match input { - transparent::Input::PrevOut { outpoint, .. } => { - batch.delete_cf(utxo_by_outpoint, outpoint.as_bytes()); + // Index each transaction + for (transaction_index, transaction) in block.transactions.iter().enumerate() { + let transaction_hash = transaction.hash(); + let transaction_location = TransactionLocation { + height, + index: transaction_index + .try_into() + .expect("no more than 4 billion transactions per block"), + }; + batch.zs_insert(tx_by_hash, transaction_hash, transaction_location); + + // Mark all transparent inputs as spent + for input in transaction.inputs() { + match input { + transparent::Input::PrevOut { outpoint, .. } => { + batch.delete_cf(utxo_by_outpoint, outpoint.as_bytes()); + } + // Coinbase inputs represent new coins, + // so there are no UTXOs to mark as spent. + transparent::Input::Coinbase { .. } => {} } - // Coinbase inputs represent new coins, - // so there are no UTXOs to mark as spent. - transparent::Input::Coinbase { .. } => {} } - } - // Index all new transparent outputs - for (index, output) in transaction.outputs().iter().enumerate() { - let outpoint = transparent::OutPoint { - hash: transaction_hash, - index: index as _, - }; - batch.zs_insert(utxo_by_outpoint, outpoint, output); - } + // Index all new transparent outputs + for (index, output) in transaction.outputs().iter().enumerate() { + let outpoint = transparent::OutPoint { + hash: transaction_hash, + index: index as _, + }; + batch.zs_insert(utxo_by_outpoint, outpoint, output); + } - // Mark sprout and sapling nullifiers as spent - for sprout_nullifier in transaction.sprout_nullifiers() { - batch.zs_insert(sprout_nullifiers, sprout_nullifier, ()); - } - for sapling_nullifier in transaction.sapling_nullifiers() { - batch.zs_insert(sapling_nullifiers, sapling_nullifier, ()); + // Mark sprout and sapling nullifiers as spent + for sprout_nullifier in transaction.sprout_nullifiers() { + batch.zs_insert(sprout_nullifiers, sprout_nullifier, ()); + } + for sapling_nullifier in transaction.sapling_nullifiers() { + batch.zs_insert(sapling_nullifiers, sapling_nullifier, ()); + } } - } + + batch + }; + + let batch = prepare_commit(); let result = self.db.write(batch).map(|()| hash); diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 742fb1ebd0f..3ab016084ba 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -493,7 +493,7 @@ const STOP_AT_HEIGHT_REGEX: &str = "stopping at configured height"; const STOP_ON_LOAD_TIMEOUT: Duration = Duration::from_secs(5); // usually it's much shorter than this -const SMALL_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(30); +const SMALL_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(120); const LARGE_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(180); /// Test if `zebrad` can sync the first checkpoint on mainnet. From 1ce8adbf84bd8239fa1f88ae19223daccc732344 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 15:49:18 -0800 Subject: [PATCH 04/18] remove all references to sled from the codebase --- Cargo.lock | 97 +------------------ .../0004-asynchronous-script-verification.md | 24 ++--- book/src/dev/rfcs/0005-state-updates.md | 52 +++++----- zebra-state/Cargo.toml | 1 - zebra-state/src/config.rs | 15 +-- zebra-state/src/constants.rs | 2 +- .../service/finalized_state/disk_format.rs | 40 ++++---- zebrad/tests/acceptance.rs | 6 +- 8 files changed, 67 insertions(+), 170 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c1851f7834a..d7b3f4d53fe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -463,15 +463,6 @@ dependencies = [ "bitflags", ] -[[package]] -name = "cloudabi" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4344512281c643ae7638bbabc3af17a11307803ec8f0fcad9fae512a8bf36467" -dependencies = [ - "bitflags", -] - [[package]] name = "color-backtrace" version = "0.3.0" @@ -939,16 +930,6 @@ dependencies = [ "percent-encoding", ] -[[package]] -name = "fs2" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9564fc758e15025b46aa6643b1b77d047d1a56a1aea6e01002ac0c7026876213" -dependencies = [ - "libc", - "winapi 0.3.9", -] - [[package]] name = "fuchsia-cprng" version = "0.1.1" @@ -1072,15 +1053,6 @@ dependencies = [ "slab", ] -[[package]] -name = "fxhash" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c31b6d751ae2c7f11320402d34e41349dd1016f8d5d45e48c4312bc8625af50c" -dependencies = [ - "byteorder", -] - [[package]] name = "generational-arena" version = "0.2.8" @@ -1390,15 +1362,6 @@ dependencies = [ "str_stack", ] -[[package]] -name = "instant" -version = "0.1.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb1fc4429a33e1f80d41dc9fea4d108a88bec1de8053878898ae448a0b52f613" -dependencies = [ - "cfg-if 1.0.0", -] - [[package]] name = "iovec" version = "0.1.4" @@ -1503,15 +1466,6 @@ dependencies = [ "scopeguard", ] -[[package]] -name = "lock_api" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "28247cc5a5be2f05fbcd76dd0cf2c7d3b5400cb978a28042abcd4fa0b3f8261c" -dependencies = [ - "scopeguard", -] - [[package]] name = "log" version = "0.4.11" @@ -1660,7 +1614,7 @@ dependencies = [ "metrics-observer-prometheus", "metrics-observer-yaml", "metrics-util", - "parking_lot 0.10.2", + "parking_lot", "quanta", ] @@ -1890,19 +1844,8 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3a704eb390aafdc107b0e392f56a82b668e3a71366993b5340f5833fd62505e" dependencies = [ - "lock_api 0.3.4", - "parking_lot_core 0.7.2", -] - -[[package]] -name = "parking_lot" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a4893845fa2ca272e647da5d0e46660a314ead9c2fdd9a883aabc32e481a8733" -dependencies = [ - "instant", - "lock_api 0.4.1", - "parking_lot_core 0.8.0", + "lock_api", + "parking_lot_core", ] [[package]] @@ -1912,22 +1855,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d58c7c768d4ba344e3e8d72518ac13e259d7c7ade24167003b8488e10b6740a3" dependencies = [ "cfg-if 0.1.10", - "cloudabi 0.0.3", - "libc", - "redox_syscall", - "smallvec 1.4.2", - "winapi 0.3.9", -] - -[[package]] -name = "parking_lot_core" -version = "0.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c361aa727dd08437f2f1447be8b59a33b0edd15e0fcee698f935613d9efbca9b" -dependencies = [ - "cfg-if 0.1.10", - "cloudabi 0.1.0", - "instant", + "cloudabi", "libc", "redox_syscall", "smallvec 1.4.2", @@ -2620,22 +2548,6 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c111b5bd5695e56cffe5129854aa230b39c93a305372fdbb2668ca2394eea9f8" -[[package]] -name = "sled" -version = "0.34.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d0132f3e393bcb7390c60bb45769498cf4550bcb7a21d7f95c02b69f6362cdc" -dependencies = [ - "crc32fast", - "crossbeam-epoch 0.9.0", - "crossbeam-utils 0.8.0", - "fs2", - "fxhash", - "libc", - "log", - "parking_lot 0.11.0", -] - [[package]] name = "smallvec" version = "0.6.13" @@ -3504,7 +3416,6 @@ dependencies = [ "proptest", "rocksdb", "serde", - "sled", "spandoc", "tempdir", "thiserror", diff --git a/book/src/dev/rfcs/0004-asynchronous-script-verification.md b/book/src/dev/rfcs/0004-asynchronous-script-verification.md index ff8074761ea..0c3cf97066f 100644 --- a/book/src/dev/rfcs/0004-asynchronous-script-verification.md +++ b/book/src/dev/rfcs/0004-asynchronous-script-verification.md @@ -97,11 +97,11 @@ with a timeout error. Currently, we outsource script verification to Implementing the state request correctly requires considering two sets of behaviors: 1. behaviors related to the state's external API (a `Buffer`ed `tower::Service`); -2. behaviors related to the state's internal implementation (using `sled`). +2. behaviors related to the state's internal implementation (using `rocksdb`). Making this distinction helps us to ensure we don't accidentally leak "internal" behaviors into "external" behaviors, which would violate -encapsulation and make it more difficult to replace `sled`. +encapsulation and make it more difficult to replace `rocksdb`. In the first category, our state is presented to the rest of the application as a `Buffer`ed `tower::Service`. The `Buffer` wrapper allows shared access @@ -116,19 +116,19 @@ This means that our external API ensures that the state service sees a linearized sequence of state requests, although the exact ordering is unpredictable when there are multiple senders making requests. -In the second category, the Sled API presents itself synchronously, but +In the second category, the rocksdb API presents itself synchronously, but database and tree handles are clonable and can be moved between threads. All that's required to process some request asynchronously is to clone the appropriate handle, move it into an async block, and make the call as part of the future. (We might want to use Tokio's blocking API for this, but that's a side detail). -Because the state service has exclusive access to the sled database, and the +Because the state service has exclusive access to the rocksdb database, and the state service sees a linearized sequence of state requests, we have an easy -way to opt in to asynchronous database access. We can perform sled operations +way to opt in to asynchronous database access. We can perform rocskdb operations synchronously in the `Service::call`, waiting for them to complete, and be -sure that all future requests will see the resulting sled state. Or, we can -perform sled operations asynchronously in the future returned by +sure that all future requests will see the resulting rocskdb state. Or, we can +perform rocskdb operations asynchronously in the future returned by `Service::call`. If we perform all *writes* synchronously and allow reads to be either @@ -139,7 +139,7 @@ time the request was processed, or a later state. Now, returning to the UTXO lookup problem, we can map out the possible states with this restriction in mind. This description assumes that UTXO storage is split into disjoint sets, one in-memory (e.g., blocks after the reorg limit) -and the other in sled (e.g., blocks after the reorg limit). The details of +and the other in rocskdb (e.g., blocks after the reorg limit). The details of this storage are not important for this design, only that the two sets are disjoint. @@ -147,14 +147,14 @@ When the state service processes a `Request::AwaitUtxo(OutPoint)` referencing some UTXO `u`, there are three disjoint possibilities: 1. `u` is already contained in an in-memory block storage; -2. `u` is already contained in the sled UTXO set; +2. `u` is already contained in the rocskdb UTXO set; 3. `u` is not yet known to the state service. In case 3, we need to queue `u` and scan all *future* blocks to see whether they contain `u`. However, if we have a mechanism to queue `u`, we can perform check 2 asynchronously, because restricting to synchronous writes means that any async read will return the current or later state. If `u` was -in the sled UTXO set when the request was processed, the only way that an +in the rocskdb UTXO set when the request was processed, the only way that an async read would not return `u` is if the UTXO were spent, in which case the service is not required to return a response. @@ -184,12 +184,12 @@ The state service should maintain an `Arc>`, used as follows 1. In `Service::call(Request::AwaitUtxo(u))`, the service should: - call `PendingUtxos::queue(u)` to get a future `f` to return to the caller; - spawn a task that does a sled lookup for `u`, calling `PendingUtxos::respond(u, output)` if present; + spawn a task that does a rocskdb lookup for `u`, calling `PendingUtxos::respond(u, output)` if present; - check the in-memory storage for `u`, calling `PendingUtxos::respond(u, output)` if present; - return `f` to the caller (it may already be ready). The common case is that `u` references an old UTXO, so spawning the lookup task first means that we don't wait to check in-memory storage for `u` - before starting the sled lookup. + before starting the rocskdb lookup. 2. In `Service::call(Request::CommitBlock(block, ..))`, the service should: - call `PendingUtxos::check_block(block.as_ref())`; diff --git a/book/src/dev/rfcs/0005-state-updates.md b/book/src/dev/rfcs/0005-state-updates.md index 3d634ab33a4..efade58ceda 100644 --- a/book/src/dev/rfcs/0005-state-updates.md +++ b/book/src/dev/rfcs/0005-state-updates.md @@ -156,7 +156,7 @@ state data at the finality boundary provided by the reorg limit. State data from blocks *above* the reorg limit (*non-finalized state*) is stored in-memory and handles multiple chains. State data from blocks *below* -the reorg limit (*finalized state*) is stored persistently using `sled` and +the reorg limit (*finalized state*) is stored persistently using `rocksdb` and only tracks a single chain. This allows a simplification of our state handling, because only finalized data is persistent and the logic for finalized data handles less invariants. @@ -169,7 +169,7 @@ Another downside of this design is that we do not achieve exactly the same behavior as `zcashd` in the event of a 51% attack: `zcashd` limits *each* chain reorganization to 100 blocks, but permits multiple reorgs, while Zebra limits *all* chain reorgs to 100 blocks. In the event of a successful 51% attack on -Zcash, this could be resolved by wiping the Sled state and re-syncing the new +Zcash, this could be resolved by wiping the rocksdb state and re-syncing the new chain, but in this scenario there are worse problems. ## Service Interface @@ -180,11 +180,11 @@ Determining what guarantees the state service can and should provide to the rest of the application requires considering two sets of behaviors: 1. behaviors related to the state's external API (a `Buffer`ed `tower::Service`); -2. behaviors related to the state's internal implementation (using `sled`). +2. behaviors related to the state's internal implementation (using `rocksdb`). Making this distinction helps us to ensure we don't accidentally leak "internal" behaviors into "external" behaviors, which would violate -encapsulation and make it more difficult to replace `sled`. +encapsulation and make it more difficult to replace `rocksdb`. In the first category, our state is presented to the rest of the application as a `Buffer`ed `tower::Service`. The `Buffer` wrapper allows shared access @@ -199,19 +199,19 @@ This means that our external API ensures that the state service sees a linearized sequence of state requests, although the exact ordering is unpredictable when there are multiple senders making requests. -In the second category, the Sled API presents itself synchronously, but +In the second category, the rocksdb API presents itself synchronously, but database and tree handles are cloneable and can be moved between threads. All that's required to process some request asynchronously is to clone the appropriate handle, move it into an async block, and make the call as part of the future. (We might want to use Tokio's blocking API for this, but this is an implementation detail). -Because the state service has exclusive access to the sled database, and the +Because the state service has exclusive access to the rocksdb database, and the state service sees a linearized sequence of state requests, we have an easy -way to opt in to asynchronous database access. We can perform sled operations +way to opt in to asynchronous database access. We can perform rocksdb operations synchronously in the `Service::call`, waiting for them to complete, and be -sure that all future requests will see the resulting sled state. Or, we can -perform sled operations asynchronously in the future returned by +sure that all future requests will see the resulting rocksdb state. Or, we can +perform rocksdb operations asynchronously in the future returned by `Service::call`. If we perform all *writes* synchronously and allow reads to be either @@ -221,10 +221,10 @@ time the request was processed, or a later state. ### Summary -- **Sled reads** may be done synchronously (in `call`) or asynchronously (in +- **rocksdb reads** may be done synchronously (in `call`) or asynchronously (in the `Future`), depending on the context; -- **Sled writes** must be done synchronously (in `call`) +- **rocksdb writes** must be done synchronously (in `call`) ## In-memory data structures [in-memory]: #in-memory @@ -580,22 +580,22 @@ New `non-finalized` blocks are commited as follows: - Remove the lowest height block from the non-finalized state with `self.mem.finalize();` - Commit that block to the finalized state with - `self.sled.commit_finalized_direct(finalized);` + `self.disk.commit_finalized_direct(finalized);` 8. Prune orphaned blocks from `self.queued_blocks` with `self.queued_blocks.prune_by_height(finalized_height);` 9. Return the receiver for the block's channel -## Sled data structures -[sled]: #sled +## rocksdb data structures +[rocksdb]: #rocksdb -Sled provides a persistent, thread-safe `BTreeMap<&[u8], &[u8]>`. Each map is +rocksdb provides a persistent, thread-safe `BTreeMap<&[u8], &[u8]>`. Each map is a distinct "tree". Keys are sorted using lex order on byte strings, so integer values should be stored using big-endian encoding (so that the lex order on byte strings is the numeric ordering). -We use the following Sled trees: +We use the following rocksdb column families: | Tree | Keys | Values | |----------------------|-----------------------|-------------------------------------| @@ -613,16 +613,16 @@ Zcash structures are encoded using `ZcashSerialize`/`ZcashDeserialize`. **Note:** We do not store the cumulative work for the finalized chain, because the finalized work is equal for all non-finalized chains. So the additional non-finalized work can be used to calculate the relative chain order, and choose the best chain. -### Notes on Sled trees +### Notes on rocksdb column families -- The `hash_by_height` and `height_by_hash` trees provide a bijection between - block heights and block hashes. (Since the Sled state only stores finalized +- The `hash_by_height` and `height_by_hash` column families provide a bijection between + block heights and block hashes. (Since the rocksdb state only stores finalized state, they are actually a bijection). -- The `block_by_height` tree provides a bijection between block heights and block - data. There is no corresponding `height_by_block` tree: instead, hash the block, - and use `height_by_hash`. (Since the Sled state only stores finalized state, - they are actually a bijection). +- The `block_by_height` column family provides a bijection between block + heights and block data. There is no corresponding `height_by_block` column + family: instead, hash the block, and use `height_by_hash`. (Since the + rocksdb state only stores finalized state, they are actually a bijection). - Blocks are stored by height, not by hash. This has the downside that looking up a block by hash requires an extra level of indirection. The upside is @@ -630,7 +630,7 @@ Zcash structures are encoded using `ZcashSerialize`/`ZcashDeserialize`. common access patterns, such as helping a client sync the chain or doing analysis, access blocks in (potentially sparse) height order. In addition, the fact that we commit blocks in order means we're writing only to the end - of the Sled tree, which may help save space. + of the rocksdb column family, which may help save space. - Transaction references are stored as a `(height, index)` pair referencing the height of the transaction's parent block and the transaction's index in that @@ -645,7 +645,7 @@ commit any queued children. (Although the checkpointer generates verified blocks in order when it completes a checkpoint, the blocks are committed in the response futures, so they may arrive out of order). -Committing a block to the sled state should be implemented as a wrapper around +Committing a block to the rocksdb state should be implemented as a wrapper around a function also called by [`Request::CommitBlock`](#request-commit-block), which should: @@ -754,7 +754,7 @@ CommitFinalizedBlock { } ``` -Commits a finalized block to the sled state, skipping contextual validation. +Commits a finalized block to the rocksdb state, skipping contextual validation. This is exposed for use in checkpointing, which produces in-order finalized blocks. Returns `Response::Added(block::Hash)` with the hash of the committed block if successful. diff --git a/zebra-state/Cargo.toml b/zebra-state/Cargo.toml index 724b1d36a59..4fb34e566ea 100644 --- a/zebra-state/Cargo.toml +++ b/zebra-state/Cargo.toml @@ -15,7 +15,6 @@ dirs = "3.0.1" hex = "0.4.2" lazy_static = "1.4.0" serde = { version = "1", features = ["serde_derive"] } -sled = "0.34.5" futures = "0.3.7" metrics = "0.12" diff --git a/zebra-state/src/config.rs b/zebra-state/src/config.rs index ad29dd90077..3a437c1ba9c 100644 --- a/zebra-state/src/config.rs +++ b/zebra-state/src/config.rs @@ -30,18 +30,6 @@ pub struct Config { /// | Other | `std::env::current_dir()/cache` | | pub cache_dir: PathBuf, - /// Controls the size of the database cache, in bytes. - /// - /// This corresponds to `sled`'s [`cache_capacity`][cc] parameter. - /// Note that the behavior of this parameter is [somewhat - /// unintuitive][gh], measuring the on-disk size of the cached data, - /// not the in-memory size, which may be much larger, especially for - /// smaller keys and values. - /// - /// [cc]: https://docs.rs/sled/0.34.4/sled/struct.Config.html#method.cache_capacity - /// [gh]: https://github.com/spacejam/sled/issues/986#issuecomment-592950100 - pub memory_cache_bytes: u64, - /// Whether to use an ephemeral database. /// /// Ephemeral databases are stored in memory on Linux, and in a temporary directory on other OSes. @@ -113,7 +101,7 @@ impl Config { } else { self.cache_dir .join("state") - .join(format!("v{}", crate::constants::SLED_FORMAT_VERSION)) + .join(format!("v{}", crate::constants::DATABASE_FORMAT_VERSION)) .join(net_dir) }; @@ -136,7 +124,6 @@ impl Default for Config { Self { cache_dir, - memory_cache_bytes: 50_000_000, ephemeral: false, debug_stop_at_height: None, } diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index 960db6737cf..b9096533697 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -11,4 +11,4 @@ pub const MIN_TRASPARENT_COINBASE_MATURITY: u32 = 100; /// coinbase transactions. pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRASPARENT_COINBASE_MATURITY - 1; -pub const SLED_FORMAT_VERSION: u32 = 1; +pub const DATABASE_FORMAT_VERSION: u32 = 1; diff --git a/zebra-state/src/service/finalized_state/disk_format.rs b/zebra-state/src/service/finalized_state/disk_format.rs index 4917b9fee2b..1d40d4a9830 100644 --- a/zebra-state/src/service/finalized_state/disk_format.rs +++ b/zebra-state/src/service/finalized_state/disk_format.rs @@ -1,4 +1,4 @@ -//! Module defining exactly how to move types in and out of sled +//! Module defining exactly how to move types in and out of rocksdb use std::{convert::TryInto, fmt::Debug, sync::Arc}; use zebra_chain::{ @@ -82,7 +82,7 @@ impl IntoDisk for Block { impl FromDisk for Block { fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { Block::zcash_deserialize(bytes.as_ref()) - .expect("deserialization format should match the serialization format used by IntoSled") + .expect("deserialization format should match the serialization format used by IntoDisk") } } @@ -103,18 +103,18 @@ impl IntoDisk for TransactionLocation { } impl FromDisk for TransactionLocation { - fn from_bytes(sled_bytes: impl AsRef<[u8]>) -> Self { - let sled_bytes = sled_bytes.as_ref(); + fn from_bytes(disk_bytes: impl AsRef<[u8]>) -> Self { + let disk_bytes = disk_bytes.as_ref(); let height = { let mut bytes = [0; 4]; - bytes.copy_from_slice(&sled_bytes[0..4]); + bytes.copy_from_slice(&disk_bytes[0..4]); let height = u32::from_be_bytes(bytes); block::Height(height) }; let index = { let mut bytes = [0; 4]; - bytes.copy_from_slice(&sled_bytes[4..8]); + bytes.copy_from_slice(&disk_bytes[4..8]); u32::from_be_bytes(bytes) }; @@ -196,7 +196,7 @@ impl IntoDisk for transparent::Output { impl FromDisk for transparent::Output { fn from_bytes(bytes: impl AsRef<[u8]>) -> Self { Self::zcash_deserialize(bytes.as_ref()) - .expect("deserialization format should match the serialization format used by IntoSled") + .expect("deserialization format should match the serialization format used by IntoDisk") } } @@ -209,10 +209,10 @@ impl IntoDisk for transparent::OutPoint { } } -/// Helper trait for inserting (Key, Value) pairs into sled with a consistently +/// Helper trait for inserting (Key, Value) pairs into rocksdb with a consistently /// defined format pub trait DiskSerialize { - /// Serialize and insert the given key and value into a sled tree. + /// Serialize and insert the given key and value into a rocksdb column family. fn zs_insert(&mut self, cf: &rocksdb::ColumnFamily, key: K, value: V) where K: IntoDisk + Debug, @@ -231,11 +231,11 @@ impl DiskSerialize for rocksdb::WriteBatch { } } -/// Helper trait for retrieving values from sled trees with a consistently +/// Helper trait for retrieving values from rocksdb column familys with a consistently /// defined format pub trait DiskDeserialize { /// Serialize the given key and use that to get and deserialize the - /// corresponding value from a sled tree, if it is present. + /// corresponding value from a rocksdb column family, if it is present. fn zs_get(&self, cf: &rocksdb::ColumnFamily, key: &K) -> Option where K: IntoDisk, @@ -252,7 +252,7 @@ impl DiskDeserialize for rocksdb::DB { let value_bytes = self .get_pinned_cf(cf, key_bytes) - .expect("expected that sled errors would not occur"); + .expect("expected that disk errors would not occur"); value_bytes.map(V::from_bytes) } @@ -326,9 +326,9 @@ mod tests { assert_eq!(*before, after); } - /// The round trip test covers types that are used as value field in a sled - /// Tree. Only these types are ever deserialized, and so they're the only - /// ones that implement both `IntoSled` and `FromSled`. + /// The round trip test covers types that are used as value field in a rocksdb + /// column family. Only these types are ever deserialized, and so they're the only + /// ones that implement both `IntoDisk` and `FromDisk`. fn assert_value_properties(input: T) where T: IntoDisk + FromDisk + Clone + PartialEq + std::fmt::Debug, @@ -338,12 +338,12 @@ mod tests { assert_round_trip(input); } - /// This test asserts that types that are used as sled keys behave correctly. - /// Any type that implements `IntoIVec` can be used as a sled key. The value - /// is serialized via `IntoSled::into_ivec` when the `key`, `value` pair is - /// inserted into the sled tree. The `as_bytes` impl on the other hand is + /// This test asserts that types that are used as rocksdb keys behave correctly. + /// Any type that implements `IntoIVec` can be used as a rocksdb key. The value + /// is serialized via `IntoDisk::into_ivec` when the `key`, `value` pair is + /// inserted into the rocksdb column family. The `as_bytes` impl on the other hand is /// called for most other operations when comparing a key against existing - /// keys in the sled database, such as `contains`. + /// keys in the rocksdb database, such as `contains`. fn assert_as_bytes_matches_ivec(input: T) where T: IntoDisk + Clone, diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 3ab016084ba..18dc3eb3582 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -313,7 +313,7 @@ fn persistent_mode() -> Result<()> { // Make sure the command was killed output.assert_was_killed()?; - // Check that we have persistent sled database + // Check that we have persistent rocksdb database let cache_dir = testdir.path().join("state"); assert_with_context!(cache_dir.read_dir()?.count() > 0, &output); @@ -536,8 +536,8 @@ fn restart_stop_at_height() -> Result<()> { SMALL_CHECKPOINT_TIMEOUT, None, )?; - // if stopping corrupts the sled database, zebrad might hang here - // if stopping does not sync the sled database, the logs will contain OnCommit + // if stopping corrupts the rocksdb database, zebrad might hang here + // if stopping does not sync the rocksdb database, the logs will contain OnCommit sync_until( Height(0), Mainnet, From 0ebfe3169b70a5aa201f46d905e215a5e68378f2 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 16:39:49 -0800 Subject: [PATCH 05/18] pls help --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 12946c086cd..caf1698052d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM rust:buster as builder RUN apt-get update && \ apt-get install -y --no-install-recommends \ - make cmake g++ gcc llvm libclang-dev + make cmake g++ gcc llvm libclang-dev clang RUN mkdir /zebra WORKDIR /zebra From 5e9df2d553c9fa3b0579279fe404ac17ac056109 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 16:50:53 -0800 Subject: [PATCH 06/18] revert change to timeout --- zebrad/tests/acceptance.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 18dc3eb3582..bd382115ce0 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -493,7 +493,7 @@ const STOP_AT_HEIGHT_REGEX: &str = "stopping at configured height"; const STOP_ON_LOAD_TIMEOUT: Duration = Duration::from_secs(5); // usually it's much shorter than this -const SMALL_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(120); +const SMALL_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(30); const LARGE_CHECKPOINT_TIMEOUT: Duration = Duration::from_secs(180); /// Test if `zebrad` can sync the first checkpoint on mainnet. From a9f42770e0adc225af7ddbfa8613e497a38140f5 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:15:38 -0800 Subject: [PATCH 07/18] fix log level --- zebra-state/src/service/finalized_state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index d50acbf7342..e9ec1149c72 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -163,7 +163,7 @@ impl FinalizedState { let hash = block.hash(); block_precommit_metrics(&hash, height, &block); - tracing::info!(?height, ?hash, "Finalized block"); + tracing::trace!(?height, ?hash, "Finalized block"); let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); From 3e7b17a8a36de868b20f619870e6f6a7bf896842 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:22:38 -0800 Subject: [PATCH 08/18] fix typo --- .../0004-asynchronous-script-verification.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/book/src/dev/rfcs/0004-asynchronous-script-verification.md b/book/src/dev/rfcs/0004-asynchronous-script-verification.md index 0c3cf97066f..1156f2a7bad 100644 --- a/book/src/dev/rfcs/0004-asynchronous-script-verification.md +++ b/book/src/dev/rfcs/0004-asynchronous-script-verification.md @@ -125,10 +125,10 @@ side detail). Because the state service has exclusive access to the rocksdb database, and the state service sees a linearized sequence of state requests, we have an easy -way to opt in to asynchronous database access. We can perform rocskdb operations +way to opt in to asynchronous database access. We can perform rocksdb operations synchronously in the `Service::call`, waiting for them to complete, and be -sure that all future requests will see the resulting rocskdb state. Or, we can -perform rocskdb operations asynchronously in the future returned by +sure that all future requests will see the resulting rocksdb state. Or, we can +perform rocksdb operations asynchronously in the future returned by `Service::call`. If we perform all *writes* synchronously and allow reads to be either @@ -139,7 +139,7 @@ time the request was processed, or a later state. Now, returning to the UTXO lookup problem, we can map out the possible states with this restriction in mind. This description assumes that UTXO storage is split into disjoint sets, one in-memory (e.g., blocks after the reorg limit) -and the other in rocskdb (e.g., blocks after the reorg limit). The details of +and the other in rocksdb (e.g., blocks after the reorg limit). The details of this storage are not important for this design, only that the two sets are disjoint. @@ -147,14 +147,14 @@ When the state service processes a `Request::AwaitUtxo(OutPoint)` referencing some UTXO `u`, there are three disjoint possibilities: 1. `u` is already contained in an in-memory block storage; -2. `u` is already contained in the rocskdb UTXO set; +2. `u` is already contained in the rocksdb UTXO set; 3. `u` is not yet known to the state service. In case 3, we need to queue `u` and scan all *future* blocks to see whether they contain `u`. However, if we have a mechanism to queue `u`, we can perform check 2 asynchronously, because restricting to synchronous writes means that any async read will return the current or later state. If `u` was -in the rocskdb UTXO set when the request was processed, the only way that an +in the rocksdb UTXO set when the request was processed, the only way that an async read would not return `u` is if the UTXO were spent, in which case the service is not required to return a response. @@ -184,12 +184,12 @@ The state service should maintain an `Arc>`, used as follows 1. In `Service::call(Request::AwaitUtxo(u))`, the service should: - call `PendingUtxos::queue(u)` to get a future `f` to return to the caller; - spawn a task that does a rocskdb lookup for `u`, calling `PendingUtxos::respond(u, output)` if present; + spawn a task that does a rocksdb lookup for `u`, calling `PendingUtxos::respond(u, output)` if present; - check the in-memory storage for `u`, calling `PendingUtxos::respond(u, output)` if present; - return `f` to the caller (it may already be ready). The common case is that `u` references an old UTXO, so spawning the lookup task first means that we don't wait to check in-memory storage for `u` - before starting the rocskdb lookup. + before starting the rocksdb lookup. 2. In `Service::call(Request::CommitBlock(block, ..))`, the service should: - call `PendingUtxos::check_block(block.as_ref())`; From f5095754c5cc794a536e94421ef4d4dbad95ea56 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:25:02 -0800 Subject: [PATCH 09/18] move use statement to a relevant location --- zebra-state/src/config.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/zebra-state/src/config.rs b/zebra-state/src/config.rs index 3a437c1ba9c..720ffaa8e19 100644 --- a/zebra-state/src/config.rs +++ b/zebra-state/src/config.rs @@ -1,8 +1,5 @@ use serde::{Deserialize, Serialize}; -use std::{ - path::PathBuf, - sync::atomic::{AtomicUsize, Ordering}, -}; +use std::path::PathBuf; use zebra_chain::parameters::Network; /// Configuration for the state service. @@ -46,6 +43,7 @@ pub struct Config { } fn gen_temp_path() -> PathBuf { + use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::SystemTime; static SALT_COUNTER: AtomicUsize = AtomicUsize::new(0); From cb636d24ddbd7f5a29d1f5aa862f266c760c0da5 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:27:06 -0800 Subject: [PATCH 10/18] bump database version --- zebra-state/src/constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index b9096533697..29e0d02491e 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -11,4 +11,4 @@ pub const MIN_TRASPARENT_COINBASE_MATURITY: u32 = 100; /// coinbase transactions. pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRASPARENT_COINBASE_MATURITY - 1; -pub const DATABASE_FORMAT_VERSION: u32 = 1; +pub const DATABASE_FORMAT_VERSION: u32 = 2; From c23dd4cd76fc7388f67d6e69fe1e0f677defe933 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:28:20 -0800 Subject: [PATCH 11/18] remove old comment --- zebra-state/src/service/finalized_state.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index e9ec1149c72..911183760da 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -18,22 +18,6 @@ use self::disk_format::{DiskDeserialize, DiskSerialize, FromDisk, IntoDisk, Tran use super::QueuedBlock; /// The finalized part of the chain state, stored in the db. -/// -/// This structure has two categories of methods: -/// -/// - *synchronous* methods that perform writes to the db state; -/// - *asynchronous* methods that perform reads. -/// -/// For more on this distinction, see RFC5. The synchronous methods are -/// implemented as ordinary methods on the [`FinalizedState`]. The asynchronous -/// methods are not implemented using `async fn`, but using normal methods that -/// return `impl Future`. This allows them to move data (e.g., -/// clones of handles for the database) into the futures they return. -/// -/// This means that the returned futures have a `'static` lifetime and don't -/// borrow any resources from the [`FinalizedState`], and the actual database work is -/// performed asynchronously when the returned future is polled, not while it is -/// created. This is analogous to the way [`tower::Service::call`] works. pub struct FinalizedState { /// Queued blocks that arrived out of order, indexed by their parent block hash. queued_by_prev_hash: HashMap, From 37ac7e55b2342600c625c63a601c30e71df02a84 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:31:49 -0800 Subject: [PATCH 12/18] rename flag field in finalized state --- zebra-state/src/service/finalized_state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 911183760da..969518e416e 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -24,7 +24,7 @@ pub struct FinalizedState { max_queued_height: i64, db: rocksdb::DB, - temporary: bool, + ephemeral: bool, /// Commit blocks to the finalized state up to this height, then exit Zebra. debug_stop_at_height: Option, } @@ -37,7 +37,7 @@ impl FinalizedState { queued_by_prev_hash: HashMap::new(), max_queued_height: -1, db, - temporary: config.ephemeral, + ephemeral: config.ephemeral, debug_stop_at_height: config.debug_stop_at_height.map(block::Height), }; @@ -329,7 +329,7 @@ impl FinalizedState { impl Drop for FinalizedState { fn drop(&mut self) { - if self.temporary { + if self.ephemeral { let path = self.db.path(); tracing::debug!("removing temporary database files {:?}", path); let _res = std::fs::remove_dir_all(path); From b3b7360b069ead5b7ce6299ba104360abe6d29c8 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:34:28 -0800 Subject: [PATCH 13/18] remove redundant trace event --- zebra-state/src/service/finalized_state.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 969518e416e..52db7149e8c 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -147,7 +147,6 @@ impl FinalizedState { let hash = block.hash(); block_precommit_metrics(&hash, height, &block); - tracing::trace!(?height, ?hash, "Finalized block"); let hash_by_height = self.db.cf_handle("hash_by_height").unwrap(); let height_by_hash = self.db.cf_handle("height_by_hash").unwrap(); From b6cd590e87878f2ed0bde08e58c1fe7c04fcf877 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:36:05 -0800 Subject: [PATCH 14/18] add clarifying comment --- zebra-state/src/service/finalized_state.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index 52db7149e8c..a898cc956b4 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -184,6 +184,8 @@ impl FinalizedState { ); } + // We use a closure so we can use an early return for control flow in + // the genesis case let prepare_commit = || -> rocksdb::WriteBatch { let mut batch = rocksdb::WriteBatch::default(); From 70f4e3a08f1317b80c9cfd890d1d3802c1079256 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:39:16 -0800 Subject: [PATCH 15/18] more clarifying comments --- zebra-state/src/service/finalized_state.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/zebra-state/src/service/finalized_state.rs b/zebra-state/src/service/finalized_state.rs index a898cc956b4..2cd3c8e749c 100644 --- a/zebra-state/src/service/finalized_state.rs +++ b/zebra-state/src/service/finalized_state.rs @@ -328,6 +328,10 @@ impl FinalizedState { } } +// Drop isn't guaranteed to run, such as when we panic, or if someone stored +// their FinalizedState in a static, but it should be fine if we don't clean +// this up since the files are placed in the os temp dir and should be cleaned +// up automatically eventually. impl Drop for FinalizedState { fn drop(&mut self) { if self.ephemeral { From 41dc5f42c99c36e945e461c1379e3926c67c93fa Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:41:26 -0800 Subject: [PATCH 16/18] more clarifying comments --- zebra-state/src/service/finalized_state/disk_format.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/zebra-state/src/service/finalized_state/disk_format.rs b/zebra-state/src/service/finalized_state/disk_format.rs index 1d40d4a9830..c0fd94f860b 100644 --- a/zebra-state/src/service/finalized_state/disk_format.rs +++ b/zebra-state/src/service/finalized_state/disk_format.rs @@ -250,6 +250,9 @@ impl DiskDeserialize for rocksdb::DB { { let key_bytes = key.as_bytes(); + // We use `get_pinned_cf` to avoid taking ownership of the serialized + // format because we're going to deserialize it anyways, which avoids an + // extra copy let value_bytes = self .get_pinned_cf(cf, key_bytes) .expect("expected that disk errors would not occur"); From 5c4b6745e37520cef63c10c7bc7064668fa24f38 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:44:03 -0800 Subject: [PATCH 17/18] remove redundant test --- .../service/finalized_state/disk_format.rs | 63 ------------------- 1 file changed, 63 deletions(-) diff --git a/zebra-state/src/service/finalized_state/disk_format.rs b/zebra-state/src/service/finalized_state/disk_format.rs index c0fd94f860b..f3d150f8135 100644 --- a/zebra-state/src/service/finalized_state/disk_format.rs +++ b/zebra-state/src/service/finalized_state/disk_format.rs @@ -341,21 +341,6 @@ mod tests { assert_round_trip(input); } - /// This test asserts that types that are used as rocksdb keys behave correctly. - /// Any type that implements `IntoIVec` can be used as a rocksdb key. The value - /// is serialized via `IntoDisk::into_ivec` when the `key`, `value` pair is - /// inserted into the rocksdb column family. The `as_bytes` impl on the other hand is - /// called for most other operations when comparing a key against existing - /// keys in the rocksdb database, such as `contains`. - fn assert_as_bytes_matches_ivec(input: T) - where - T: IntoDisk + Clone, - { - let before = input.clone(); - let ivec = input.as_bytes(); - assert_eq!(before.as_bytes().as_ref(), ivec.as_ref()); - } - #[test] fn roundtrip_transaction_location() { zebra_test::init(); @@ -387,52 +372,4 @@ mod tests { proptest!(|(val in any::())| assert_value_properties(val)); } - - #[test] - fn key_matches_ivec_transaction_location() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_trans_hash() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_block_hash() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_sprout_nullifier() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_sapling_nullifier() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_block_height() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_transparent_output() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } - - #[test] - fn key_matches_ivec_transparent_outpoint() { - zebra_test::init(); - proptest!(|(val in any::())| assert_as_bytes_matches_ivec(val)); - } } From 370015cbe3c73a64d7030bb4d0831405e9838449 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Wed, 18 Nov 2020 17:46:53 -0800 Subject: [PATCH 18/18] remove outdated paragraph --- book/src/dev/rfcs/0004-asynchronous-script-verification.md | 7 ------- book/src/dev/rfcs/0005-state-updates.md | 7 ------- 2 files changed, 14 deletions(-) diff --git a/book/src/dev/rfcs/0004-asynchronous-script-verification.md b/book/src/dev/rfcs/0004-asynchronous-script-verification.md index 1156f2a7bad..9e42917ca81 100644 --- a/book/src/dev/rfcs/0004-asynchronous-script-verification.md +++ b/book/src/dev/rfcs/0004-asynchronous-script-verification.md @@ -116,13 +116,6 @@ This means that our external API ensures that the state service sees a linearized sequence of state requests, although the exact ordering is unpredictable when there are multiple senders making requests. -In the second category, the rocksdb API presents itself synchronously, but -database and tree handles are clonable and can be moved between threads. All -that's required to process some request asynchronously is to clone the -appropriate handle, move it into an async block, and make the call as part of -the future. (We might want to use Tokio's blocking API for this, but that's a -side detail). - Because the state service has exclusive access to the rocksdb database, and the state service sees a linearized sequence of state requests, we have an easy way to opt in to asynchronous database access. We can perform rocksdb operations diff --git a/book/src/dev/rfcs/0005-state-updates.md b/book/src/dev/rfcs/0005-state-updates.md index efade58ceda..b685af0eae0 100644 --- a/book/src/dev/rfcs/0005-state-updates.md +++ b/book/src/dev/rfcs/0005-state-updates.md @@ -199,13 +199,6 @@ This means that our external API ensures that the state service sees a linearized sequence of state requests, although the exact ordering is unpredictable when there are multiple senders making requests. -In the second category, the rocksdb API presents itself synchronously, but -database and tree handles are cloneable and can be moved between threads. All -that's required to process some request asynchronously is to clone the -appropriate handle, move it into an async block, and make the call as part of -the future. (We might want to use Tokio's blocking API for this, but this is -an implementation detail). - Because the state service has exclusive access to the rocksdb database, and the state service sees a linearized sequence of state requests, we have an easy way to opt in to asynchronous database access. We can perform rocksdb operations