From a929aa696a1ca3cc4c1189d6ae433654e8925398 Mon Sep 17 00:00:00 2001 From: steviez Date: Wed, 11 Dec 2024 11:40:03 -0600 Subject: [PATCH] blockstore: Refactor Rocks iterator methods (#4030) * Shift C::Index to &[u8] converstion from Rocks to LedgerColumn * Deduplicate multiple iterator function * Test cleanup --- ledger/src/blockstore.rs | 2 +- ledger/src/blockstore/blockstore_purge.rs | 163 ++++++++-------------- ledger/src/blockstore_db.rs | 81 +++++------ 3 files changed, 104 insertions(+), 142 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 77a7193135f226..7731925f6b4417 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -675,7 +675,7 @@ impl Blockstore { cf_name: &str, ) -> Result, Box<[u8]>)> + '_> { let cf = self.db.cf_handle(cf_name); - let iterator = self.db.iterator_cf_raw_key(cf, IteratorMode::Start); + let iterator = self.db.iterator_cf(cf, rocksdb::IteratorMode::Start); Ok(iterator.map(|pair| pair.unwrap())) } diff --git a/ledger/src/blockstore/blockstore_purge.rs b/ledger/src/blockstore/blockstore_purge.rs index 8b7895612b3a6d..66cf51e7970e63 100644 --- a/ledger/src/blockstore/blockstore_purge.rs +++ b/ledger/src/blockstore/blockstore_purge.rs @@ -804,18 +804,17 @@ pub mod tests { } fn get_index_bounds(blockstore: &Blockstore) -> (Box<[u8]>, Box<[u8]>) { - let first_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); - status_entry_iterator.next().unwrap().unwrap().0 - }; - let last_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::End); - status_entry_iterator.next().unwrap().unwrap().0 - }; + let (first_index, _value) = blockstore + .transaction_status_cf + .iterator_cf_raw_key(IteratorMode::Start) + .next() + .unwrap(); + let (last_index, _value) = blockstore + .transaction_status_cf + .iterator_cf_raw_key(IteratorMode::End) + .next() + .unwrap(); + (first_index, last_index) } @@ -866,22 +865,22 @@ pub mod tests { .put(1, &index1) .unwrap(); - let statuses: Vec<_> = blockstore + let num_statuses = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start) - .collect(); - assert_eq!(statuses.len(), 15); + .iter(IteratorMode::Start) + .unwrap() + .count(); + assert_eq!(num_statuses, 15); // Delete some of primary-index 0 let oldest_slot = 3; purge(&blockstore, oldest_slot); let status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for entry in status_entry_iterator { - let (key, _value) = entry.unwrap(); - let (_signature, slot) = ::index(&key); + for ((_signature, slot), _value) in status_entry_iterator { assert!(slot >= oldest_slot); count += 1; } @@ -892,11 +891,10 @@ pub mod tests { purge(&blockstore, oldest_slot); let status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for entry in status_entry_iterator { - let (key, _value) = entry.unwrap(); - let (_signature, slot) = ::index(&key); + for ((_signature, slot), _value) in status_entry_iterator { assert!(slot >= oldest_slot); count += 1; } @@ -907,11 +905,10 @@ pub mod tests { purge(&blockstore, oldest_slot); let status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for entry in status_entry_iterator { - let (key, _value) = entry.unwrap(); - let (_signature, slot) = ::index(&key); + for ((_signature, slot), _value) in status_entry_iterator { assert!(slot >= oldest_slot); count += 1; } @@ -922,11 +919,10 @@ pub mod tests { purge(&blockstore, oldest_slot); let status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for entry in status_entry_iterator { - let (key, _value) = entry.unwrap(); - let (_signature, slot) = ::index(&key); + for ((_signature, slot), _value) in status_entry_iterator { assert!(slot >= oldest_slot); count += 1; } @@ -937,11 +933,10 @@ pub mod tests { purge(&blockstore, oldest_slot); let status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for entry in status_entry_iterator { - let (key, _value) = entry.unwrap(); - let (_signature, slot) = ::index(&key); + for ((_signature, slot), _value) in status_entry_iterator { assert!(slot >= oldest_slot); count += 1; } @@ -952,7 +947,8 @@ pub mod tests { purge(&blockstore, oldest_slot); let mut status_entry_iterator = blockstore .transaction_status_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); assert!(status_entry_iterator.next().is_none()); } @@ -993,27 +989,13 @@ pub mod tests { let max_slot = 19; clear_and_repopulate_transaction_statuses_for_test(&blockstore, max_slot); - let first_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iter(IteratorMode::Start) - .unwrap(); - status_entry_iterator.next().unwrap().0 - }; - let last_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iter(IteratorMode::End) - .unwrap(); - status_entry_iterator.next().unwrap().0 - }; + let (first_index, last_index) = get_index_bounds(&blockstore); let oldest_slot = 3; blockstore.db.set_oldest_slot(oldest_slot); - blockstore.transaction_status_cf.compact_range_raw_key( - &cf::TransactionStatus::key(first_index), - &cf::TransactionStatus::key(last_index), - ); + blockstore + .transaction_status_cf + .compact_range_raw_key(&first_index, &last_index); let status_entry_iterator = blockstore .transaction_status_cf @@ -1027,27 +1009,13 @@ pub mod tests { assert_eq!(count, max_slot - (oldest_slot - 1)); clear_and_repopulate_transaction_statuses_for_test(&blockstore, max_slot); - let first_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iter(IteratorMode::Start) - .unwrap(); - status_entry_iterator.next().unwrap().0 - }; - let last_index = { - let mut status_entry_iterator = blockstore - .transaction_status_cf - .iter(IteratorMode::End) - .unwrap(); - status_entry_iterator.next().unwrap().0 - }; + let (first_index, last_index) = get_index_bounds(&blockstore); let oldest_slot = 12; blockstore.db.set_oldest_slot(oldest_slot); - blockstore.transaction_status_cf.compact_range_raw_key( - &cf::TransactionStatus::key(first_index), - &cf::TransactionStatus::key(last_index), - ); + blockstore + .transaction_status_cf + .compact_range_raw_key(&first_index, &last_index); let status_entry_iterator = blockstore .transaction_status_cf @@ -1103,33 +1071,28 @@ pub mod tests { ) .unwrap(); - let first_index = { - let mut memos_iterator = blockstore - .transaction_memos_cf - .iterator_cf_raw_key(IteratorMode::Start); - memos_iterator.next().unwrap().unwrap().0 - }; - let last_index = { - let mut memos_iterator = blockstore - .transaction_memos_cf - .iterator_cf_raw_key(IteratorMode::End); - memos_iterator.next().unwrap().unwrap().0 - }; + let (first_index, _value) = blockstore + .transaction_memos_cf + .iterator_cf_raw_key(IteratorMode::Start) + .next() + .unwrap(); + let (last_index, _value) = blockstore + .transaction_memos_cf + .iterator_cf_raw_key(IteratorMode::End) + .next() + .unwrap(); // Purge at slot 0 should not affect any memos blockstore.db.set_oldest_slot(0); blockstore .transaction_memos_cf .compact_range_raw_key(&first_index, &last_index); - let memos_iterator = blockstore + let num_memos = blockstore .transaction_memos_cf - .iterator_cf_raw_key(IteratorMode::Start); - let mut count = 0; - for item in memos_iterator { - let _item = item.unwrap(); - count += 1; - } - assert_eq!(count, 4); + .iter(IteratorMode::Start) + .unwrap() + .count(); + assert_eq!(num_memos, 4); // Purge at oldest_slot without clean_slot_0 only purges the current memo at slot 4 blockstore.db.set_oldest_slot(oldest_slot); @@ -1138,11 +1101,10 @@ pub mod tests { .compact_range_raw_key(&first_index, &last_index); let memos_iterator = blockstore .transaction_memos_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for item in memos_iterator { - let (key, _value) = item.unwrap(); - let slot = ::index(&key).1; + for ((_signature, slot), _value) in memos_iterator { assert!(slot == 0 || slot >= oldest_slot); count += 1; } @@ -1155,11 +1117,10 @@ pub mod tests { .compact_range_raw_key(&first_index, &last_index); let memos_iterator = blockstore .transaction_memos_cf - .iterator_cf_raw_key(IteratorMode::Start); + .iter(IteratorMode::Start) + .unwrap(); let mut count = 0; - for item in memos_iterator { - let (key, _value) = item.unwrap(); - let slot = ::index(&key).1; + for ((_signature, slot), _value) in memos_iterator { assert!(slot >= oldest_slot); count += 1; } diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs index 406b3aa01c0339..da270dc853d24c 100644 --- a/ledger/src/blockstore_db.rs +++ b/ledger/src/blockstore_db.rs @@ -695,36 +695,11 @@ impl Rocks { Ok(()) } - fn iterator_cf(&self, cf: &ColumnFamily, iterator_mode: IteratorMode) -> DBIterator - where - C: Column, - { - let start_key; - let iterator_mode = match iterator_mode { - IteratorMode::From(start_from, direction) => { - start_key = C::key(start_from); - RocksIteratorMode::From(&start_key, direction) - } - IteratorMode::Start => RocksIteratorMode::Start, - IteratorMode::End => RocksIteratorMode::End, - }; - self.db.iterator_cf(cf, iterator_mode) - } - - pub(crate) fn iterator_cf_raw_key( + pub(crate) fn iterator_cf( &self, cf: &ColumnFamily, - iterator_mode: IteratorMode>, + iterator_mode: RocksIteratorMode, ) -> DBIterator { - let start_key; - let iterator_mode = match iterator_mode { - IteratorMode::From(start_from, direction) => { - start_key = start_from; - RocksIteratorMode::From(&start_key, direction) - } - IteratorMode::Start => RocksIteratorMode::Start, - IteratorMode::End => RocksIteratorMode::End, - }; self.db.iterator_cf(cf, iterator_mode) } @@ -1498,7 +1473,17 @@ where iterator_mode: IteratorMode, ) -> Result)> + '_> { let cf = self.handle(); - let iter = self.backend.iterator_cf::(cf, iterator_mode); + let start_key; + let iterator_mode = match iterator_mode { + IteratorMode::Start => RocksIteratorMode::Start, + IteratorMode::End => RocksIteratorMode::End, + IteratorMode::From(start_from, direction) => { + start_key = C::key(start_from); + RocksIteratorMode::From(&start_key, direction) + } + }; + + let iter = self.backend.iterator_cf(cf, iterator_mode); Ok(iter.map(|pair| { let (key, value) = pair.unwrap(); (C::index(&key), value) @@ -1808,7 +1793,17 @@ where iterator_mode: IteratorMode, ) -> Result)> + '_> { let cf = self.handle(); - let iter = self.backend.iterator_cf::(cf, iterator_mode); + let start_key; + let iterator_mode = match iterator_mode { + IteratorMode::Start => RocksIteratorMode::Start, + IteratorMode::End => RocksIteratorMode::End, + IteratorMode::From(start_from, direction) => { + start_key = C::key(start_from); + RocksIteratorMode::From(&start_key, direction) + } + }; + + let iter = self.backend.iterator_cf(cf, iterator_mode); Ok(iter.filter_map(|pair| { let (key, value) = pair.unwrap(); C::try_current_index(&key).ok().map(|index| (index, value)) @@ -1820,16 +1815,18 @@ where iterator_mode: IteratorMode, ) -> Result)> + '_> { let cf = self.handle(); - let iterator_mode_raw_key = match iterator_mode { - IteratorMode::Start => IteratorMode::Start, - IteratorMode::End => IteratorMode::End, + let start_key; + let iterator_mode = match iterator_mode { + IteratorMode::Start => RocksIteratorMode::Start, + IteratorMode::End => RocksIteratorMode::End, IteratorMode::From(start_from, direction) => { - let raw_key = C::deprecated_key(start_from); - IteratorMode::From(raw_key, direction) + start_key = C::deprecated_key(start_from); + RocksIteratorMode::From(&start_key, direction) } }; - let iter = self.backend.iterator_cf_raw_key(cf, iterator_mode_raw_key); - Ok(iter.filter_map(|pair| { + + let iterator = self.backend.iterator_cf(cf, iterator_mode); + Ok(iterator.filter_map(|pair| { let (key, value) = pair.unwrap(); C::try_deprecated_index(&key) .ok() @@ -2204,10 +2201,14 @@ pub mod tests { { pub(crate) fn iterator_cf_raw_key( &self, - iterator_mode: IteratorMode>, - ) -> DBIterator { - let cf = self.handle(); - self.backend.iterator_cf_raw_key(cf, iterator_mode) + iterator_mode: IteratorMode, + ) -> impl Iterator, Box<[u8]>)> + '_ { + // The conversion of key back into Box<[u8]> incurs an extra + // allocation. However, this is test code and the goal is to + // maximize code reuse over efficiency + self.iter(iterator_mode) + .unwrap() + .map(|(key, value)| (Box::from(C::key(key)), value)) } } }