Skip to content

Commit

Permalink
Avoid taking the write-lock on val monitor
Browse files Browse the repository at this point in the history
  • Loading branch information
paulhauner committed Jun 21, 2021
1 parent 3dc1eb5 commit 280e3d8
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
10 changes: 10 additions & 0 deletions beacon_node/beacon_chain/src/validator_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,16 @@ impl<T: EthSpec> ValidatorMonitor<T> {
.and_then(|pubkey| self.validators.get(pubkey))
}

/// Return `true` if the `validator_index` has been registered with `self`.
pub fn contains_validator(&self, validator_index: u64) -> bool {
self.indices.contains_key(&validator_index)
}

/// Return `true` if the automatic validator registration is enabled.
pub fn auto_register_enabled(&self) -> bool {
self.auto_register
}

/// Returns the number of validators monitored by `self`.
pub fn num_validators(&self) -> usize {
self.validators.len()
Expand Down
24 changes: 20 additions & 4 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1880,10 +1880,26 @@ pub fn serve<T: BeaconChainTypes>(
chain: Arc<BeaconChain<T>>| {
blocking_json_task(move || {
for subscription in &subscriptions {
chain
.validator_monitor
.write()
.auto_register_local_validator(subscription.validator_index);
// Assign the result from the read to a separate variable, to avoid a Rust
// quirk where a read-lock obtained inside an `if` statement is for the
// execution inside it.
//
// Checking the read-lock first helps avoid lengthy waits for the
// write-lock. Although another thread may add the validator between
// dropping the read-lock and obtaining the write-lock, there's no harm in
// registering the same validator twice.
let should_register = {
let validator_monitor = chain.validator_monitor.read();
validator_monitor.auto_register_enabled()
&& validator_monitor
.contains_validator(subscription.validator_index)
};
if should_register {
chain
.validator_monitor
.write()
.auto_register_local_validator(subscription.validator_index);
}

let subscription = api_types::ValidatorSubscription {
validator_index: subscription.validator_index,
Expand Down

0 comments on commit 280e3d8

Please sign in to comment.