Make eviction less "contentious" (#787)

* move retry logic inside `pop_random`

* don't try to evict after we exceed the maximum number of retries

* move max retries to `TangleConfig`

bee-tangle/src/config.rs

@@ -8,12 +8,14 @@ use std::num::NonZeroUsize;
 const DEFAULT_BELOW_MAX_DEPTH: u32 = 15;
 // SAFETY: initialised with a non-zero value.
 const DEFAULT_NUM_PARTITIONS: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(16) };
+const DEFAULT_MAX_EVICTION_RETRIES: usize = 10;
 
 /// A builder type for a tangle configuration.
 #[derive(Default, Deserialize)]
 pub struct TangleConfigBuilder {
  below_max_depth: Option<u32>,
  num_partitions: Option<NonZeroUsize>,
+ max_eviction_retries: Option<usize>,
 }
 
 impl TangleConfigBuilder {
@@ -27,6 +29,7 @@ impl TangleConfigBuilder {
  TangleConfig {
  below_max_depth: self.below_max_depth.unwrap_or(DEFAULT_BELOW_MAX_DEPTH),
  num_partitions: self.num_partitions.unwrap_or(DEFAULT_NUM_PARTITIONS),
+ max_eviction_retries: self.max_eviction_retries.unwrap_or(DEFAULT_MAX_EVICTION_RETRIES),
  }
  }
 }
@@ -36,6 +39,7 @@ impl TangleConfigBuilder {
 pub struct TangleConfig {
  below_max_depth: u32,
  num_partitions: NonZeroUsize,
+ max_eviction_retries: usize,
 }
 
 impl TangleConfig {
@@ -53,4 +57,9 @@ impl TangleConfig {
  pub fn num_partitions(&self) -> NonZeroUsize {
  self.num_partitions
  }
+
+ /// Get the value of `max_eviction_retries`.
+ pub fn max_eviction_retries(&self) -> usize {
+ self.max_eviction_retries
+ }
 }

bee-tangle/src/tangle.rs

@@ -33,7 +33,6 @@ const DEFAULT_CACHE_LEN: usize = 100_000;
 const CACHE_THRESHOLD_FACTOR: f64 = 0.1;
 const SYNCED_THRESHOLD: u32 = 2;
 const CONFIRMED_THRESHOLD: u32 = 2;
-const MAX_EVICTION_RETRIES: usize = 10;
 
 /// A Tangle wrapper designed to encapsulate milestone state.
 pub struct Tangle<B> {
@@ -608,22 +607,17 @@ impl<B: StorageBackend> Tangle<B> {
 
  async fn perform_eviction(&self) {
  let max_len = self.max_len.load(Ordering::Relaxed);
+ let max_eviction_retries = self.config.max_eviction_retries();
 
  if self.vertices.len() > max_len {
  while self.vertices.len() > ((1.0 - CACHE_THRESHOLD_FACTOR) * max_len as f64) as usize {
- let mut retries = 1;
- loop {
- if self.vertices.pop_random().await.is_none() {
- log::debug!("retrying cache eviction (attempt #{})", retries);
- retries += 1;
- } else {
- break;
- }
+ if self.vertices.pop_random(max_eviction_retries).await.is_none() {
+ log::warn!(
+ "could not perform cache eviction after {} attempts",
+ max_eviction_retries
+ );
 
- if retries > MAX_EVICTION_RETRIES {
- log::warn!("could not perform cache eviction after {} attempts", retries);
- return;
- }
+ break;
  }
  }
  }

bee-tangle/src/vertices.rs

@@ -76,24 +76,32 @@ impl Vertices {
  .ok()
  }
 
- pub(crate) async fn pop_random(&self) -> Option<Vertex> {
- let index = rand::thread_rng().gen_range(0..self.tables.len());
- // SAFETY: `index < self.tables.len()` by construction.
- let mut table = unsafe { self.tables.get_unchecked(index) }.write().await;
+ pub(crate) async fn pop_random(&self, max_retries: usize) -> Option<Vertex> {
+ let mut retries = 0;
+
+ while retries < max_retries {
+ let index = rand::thread_rng().gen_range(0..self.tables.len());
 
- // SAFETY: We are holding the lock over the table, which means that no other thread could have modified, added
- // nor deleted any bucket. This applies to all the following `unsafe` blocks.
- let buckets = unsafe { table.iter() };
+ // SAFETY: `index < self.tables.len()` by construction.
+ if let Ok(mut table) = unsafe { self.tables.get_unchecked(index) }.try_write() {
+ // SAFETY: We are holding the lock over the table, which means that no other thread could have modified,
+ // added nor deleted any bucket. This applies to all the following `unsafe` blocks.
+ let buckets = unsafe { table.iter() };
 
- for bucket in buckets {
- let (_, vertex) = unsafe { bucket.as_ref() };
+  for bucket in buckets {
+  let (_, vertex) = unsafe { bucket.as_ref() };
 
- if vertex.can_evict() {
- self.len.fetch_sub(1, Ordering::Relaxed);
- let (_, vertex) = unsafe { table.remove(bucket) };
+  if vertex.can_evict() {
+  self.len.fetch_sub(1, Ordering::Relaxed);
+  let (_, vertex) = unsafe { table.remove(bucket) };
 
- return Some(vertex);
+ return Some(vertex);
+ }
+ }
  }
+
+ retries += 1;
+ log::debug!("retrying cache eviction (attempt #{})", retries);
  }
 
  None