Skip to content

Commit

Permalink
cleanup(db): Give forward and reverse database iterators explicit nam…
Browse files Browse the repository at this point in the history
…es (#8063)

* Give forward and reverse database iterators explicit names

* clippy: Ignore large error variants
  • Loading branch information
teor2345 authored Dec 6, 2023
1 parent 1be140b commit 9fec711
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 30 deletions.
3 changes: 3 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ rustflags = [
"-Wmissing_docs",

# TODOs:
# Fix this lint eventually.
"-Aclippy::result_large_err",

# `cargo fix` might help do these fixes,
# or add a config.toml to sub-directories which should allow these lints,
# or try allowing the lint in the specific module (lib.rs doesn't seem to work in some cases)
Expand Down
27 changes: 10 additions & 17 deletions zebra-state/src/service/finalized_state/disk_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl WriteDisk for DiskWriteBatch {
}

// TODO: convert zs_delete_range() to take std::ops::RangeBounds
// see zs_range_iter() for an example of the edge cases
// see zs_forward_range_iter() for an example of the edge cases
fn zs_delete_range<C, K>(&mut self, cf: &C, from: K, to: K)
where
C: rocksdb::AsColumnFamilyRef,
Expand Down Expand Up @@ -329,7 +329,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
{
// Reading individual values from iterators does not seem to cause database hangs.
self.zs_range_iter(cf, .., false).next()
self.zs_forward_range_iter(cf, ..).next()
}

#[allow(clippy::unwrap_in_result)]
Expand All @@ -340,7 +340,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
{
// Reading individual values from iterators does not seem to cause database hangs.
self.zs_range_iter(cf, .., true).next()
self.zs_reverse_range_iter(cf, ..).next()
}

#[allow(clippy::unwrap_in_result)]
Expand All @@ -351,7 +351,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
{
// Reading individual values from iterators does not seem to cause database hangs.
self.zs_range_iter(cf, lower_bound.., false).next()
self.zs_forward_range_iter(cf, lower_bound..).next()
}

#[allow(clippy::unwrap_in_result)]
Expand All @@ -362,7 +362,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
{
// Reading individual values from iterators does not seem to cause database hangs.
self.zs_range_iter(cf, ..=upper_bound, true).next()
self.zs_reverse_range_iter(cf, ..=upper_bound).next()
}

fn zs_items_in_range_ordered<C, K, V, R>(&self, cf: &C, range: R) -> BTreeMap<K, V>
Expand All @@ -372,7 +372,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
R: RangeBounds<K>,
{
self.zs_range_iter(cf, range, false).collect()
self.zs_forward_range_iter(cf, range).collect()
}

fn zs_items_in_range_unordered<C, K, V, R>(&self, cf: &C, range: R) -> HashMap<K, V>
Expand All @@ -382,7 +382,7 @@ impl ReadDisk for DiskDb {
V: FromDisk,
R: RangeBounds<K>,
{
self.zs_range_iter(cf, range, false).collect()
self.zs_forward_range_iter(cf, range).collect()
}
}

Expand All @@ -402,33 +402,26 @@ impl DiskWriteBatch {
}

impl DiskDb {
/// Returns an iterator over the items in `cf` in `range`.
///
/// Accepts a `reverse` argument. If it is `true`, creates the iterator with an
/// [`IteratorMode`](rocksdb::IteratorMode) of [`End`](rocksdb::IteratorMode::End), or
/// [`From`](rocksdb::IteratorMode::From) with [`Direction::Reverse`](rocksdb::Direction::Reverse).
/// Returns a forward iterator over the items in `cf` in `range`.
///
/// Holding this iterator open might delay block commit transactions.
pub fn zs_range_iter<C, K, V, R>(
pub fn zs_forward_range_iter<C, K, V, R>(
&self,
cf: &C,
range: R,
reverse: bool,
) -> impl Iterator<Item = (K, V)> + '_
where
C: rocksdb::AsColumnFamilyRef,
K: IntoDisk + FromDisk,
V: FromDisk,
R: RangeBounds<K>,
{
self.zs_range_iter_with_direction(cf, range, reverse)
self.zs_range_iter_with_direction(cf, range, false)
}

/// Returns a reverse iterator over the items in `cf` in `range`.
///
/// Holding this iterator open might delay block commit transactions.
///
/// This code is copied from `zs_range_iter()`, but with the mode reversed.
pub fn zs_reverse_range_iter<C, K, V, R>(
&self,
cf: &C,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,13 @@ impl AddressTransaction {
/// address. Starts at the first UTXO, or at the `query` start height, whichever is greater.
/// Ends at the maximum possible transaction index for the end height.
///
/// Used to look up transactions with [`DiskDb::zs_range_iter`][1].
/// Used to look up transactions with [`DiskDb::zs_forward_range_iter`][1].
///
/// The transaction locations in the:
/// - start bound might be invalid, if it is based on the `query` start height.
/// - end bound will always be invalid.
///
/// But this is not an issue, since [`DiskDb::zs_range_iter`][1] will fetch all existing
/// But this is not an issue, since [`DiskDb::zs_forward_range_iter`][1] will fetch all existing
/// (valid) values in the range.
///
/// [1]: super::super::disk_db::DiskDb
Expand Down
2 changes: 1 addition & 1 deletion zebra-state/src/service/finalized_state/zebra_db/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl ZebraDb {
) -> impl Iterator<Item = (RawBytes, Arc<HistoryTree>)> + '_ {
let history_tree_cf = self.db.cf_handle("history_tree").unwrap();

self.db.zs_range_iter(&history_tree_cf, .., false)
self.db.zs_forward_range_iter(&history_tree_cf, ..)
}

// Value pool methods
Expand Down
10 changes: 5 additions & 5 deletions zebra-state/src/service/finalized_state/zebra_db/shielded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl ZebraDb {
&self,
) -> impl Iterator<Item = (RawBytes, Arc<sprout::tree::NoteCommitmentTree>)> + '_ {
let sprout_trees = self.db.cf_handle("sprout_note_commitment_tree").unwrap();
self.db.zs_range_iter(&sprout_trees, .., false)
self.db.zs_forward_range_iter(&sprout_trees, ..)
}

// # Sapling trees
Expand Down Expand Up @@ -209,7 +209,7 @@ impl ZebraDb {
R: std::ops::RangeBounds<Height>,
{
let sapling_trees = self.db.cf_handle("sapling_note_commitment_tree").unwrap();
self.db.zs_range_iter(&sapling_trees, range, false)
self.db.zs_forward_range_iter(&sapling_trees, range)
}

/// Returns the Sapling note commitment trees in the reversed range, in decreasing height order.
Expand Down Expand Up @@ -259,7 +259,7 @@ impl ZebraDb {
.unwrap();

self.db
.zs_range_iter(&sapling_subtrees, range, false)
.zs_forward_range_iter(&sapling_subtrees, range)
.collect()
}

Expand Down Expand Up @@ -335,7 +335,7 @@ impl ZebraDb {
R: std::ops::RangeBounds<Height>,
{
let orchard_trees = self.db.cf_handle("orchard_note_commitment_tree").unwrap();
self.db.zs_range_iter(&orchard_trees, range, false)
self.db.zs_forward_range_iter(&orchard_trees, range)
}

/// Returns the Orchard note commitment trees in the reversed range, in decreasing height order.
Expand Down Expand Up @@ -385,7 +385,7 @@ impl ZebraDb {
.unwrap();

self.db
.zs_range_iter(&orchard_subtrees, range, false)
.zs_forward_range_iter(&orchard_subtrees, range)
.collect()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,7 @@ impl ZebraDb {
AddressTransaction::address_iterator_range(address_location, query_height_range);

self.db
.zs_range_iter(
&tx_loc_by_transparent_addr_loc,
transaction_location_range,
false,
)
.zs_forward_range_iter(&tx_loc_by_transparent_addr_loc, transaction_location_range)
.map(|(tx_loc, ())| tx_loc)
.collect()
}
Expand Down

0 comments on commit 9fec711

Please sign in to comment.