blockstore: Refactor Rocks iterator methods #4030
Conversation
| ) -> impl Iterator<Item = (Box<[u8]>, 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 |
There was a problem hiding this comment.
I added this comment for the interim, but this duplicate allocation/conversion will go away later in upcoming PR. I think the comment is valid regardless tho
ledger/src/blockstore_db.rs
Outdated
| IteratorMode::Start => RocksIteratorMode::Start, | ||
| IteratorMode::End => RocksIteratorMode::End, | ||
| }; | ||
| fn iterator_cf(&self, cf: &ColumnFamily, iterator_mode: RocksIteratorMode) -> DBIterator { |
There was a problem hiding this comment.
The method in Rocks is now a simple pass-through
| 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(); |
There was a problem hiding this comment.
We can't use get_index_bounds() here since this is operating on a different column.
I could hypothetically parameterize get_index_bounds() to take a column instead of Blockstore (currently, get_index_bounds() uses transaction_status_cf). However, I think that will make things more difficult for a future PR so I'd rather hold off on that for now. IMO, what I did here is still cleaner than what was there (ie no .0)
| .iterator_cf_raw_key(IteratorMode::Start) | ||
| .collect(); | ||
| assert_eq!(statuses.len(), 15); | ||
| .iter(IteratorMode::Start) |
There was a problem hiding this comment.
I started off wanting to kill iterator_cf_raw_key() altogether. However, we still need it for the cases where we perform compaction.
Regardless, I opted to use iter() where possible/it made more sense instead of iterator_cf_raw_key() to work towards being able to remove the test only iterator_cf_raw_key() function
Problem
There will be an upcoming PR for #3850, this breaks out some cleanup and refactoring that will enable that PR to come in with a smaller diff. No functional changes in this PR.
Summary of Changes
C::indexconversion inLedgerColumninstead ofRocksblockstore_purge.rs