Skip to content

Commit 78d0496

Browse files
michaelsproulmichaelsproul
committed
Rework store config and compatibility checks
1 parent 8ebe1b2 commit 78d0496

File tree

2 files changed

+24
-142
lines changed

2 files changed

+24
-142
lines changed

‎beacon_node/store/src/config.rs‎

Lines changed: 14 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::hdiff::HierarchyConfig;
22
use crate::superstruct;
3-
use crate::{AnchorInfo, DBColumn, Error, Split, StoreItem};
3+
use crate::{DBColumn, Error, StoreItem};
44
use serde::{Deserialize, Serialize};
55
use ssz::{Decode, Encode};
66
use ssz_derive::{Decode, Encode};
@@ -65,14 +65,12 @@ pub struct StoreConfig {
6565

6666
/// Variant of `StoreConfig` that gets written to disk. Contains immutable configuration params.
6767
#[superstruct(
68-
variants(V1, V22),
68+
variants(V22),
6969
variant_attributes(derive(Debug, Clone, PartialEq, Eq, Encode, Decode))
7070
)]
7171
#[derive(Clone, Debug, PartialEq, Eq)]
7272
pub struct OnDiskStoreConfig {
73-
#[superstruct(only(V1))]
74-
pub slots_per_restore_point: u64,
75-
/// Prefix byte to future-proof versions of the `OnDiskStoreConfig` post V1
73+
/// Prefix byte to future-proof versions of the `OnDiskStoreConfig`.
7674
#[superstruct(only(V22))]
7775
version_byte: u8,
7876
#[superstruct(only(V22))]
@@ -90,10 +88,6 @@ impl OnDiskStoreConfigV22 {
9088

9189
#[derive(Debug, Clone)]
9290
pub enum StoreConfigError {
93-
MismatchedSlotsPerRestorePoint {
94-
config: u64,
95-
on_disk: u64,
96-
},
9791
InvalidCompressionLevel {
9892
level: i32,
9993
},
@@ -134,21 +128,13 @@ impl StoreConfig {
134128
pub fn check_compatibility(
135129
&self,
136130
on_disk_config: &OnDiskStoreConfig,
137-
split: &Split,
138-
anchor: &AnchorInfo,
139131
) -> Result<(), StoreConfigError> {
140-
// Allow changing the hierarchy exponents if no historic states are stored.
141-
let no_historic_states_stored = anchor.no_historic_states_stored(split.slot);
142-
let hierarchy_config_changed =
143-
if let Ok(on_disk_hierarchy_config) = on_disk_config.hierarchy_config() {
144-
*on_disk_hierarchy_config != self.hierarchy_config
145-
} else {
146-
false
147-
};
148-
149-
if hierarchy_config_changed && !no_historic_states_stored {
132+
// We previously allowed the hierarchy exponents to change on non-archive nodes, but since
133+
// schema v24 and the use of hdiffs in the hot DB, changing will require a resync.
134+
let current_config = self.as_disk_config();
135+
if current_config != *on_disk_config {
150136
Err(StoreConfigError::IncompatibleStoreConfig {
151-
config: self.as_disk_config(),
137+
config: current_config,
152138
on_disk: on_disk_config.clone(),
153139
})
154140
} else {
@@ -222,32 +208,21 @@ impl StoreItem for OnDiskStoreConfig {
222208

223209
fn as_store_bytes(&self) -> Vec<u8> {
224210
match self {
225-
OnDiskStoreConfig::V1(value) => value.as_ssz_bytes(),
226211
OnDiskStoreConfig::V22(value) => value.as_ssz_bytes(),
227212
}
228213
}
229214

230215
fn from_store_bytes(bytes: &[u8]) -> Result<Self, Error> {
231-
// NOTE: V22 config can never be deserialized as a V1 because the minimum length of its
232-
// serialization is: 1 prefix byte + 1 offset (OnDiskStoreConfigV1 container) +
233-
// 1 offset (HierarchyConfig container) = 9.
234-
if let Ok(value) = OnDiskStoreConfigV1::from_ssz_bytes(bytes) {
235-
return Ok(Self::V1(value));
216+
match bytes.first() {
217+
Some(22) => Ok(Self::V22(OnDiskStoreConfigV22::from_ssz_bytes(bytes)?)),
218+
version_byte => Err(StoreConfigError::InvalidVersionByte(version_byte.copied()).into()),
236219
}
237-
238-
Ok(Self::V22(OnDiskStoreConfigV22::from_ssz_bytes(bytes)?))
239220
}
240221
}
241222

242223
#[cfg(test)]
243224
mod test {
244225
use super::*;
245-
use crate::{
246-
metadata::{ANCHOR_UNINITIALIZED, STATE_UPPER_LIMIT_NO_RETAIN},
247-
AnchorInfo, Split,
248-
};
249-
use ssz::DecodeError;
250-
use types::{Hash256, Slot};
251226

252227
#[test]
253228
fn check_compatibility_ok() {
@@ -257,24 +232,7 @@ mod test {
257232
let on_disk_config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(
258233
store_config.hierarchy_config.clone(),
259234
));
260-
let split = Split::default();
261-
assert!(store_config
262-
.check_compatibility(&on_disk_config, &split, &ANCHOR_UNINITIALIZED)
263-
.is_ok());
264-
}
265-
266-
#[test]
267-
fn check_compatibility_after_migration() {
268-
let store_config = StoreConfig {
269-
..Default::default()
270-
};
271-
let on_disk_config = OnDiskStoreConfig::V1(OnDiskStoreConfigV1 {
272-
slots_per_restore_point: 8192,
273-
});
274-
let split = Split::default();
275-
assert!(store_config
276-
.check_compatibility(&on_disk_config, &split, &ANCHOR_UNINITIALIZED)
277-
.is_ok());
235+
assert!(store_config.check_compatibility(&on_disk_config).is_ok());
278236
}
279237

280238
#[test]
@@ -283,77 +241,11 @@ mod test {
283241
let on_disk_config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(HierarchyConfig {
284242
exponents: vec![5, 8, 11, 13, 16, 18, 21],
285243
}));
286-
let split = Split {
287-
slot: Slot::new(32),
288-
..Default::default()
289-
};
290-
let anchor = AnchorInfo {
291-
anchor_slot: Slot::new(0),
292-
oldest_block_slot: Slot::new(0),
293-
oldest_block_parent: Hash256::ZERO,
294-
state_upper_limit: Slot::new(0),
295-
state_lower_limit: Slot::new(0),
296-
};
297-
assert!(store_config
298-
.check_compatibility(&on_disk_config, &split, &anchor)
299-
.is_err());
300-
}
301-
302-
#[test]
303-
fn check_compatibility_hierarchy_config_update() {
304-
let store_config = StoreConfig {
305-
..Default::default()
306-
};
307-
let on_disk_config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(HierarchyConfig {
308-
exponents: vec![5, 8, 11, 13, 16, 18, 21],
309-
}));
310-
let split = Split::default();
311-
let anchor = AnchorInfo {
312-
anchor_slot: Slot::new(0),
313-
oldest_block_slot: Slot::new(0),
314-
oldest_block_parent: Hash256::ZERO,
315-
state_upper_limit: STATE_UPPER_LIMIT_NO_RETAIN,
316-
state_lower_limit: Slot::new(0),
317-
};
318-
assert!(store_config
319-
.check_compatibility(&on_disk_config, &split, &anchor)
320-
.is_ok());
321-
}
322-
323-
#[test]
324-
fn serde_on_disk_config_v0_from_v1_default() {
325-
let config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(<_>::default()));
326-
let config_bytes = config.as_store_bytes();
327-
// On a downgrade, the previous version of lighthouse will attempt to deserialize the
328-
// prefixed V22 as just the V1 version.
329-
assert_eq!(
330-
OnDiskStoreConfigV1::from_ssz_bytes(&config_bytes).unwrap_err(),
331-
DecodeError::InvalidByteLength {
332-
len: 16,
333-
expected: 8
334-
},
335-
);
336-
}
337-
338-
#[test]
339-
fn serde_on_disk_config_v0_from_v1_empty() {
340-
let config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(HierarchyConfig {
341-
exponents: vec![],
342-
}));
343-
let config_bytes = config.as_store_bytes();
344-
// On a downgrade, the previous version of lighthouse will attempt to deserialize the
345-
// prefixed V22 as just the V1 version.
346-
assert_eq!(
347-
OnDiskStoreConfigV1::from_ssz_bytes(&config_bytes).unwrap_err(),
348-
DecodeError::InvalidByteLength {
349-
len: 9,
350-
expected: 8
351-
},
352-
);
244+
assert!(store_config.check_compatibility(&on_disk_config).is_err());
353245
}
354246

355247
#[test]
356-
fn serde_on_disk_config_v1_roundtrip() {
248+
fn on_disk_config_v22_roundtrip() {
357249
let config = OnDiskStoreConfig::V22(OnDiskStoreConfigV22::new(<_>::default()));
358250
let bytes = config.as_store_bytes();
359251
assert_eq!(bytes[0], 22);

‎beacon_node/store/src/hot_cold_store.rs‎

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,16 @@ impl<E: EthSpec> HotColdDB<E, BeaconNodeBackend<E>, BeaconNodeBackend<E>> {
366366
"Blob DB initialized"
367367
);
368368

369+
// Ensure that any on-disk config is compatible with the supplied config.
370+
//
371+
// We do this prior to the migration now, because we don't want the migration using the
372+
// in-memory config if it is inconsistent with the on-disk config. In future we may need
373+
// to put this in/after the migration if the migration changes the config format.
374+
if let Some(disk_config) = db.load_config()? {
375+
db.config.check_compatibility(&disk_config)?;
376+
}
377+
db.store_config()?;
378+
369379
// Ensure that the schema version of the on-disk database matches the software.
370380
// If the version is mismatched, an automatic migration will be attempted.
371381
let db = Arc::new(db);
@@ -393,26 +403,6 @@ impl<E: EthSpec> HotColdDB<E, BeaconNodeBackend<E>, BeaconNodeBackend<E>> {
393403
debug!(?split, "Hot-Cold DB split initialized");
394404
}
395405

396-
// Ensure that any on-disk config is compatible with the supplied config.
397-
if let Some(disk_config) = db.load_config()? {
398-
let split = db.get_split_info();
399-
let anchor = db.get_anchor_info();
400-
db.config
401-
.check_compatibility(&disk_config, &split, &anchor)?;
402-
403-
// Inform user if hierarchy config is changing.
404-
if let Ok(hierarchy_config) = disk_config.hierarchy_config() {
405-
if &db.config.hierarchy_config != hierarchy_config {
406-
info!(
407-
previous_config = %hierarchy_config,
408-
new_config = %db.config.hierarchy_config,
409-
"Updating historic state config"
410-
);
411-
}
412-
}
413-
}
414-
db.store_config()?;
415-
416406
// TODO(tree-states): Here we can choose to prune advanced states to reclaim disk space. As
417407
// it's a foreground task there's no risk of race condition that can corrupt the DB.
418408
// Advanced states for invalid blocks that were never written to the DB, or descendants of

0 commit comments

Comments
 (0)