-
Notifications
You must be signed in to change notification settings - Fork 110
wip(l1): alternate snapshot calculation #4884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
This reverts commit 175de2d.
Benchmark for 02a5733Click to view benchmark
|
Benchmark for 781ab2fClick to view benchmark
|
Benchmark for 8c91fc8Click to view benchmark
|
Benchmark for bb899daClick to view benchmark
|
Benchmark for f78d0a8Click to view benchmark
|
Benchmark for d88bcb2Click to view benchmark
|
crates/storage/store_db/in_memory.rs
Outdated
} | ||
|
||
fn generate_snapshot(&self) -> Result<(), StoreError> { | ||
todo!() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relace this todo!
for a Ok(())
.
crates/storage/store_db/rocksdb.rs
Outdated
const CF_INVALID_ANCESTORS: &str = "invalid_ancestors"; | ||
|
||
#[derive(Debug)] | ||
const CF_SNAPSHOTS: &str = "snapshots"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use a better name for this CF. For example: flatkeyvalue
crates/storage/store_db/rocksdb.rs
Outdated
let store_clone = store.clone(); | ||
std::thread::spawn(move || { | ||
let mut rx = rx; | ||
while rx.recv() != Ok(SnapshotControlMessage::Continue) {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should take in account Err
results of this call.
crates/storage/store_db/rocksdb.rs
Outdated
self.db | ||
.delete_range_cf(&cf_snapshot, vec![], vec![0xff; 131])?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this delete should be done after snap sync, not before
crates/storage/store_db/rocksdb.rs
Outdated
.map(|v| Nibbles::from_hex(v.to_vec())) | ||
.unwrap_or_default(); | ||
|
||
println!("Starting snapshooting loop pivot={last_written:?} SR={state_root:x}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to debug!
Benchmark for 741e9baClick to view benchmark
|
Benchmark for 1ada7d2Click to view benchmark
|
Benchmark for 18ae5ffClick to view benchmark
|
batch.put_cf(&cf_misc, "last_written", path.as_ref()); | ||
batch.put_cf(&cf_snapshot, path.as_ref(), node.value); | ||
ctr += 1; | ||
if ctr > 10_000 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think its possible to miss a db.write if the last loop doesnt land on a > 10_000, i.e ctr < 10_000 and we dont loop anymore, isnt the batch lost?, i dont see any other db.write outsdie a if ctr > 10_000 to handle that case
Motivation
Description
Closes #issue_number