Skip to content

Commit 051c3e8

Browse files
michaelsproulmichaelsproul
authored
Always use a separate database for blobs (sigp#4892)
* Always use a separate blobs DB * Add + update tests
1 parent 1b8c0ed commit 051c3e8

File tree

9 files changed

+89
-53
lines changed

9 files changed

+89
-53
lines changed

‎beacon_node/beacon_chain/src/data_availability_checker/overflow_lru_cache.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -771,12 +771,13 @@ mod test {
771771
) -> Arc<HotColdDB<E, LevelDB<E>, LevelDB<E>>> {
772772
let hot_path = db_path.path().join("hot_db");
773773
let cold_path = db_path.path().join("cold_db");
774+
let blobs_path = db_path.path().join("blobs_db");
774775
let config = StoreConfig::default();
775776

776777
HotColdDB::open(
777778
&hot_path,
778779
&cold_path,
779-
None,
780+
&blobs_path,
780781
|_, _, _| Ok(()),
781782
config,
782783
spec,

‎beacon_node/beacon_chain/tests/op_verification.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,13 @@ fn get_store(db_path: &TempDir) -> Arc<HotColdDB> {
2929
let spec = test_spec::<E>();
3030
let hot_path = db_path.path().join("hot_db");
3131
let cold_path = db_path.path().join("cold_db");
32+
let blobs_path = db_path.path().join("blobs_db");
3233
let config = StoreConfig::default();
3334
let log = NullLoggerBuilder.build().expect("logger should build");
3435
HotColdDB::open(
3536
&hot_path,
3637
&cold_path,
37-
None,
38+
&blobs_path,
3839
|_, _, _| Ok(()),
3940
config,
4041
spec,

‎beacon_node/beacon_chain/tests/store_tests.rs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use store::{
3131
chunked_vector::{chunk_key, Field},
3232
get_key_for_col,
3333
iter::{BlockRootsIterator, StateRootsIterator},
34-
DBColumn, HotColdDB, KeyValueStore, KeyValueStoreOp, LevelDB, StoreConfig,
34+
BlobInfo, DBColumn, HotColdDB, KeyValueStore, KeyValueStoreOp, LevelDB, StoreConfig,
3535
};
3636
use tempfile::{tempdir, TempDir};
3737
use tokio::time::sleep;
@@ -62,12 +62,13 @@ fn get_store_generic(
6262
) -> Arc<HotColdDB<E, LevelDB<E>, LevelDB<E>>> {
6363
let hot_path = db_path.path().join("hot_db");
6464
let cold_path = db_path.path().join("cold_db");
65+
let blobs_path = db_path.path().join("blobs_db");
6566
let log = test_logger();
6667

6768
HotColdDB::open(
6869
&hot_path,
6970
&cold_path,
70-
None,
71+
&blobs_path,
7172
|_, _, _| Ok(()),
7273
config,
7374
spec,
@@ -3278,6 +3279,40 @@ async fn deneb_prune_blobs_margin_test(margin: u64) {
32783279
check_blob_existence(&harness, oldest_blob_slot, harness.head_slot(), true);
32793280
}
32803281

3282+
/// Check that a database with `blobs_db=false` can be upgraded to `blobs_db=true` before Deneb.
3283+
#[tokio::test]
3284+
async fn change_to_separate_blobs_db_before_deneb() {
3285+
let db_path = tempdir().unwrap();
3286+
let store = get_store(&db_path);
3287+
3288+
// Only run this test on forks prior to Deneb. If the blobs database already has blobs, we can't
3289+
// move it.
3290+
if store.get_chain_spec().deneb_fork_epoch.is_some() {
3291+
return;
3292+
}
3293+
3294+
let init_blob_info = store.get_blob_info();
3295+
assert!(
3296+
init_blob_info.blobs_db,
3297+
"separate blobs DB should be the default"
3298+
);
3299+
3300+
// Change to `blobs_db=false` to emulate legacy Deneb DB.
3301+
let legacy_blob_info = BlobInfo {
3302+
blobs_db: false,
3303+
..init_blob_info
3304+
};
3305+
store
3306+
.compare_and_set_blob_info_with_write(init_blob_info.clone(), legacy_blob_info.clone())
3307+
.unwrap();
3308+
assert_eq!(store.get_blob_info(), legacy_blob_info);
3309+
3310+
// Re-open the DB and check that `blobs_db` gets changed back to true.
3311+
drop(store);
3312+
let store = get_store(&db_path);
3313+
assert_eq!(store.get_blob_info(), init_blob_info);
3314+
}
3315+
32813316
/// Check that there are blob sidecars (or not) at every slot in the range.
32823317
fn check_blob_existence(
32833318
harness: &TestHarness,

‎beacon_node/client/src/builder.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,7 @@ where
901901
mut self,
902902
hot_path: &Path,
903903
cold_path: &Path,
904-
blobs_path: Option<PathBuf>,
904+
blobs_path: &Path,
905905
config: StoreConfig,
906906
log: Logger,
907907
) -> Result<Self, String> {

‎beacon_node/client/src/config.rs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ use std::fs;
1010
use std::path::PathBuf;
1111
use std::time::Duration;
1212
use types::Graffiti;
13+
1314
/// Default directory name for the freezer database under the top-level data dir.
1415
const DEFAULT_FREEZER_DB_DIR: &str = "freezer_db";
16+
/// Default directory name for the blobs database under the top-level data dir.
17+
const DEFAULT_BLOBS_DB_DIR: &str = "blobs_db";
1518

1619
/// Defines how the client should initialize the `BeaconChain` and other components.
1720
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
@@ -146,12 +149,19 @@ impl Config {
146149
.unwrap_or_else(|| self.default_freezer_db_path())
147150
}
148151

152+
/// Fetch default path to use for the blobs database.
153+
fn default_blobs_db_path(&self) -> PathBuf {
154+
self.get_data_dir().join(DEFAULT_BLOBS_DB_DIR)
155+
}
156+
149157
/// Returns the path to which the client may initialize the on-disk blobs database.
150158
///
151159
/// Will attempt to use the user-supplied path from e.g. the CLI, or will default
152160
/// to None.
153-
pub fn get_blobs_db_path(&self) -> Option<PathBuf> {
154-
self.blobs_db_path.clone()
161+
pub fn get_blobs_db_path(&self) -> PathBuf {
162+
self.blobs_db_path
163+
.clone()
164+
.unwrap_or_else(|| self.default_blobs_db_path())
155165
}
156166

157167
/// Get the freezer DB path, creating it if necessary.
@@ -160,11 +170,8 @@ impl Config {
160170
}
161171

162172
/// Get the blobs DB path, creating it if necessary.
163-
pub fn create_blobs_db_path(&self) -> Result<Option<PathBuf>, String> {
164-
match self.get_blobs_db_path() {
165-
Some(blobs_db_path) => Ok(Some(ensure_dir_exists(blobs_db_path)?)),
166-
None => Ok(None),
167-
}
173+
pub fn create_blobs_db_path(&self) -> Result<PathBuf, String> {
174+
ensure_dir_exists(self.get_blobs_db_path())
168175
}
169176

170177
/// Returns the "modern" path to the data_dir.

‎beacon_node/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ impl<E: EthSpec> ProductionBeaconNode<E> {
8989
.disk_store(
9090
&db_path,
9191
&freezer_db_path,
92-
blobs_db_path,
92+
&blobs_db_path,
9393
store_config,
9494
log.clone(),
9595
)?;

‎beacon_node/store/src/hot_cold_store.rs‎

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use state_processing::{
3535
use std::cmp::min;
3636
use std::convert::TryInto;
3737
use std::marker::PhantomData;
38-
use std::path::{Path, PathBuf};
38+
use std::path::Path;
3939
use std::sync::Arc;
4040
use std::time::Duration;
4141
use types::blob_sidecar::BlobSidecarList;
@@ -61,7 +61,7 @@ pub struct HotColdDB<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> {
6161
/// Cold database containing compact historical data.
6262
pub cold_db: Cold,
6363
/// Database containing blobs. If None, store falls back to use `cold_db`.
64-
pub blobs_db: Option<Cold>,
64+
pub blobs_db: Cold,
6565
/// Hot database containing duplicated but quick-to-access recent data.
6666
///
6767
/// The hot database also contains all blocks.
@@ -138,7 +138,6 @@ pub enum HotColdDBError {
138138
MissingExecutionPayload(Hash256),
139139
MissingFullBlockExecutionPayloadPruned(Hash256, Slot),
140140
MissingAnchorInfo,
141-
MissingPathToBlobsDatabase,
142141
BlobsPreviouslyInDefaultStore,
143142
HotStateSummaryError(BeaconStateError),
144143
RestorePointDecodeError(ssz::DecodeError),
@@ -178,7 +177,7 @@ impl<E: EthSpec> HotColdDB<E, MemoryStore<E>, MemoryStore<E>> {
178177
anchor_info: RwLock::new(None),
179178
blob_info: RwLock::new(BlobInfo::default()),
180179
cold_db: MemoryStore::open(),
181-
blobs_db: Some(MemoryStore::open()),
180+
blobs_db: MemoryStore::open(),
182181
hot_db: MemoryStore::open(),
183182
block_cache: Mutex::new(BlockCache::new(config.block_cache_size)),
184183
state_cache: Mutex::new(LruCache::new(config.historic_state_cache_size)),
@@ -202,7 +201,7 @@ impl<E: EthSpec> HotColdDB<E, LevelDB<E>, LevelDB<E>> {
202201
pub fn open(
203202
hot_path: &Path,
204203
cold_path: &Path,
205-
blobs_db_path: Option<PathBuf>,
204+
blobs_db_path: &Path,
206205
migrate_schema: impl FnOnce(Arc<Self>, SchemaVersion, SchemaVersion) -> Result<(), Error>,
207206
config: StoreConfig,
208207
spec: ChainSpec,
@@ -215,7 +214,7 @@ impl<E: EthSpec> HotColdDB<E, LevelDB<E>, LevelDB<E>> {
215214
anchor_info: RwLock::new(None),
216215
blob_info: RwLock::new(BlobInfo::default()),
217216
cold_db: LevelDB::open(cold_path)?,
218-
blobs_db: None,
217+
blobs_db: LevelDB::open(blobs_db_path)?,
219218
hot_db: LevelDB::open(hot_path)?,
220219
block_cache: Mutex::new(BlockCache::new(config.block_cache_size)),
221220
state_cache: Mutex::new(LruCache::new(config.historic_state_cache_size)),
@@ -271,37 +270,29 @@ impl<E: EthSpec> HotColdDB<E, LevelDB<E>, LevelDB<E>> {
271270
Some(blob_info) => {
272271
// If the oldest block slot is already set do not allow the blob DB path to be
273272
// changed (require manual migration).
274-
if blob_info.oldest_blob_slot.is_some() {
275-
if blobs_db_path.is_some() && !blob_info.blobs_db {
276-
return Err(HotColdDBError::BlobsPreviouslyInDefaultStore.into());
277-
} else if blobs_db_path.is_none() && blob_info.blobs_db {
278-
return Err(HotColdDBError::MissingPathToBlobsDatabase.into());
279-
}
273+
if blob_info.oldest_blob_slot.is_some() && !blob_info.blobs_db {
274+
return Err(HotColdDBError::BlobsPreviouslyInDefaultStore.into());
280275
}
281276
// Set the oldest blob slot to the Deneb fork slot if it is not yet set.
277+
// Always initialize `blobs_db` to true, we no longer support storing the blobs
278+
// in the freezer DB, because the UX is strictly worse for relocating the DB.
282279
let oldest_blob_slot = blob_info.oldest_blob_slot.or(deneb_fork_slot);
283280
BlobInfo {
284281
oldest_blob_slot,
285-
blobs_db: blobs_db_path.is_some(),
282+
blobs_db: true,
286283
}
287284
}
288285
// First start.
289286
None => BlobInfo {
290287
// Set the oldest blob slot to the Deneb fork slot if it is not yet set.
291288
oldest_blob_slot: deneb_fork_slot,
292-
blobs_db: blobs_db_path.is_some(),
289+
blobs_db: true,
293290
},
294291
};
295-
if new_blob_info.blobs_db {
296-
if let Some(path) = &blobs_db_path {
297-
db.blobs_db = Some(LevelDB::open(path.as_path())?);
298-
}
299-
}
300292
db.compare_and_set_blob_info_with_write(<_>::default(), new_blob_info.clone())?;
301293
info!(
302294
db.log,
303295
"Blob DB initialized";
304-
"separate_db" => new_blob_info.blobs_db,
305296
"path" => ?blobs_db_path,
306297
"oldest_blob_slot" => ?new_blob_info.oldest_blob_slot,
307298
);
@@ -575,8 +566,8 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
575566

576567
/// Check if the blobs for a block exists on disk.
577568
pub fn blobs_exist(&self, block_root: &Hash256) -> Result<bool, Error> {
578-
let blobs_db = self.blobs_db.as_ref().unwrap_or(&self.cold_db);
579-
blobs_db.key_exists(DBColumn::BeaconBlob.into(), block_root.as_bytes())
569+
self.blobs_db
570+
.key_exists(DBColumn::BeaconBlob.into(), block_root.as_bytes())
580571
}
581572

582573
/// Determine whether a block exists in the database.
@@ -592,13 +583,12 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
592583
.key_delete(DBColumn::BeaconBlock.into(), block_root.as_bytes())?;
593584
self.hot_db
594585
.key_delete(DBColumn::ExecPayload.into(), block_root.as_bytes())?;
595-
let blobs_db = self.blobs_db.as_ref().unwrap_or(&self.cold_db);
596-
blobs_db.key_delete(DBColumn::BeaconBlob.into(), block_root.as_bytes())
586+
self.blobs_db
587+
.key_delete(DBColumn::BeaconBlob.into(), block_root.as_bytes())
597588
}
598589

599590
pub fn put_blobs(&self, block_root: &Hash256, blobs: BlobSidecarList<E>) -> Result<(), Error> {
600-
let blobs_db = self.blobs_db.as_ref().unwrap_or(&self.cold_db);
601-
blobs_db.put_bytes(
591+
self.blobs_db.put_bytes(
602592
DBColumn::BeaconBlob.into(),
603593
block_root.as_bytes(),
604594
&blobs.as_ssz_bytes(),
@@ -988,9 +978,9 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
988978
let mut guard = self.block_cache.lock();
989979

990980
let blob_cache_ops = blobs_ops.clone();
991-
let blobs_db = self.blobs_db.as_ref().unwrap_or(&self.cold_db);
992981
// Try to execute blobs store ops.
993-
blobs_db.do_atomically(self.convert_to_kv_batch(blobs_ops)?)?;
982+
self.blobs_db
983+
.do_atomically(self.convert_to_kv_batch(blobs_ops)?)?;
994984

995985
let hot_db_cache_ops = hot_db_ops.clone();
996986
// Try to execute hot db store ops.
@@ -1018,7 +1008,8 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
10181008
};
10191009
*op = reverse_op;
10201010
}
1021-
blobs_db.do_atomically(self.convert_to_kv_batch(blob_cache_ops)?)?;
1011+
self.blobs_db
1012+
.do_atomically(self.convert_to_kv_batch(blob_cache_ops)?)?;
10221013
return Err(e);
10231014
}
10241015

@@ -1436,15 +1427,16 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
14361427

14371428
/// Fetch blobs for a given block from the store.
14381429
pub fn get_blobs(&self, block_root: &Hash256) -> Result<Option<BlobSidecarList<E>>, Error> {
1439-
let blobs_db = self.blobs_db.as_ref().unwrap_or(&self.cold_db);
1440-
14411430
// Check the cache.
14421431
if let Some(blobs) = self.block_cache.lock().get_blobs(block_root) {
14431432
metrics::inc_counter(&metrics::BEACON_BLOBS_CACHE_HIT_COUNT);
14441433
return Ok(Some(blobs.clone()));
14451434
}
14461435

1447-
match blobs_db.get_bytes(DBColumn::BeaconBlob.into(), block_root.as_bytes())? {
1436+
match self
1437+
.blobs_db
1438+
.get_bytes(DBColumn::BeaconBlob.into(), block_root.as_bytes())?
1439+
{
14481440
Some(ref blobs_bytes) => {
14491441
let blobs = BlobSidecarList::from_ssz_bytes(blobs_bytes)?;
14501442
self.block_cache
@@ -1640,7 +1632,7 @@ impl<E: EthSpec, Hot: ItemStore<E>, Cold: ItemStore<E>> HotColdDB<E, Hot, Cold>
16401632
});
16411633
let blob_info = BlobInfo {
16421634
oldest_blob_slot,
1643-
blobs_db: self.blobs_db.is_some(),
1635+
blobs_db: true,
16441636
};
16451637
self.compare_and_set_blob_info(self.get_blob_info(), blob_info)
16461638
}

‎beacon_node/store/src/metadata.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ pub struct BlobInfo {
135135
/// If the `oldest_blob_slot` is `None` then this means that the Deneb fork epoch is not yet
136136
/// known.
137137
pub oldest_blob_slot: Option<Slot>,
138-
/// A separate blobs database is in use.
138+
/// A separate blobs database is in use (deprecated, always `true`).
139139
pub blobs_db: bool,
140140
}
141141

‎database_manager/src/lib.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ pub fn display_db_version<E: EthSpec>(
210210
HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
211211
&hot_path,
212212
&cold_path,
213-
blobs_path,
213+
&blobs_path,
214214
|_, from, _| {
215215
version = from;
216216
Ok(())
@@ -288,7 +288,7 @@ pub fn inspect_db<E: EthSpec>(
288288
let db = HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
289289
&hot_path,
290290
&cold_path,
291-
blobs_path,
291+
&blobs_path,
292292
|_, _, _| Ok(()),
293293
client_config.store,
294294
spec,
@@ -410,7 +410,7 @@ pub fn migrate_db<E: EthSpec>(
410410
let db = HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
411411
&hot_path,
412412
&cold_path,
413-
blobs_path,
413+
&blobs_path,
414414
|_, db_initial_version, _| {
415415
from = db_initial_version;
416416
Ok(())
@@ -450,7 +450,7 @@ pub fn prune_payloads<E: EthSpec>(
450450
let db = HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
451451
&hot_path,
452452
&cold_path,
453-
blobs_path,
453+
&blobs_path,
454454
|_, _, _| Ok(()),
455455
client_config.store,
456456
spec.clone(),
@@ -476,7 +476,7 @@ pub fn prune_blobs<E: EthSpec>(
476476
let db = HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
477477
&hot_path,
478478
&cold_path,
479-
blobs_path,
479+
&blobs_path,
480480
|_, _, _| Ok(()),
481481
client_config.store,
482482
spec.clone(),
@@ -512,7 +512,7 @@ pub fn prune_states<E: EthSpec>(
512512
let db = HotColdDB::<E, LevelDB<E>, LevelDB<E>>::open(
513513
&hot_path,
514514
&cold_path,
515-
blobs_path,
515+
&blobs_path,
516516
|_, _, _| Ok(()),
517517
client_config.store,
518518
spec.clone(),

0 commit comments

Comments
 (0)