Skip to content

Commit

Permalink
Change Blockstore max_root from RwLock<Slot> to AtomicU64 (solana-lab…
Browse files Browse the repository at this point in the history
…s#33998)

The Blockstore currently maintains a RwLock<Slot> of the maximum root
it has seen inserted. The value is initialized during
Blockstore::open() and updated during calls to Blockstore::set_roots().
The max root is queried fairly often for several use cases, and caching
the value is cheaper than constructing an iterator to look it up every
time.

However, the access patterns of these RwLock match that of an atomic.
That is, there is no critical section of code that is run while the
lock is head. Rather, read/write locks are acquired in order to read/
update, respectively. So, change the RwLock<u64> to an AtomicU64.
  • Loading branch information
steviez authored Nov 10, 2023
1 parent 60d267a commit b91da22
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 61 deletions.
14 changes: 7 additions & 7 deletions core/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1452,7 +1452,7 @@ impl ExternalRootSource {
pub fn reconcile_blockstore_roots_with_external_source(
external_source: ExternalRootSource,
blockstore: &Blockstore,
// blockstore.last_root() might have been updated already.
// blockstore.max_root() might have been updated already.
// so take a &mut param both to input (and output iff we update root)
last_blockstore_root: &mut Slot,
) -> blockstore_db::Result<()> {
Expand Down Expand Up @@ -1489,7 +1489,7 @@ pub fn reconcile_blockstore_roots_with_external_source(
// Update the caller-managed state of last root in blockstore.
// Repeated calls of this function should result in a no-op for
// the range of `new_roots`.
*last_blockstore_root = blockstore.last_root();
*last_blockstore_root = blockstore.max_root();
} else {
// This indicates we're in bad state; but still don't panic here.
// That's because we might have a chance of recovering properly with
Expand Down Expand Up @@ -2947,7 +2947,7 @@ pub mod test {
reconcile_blockstore_roots_with_external_source(
ExternalRootSource::Tower(tower.root()),
&blockstore,
&mut blockstore.last_root(),
&mut blockstore.max_root(),
)
.unwrap();

Expand Down Expand Up @@ -2983,7 +2983,7 @@ pub mod test {
reconcile_blockstore_roots_with_external_source(
ExternalRootSource::Tower(tower.root()),
&blockstore,
&mut blockstore.last_root(),
&mut blockstore.max_root(),
)
.unwrap();
}
Expand All @@ -3004,14 +3004,14 @@ pub mod test {

let mut tower = Tower::default();
tower.vote_state.root_slot = Some(4);
assert_eq!(blockstore.last_root(), 0);
assert_eq!(blockstore.max_root(), 0);
reconcile_blockstore_roots_with_external_source(
ExternalRootSource::Tower(tower.root()),
&blockstore,
&mut blockstore.last_root(),
&mut blockstore.max_root(),
)
.unwrap();
assert_eq!(blockstore.last_root(), 0);
assert_eq!(blockstore.max_root(), 0);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion core/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,7 +1747,7 @@ fn load_blockstore(
blockstore.shred_timing_point_sender = poh_timing_point_sender;
// following boot sequence (esp BankForks) could set root. so stash the original value
// of blockstore root away here as soon as possible.
let original_blockstore_root = blockstore.last_root();
let original_blockstore_root = blockstore.max_root();

let blockstore = Arc::new(blockstore);
let blockstore_root_scan = BlockstoreRootScan::new(config, blockstore.clone(), exit.clone());
Expand Down
4 changes: 2 additions & 2 deletions gossip/src/duplicate_shred_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl DuplicateShredHandler {
}

fn cache_root_info(&mut self) {
let last_root = self.blockstore.last_root();
let last_root = self.blockstore.max_root();
if last_root == self.last_root && !self.cached_staked_nodes.is_empty() {
return;
}
Expand Down Expand Up @@ -361,7 +361,7 @@ mod tests {

// This proof will be rejected because the slot is too far away in the future.
let future_slot =
blockstore.last_root() + duplicate_shred_handler.cached_slots_in_epoch + start_slot;
blockstore.max_root() + duplicate_shred_handler.cached_slots_in_epoch + start_slot;
let chunks = create_duplicate_proof(
my_keypair.clone(),
None,
Expand Down
2 changes: 1 addition & 1 deletion ledger-tool/src/bigtable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async fn upload(
None => blockstore.get_first_available_block()?,
};

let ending_slot = ending_slot.unwrap_or_else(|| blockstore.last_root());
let ending_slot = ending_slot.unwrap_or_else(|| blockstore.max_root());

while starting_slot <= ending_slot {
let current_ending_slot = min(
Expand Down
Loading

0 comments on commit b91da22

Please sign in to comment.