rolls back min number of bloom items for debug builds (#17420)

coverage ci builds are have become flaky presumably because of the
overhead added in https://github.com/solana-labs/solana/pull/17236
for very small test clusters.

This commit uses a smaller min number of bloom items condition on that
if debug assertions are enabled or not.

Previous attempt at fixing the flakiness:
https://github.com/solana-labs/solana/pull/17408
This commit is contained in:
behzad nouri 2021-05-23 16:50:19 +00:00 committed by GitHub
parent 8664b2cc39
commit 5567305a5f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 29 additions and 16 deletions

View File

@ -3046,6 +3046,7 @@ pub fn stake_weight_peers(
mod tests { mod tests {
use super::*; use super::*;
use crate::{ use crate::{
crds_gossip_pull::tests::MIN_NUM_BLOOM_FILTERS,
crds_value::{CrdsValue, CrdsValueLabel, Vote as CrdsVote}, crds_value::{CrdsValue, CrdsValueLabel, Vote as CrdsVote},
duplicate_shred::{self, tests::new_rand_shred, MAX_DUPLICATE_SHREDS}, duplicate_shred::{self, tests::new_rand_shred, MAX_DUPLICATE_SHREDS},
}; };
@ -3800,7 +3801,7 @@ mod tests {
cluster_info.set_entrypoint(entrypoint.clone()); cluster_info.set_entrypoint(entrypoint.clone());
let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &HashMap::new()); let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &HashMap::new());
assert!(pings.is_empty()); assert!(pings.is_empty());
assert_eq!(pulls.len(), 64); assert_eq!(pulls.len(), MIN_NUM_BLOOM_FILTERS);
for (addr, msg) in pulls { for (addr, msg) in pulls {
assert_eq!(addr, entrypoint.gossip); assert_eq!(addr, entrypoint.gossip);
match msg { match msg {
@ -3830,7 +3831,7 @@ mod tests {
); );
let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &HashMap::new()); let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &HashMap::new());
assert_eq!(pings.len(), 1); assert_eq!(pings.len(), 1);
assert_eq!(pulls.len(), 64); assert_eq!(pulls.len(), MIN_NUM_BLOOM_FILTERS);
assert_eq!(*cluster_info.entrypoints.read().unwrap(), vec![entrypoint]); assert_eq!(*cluster_info.entrypoints.read().unwrap(), vec![entrypoint]);
} }
@ -4019,7 +4020,7 @@ mod tests {
// fresh timestamp). There should only be one pull request to `other_node` // fresh timestamp). There should only be one pull request to `other_node`
let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes); let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes);
assert!(pings.is_empty()); assert!(pings.is_empty());
assert_eq!(64, pulls.len()); assert_eq!(pulls.len(), MIN_NUM_BLOOM_FILTERS);
assert!(pulls.into_iter().all(|(addr, _)| addr == other_node.gossip)); assert!(pulls.into_iter().all(|(addr, _)| addr == other_node.gossip));
// Pull request 2: pretend it's been a while since we've pulled from `entrypoint`. There should // Pull request 2: pretend it's been a while since we've pulled from `entrypoint`. There should
@ -4027,21 +4028,21 @@ mod tests {
cluster_info.entrypoints.write().unwrap()[0].wallclock = 0; cluster_info.entrypoints.write().unwrap()[0].wallclock = 0;
let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes); let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes);
assert!(pings.is_empty()); assert!(pings.is_empty());
assert_eq!(pulls.len(), 64 * 2); assert_eq!(pulls.len(), 2 * MIN_NUM_BLOOM_FILTERS);
assert!(pulls assert!(pulls
.iter() .iter()
.take(64) .take(MIN_NUM_BLOOM_FILTERS)
.all(|(addr, _)| *addr == other_node.gossip)); .all(|(addr, _)| *addr == other_node.gossip));
assert!(pulls assert!(pulls
.iter() .iter()
.skip(64) .skip(MIN_NUM_BLOOM_FILTERS)
.all(|(addr, _)| *addr == entrypoint.gossip)); .all(|(addr, _)| *addr == entrypoint.gossip));
// Pull request 3: `other_node` is present and `entrypoint` was just pulled from. There should // Pull request 3: `other_node` is present and `entrypoint` was just pulled from. There should
// only be one pull request to `other_node` // only be one pull request to `other_node`
let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes); let (pings, pulls) = cluster_info.new_pull_requests(&thread_pool, None, &stakes);
assert!(pings.is_empty()); assert!(pings.is_empty());
assert_eq!(pulls.len(), 64); assert_eq!(pulls.len(), MIN_NUM_BLOOM_FILTERS);
assert!(pulls.into_iter().all(|(addr, _)| addr == other_node.gossip)); assert!(pulls.into_iter().all(|(addr, _)| addr == other_node.gossip));
} }

View File

@ -468,6 +468,9 @@ impl CrdsGossipPull {
bloom_size: usize, bloom_size: usize,
) -> Vec<CrdsFilter> { ) -> Vec<CrdsFilter> {
const PAR_MIN_LENGTH: usize = 512; const PAR_MIN_LENGTH: usize = 512;
#[cfg(debug_assertions)]
const MIN_NUM_BLOOM_ITEMS: usize = 512;
#[cfg(not(debug_assertions))]
const MIN_NUM_BLOOM_ITEMS: usize = 65_536; const MIN_NUM_BLOOM_ITEMS: usize = 65_536;
let num_items = crds.len() + self.purged_values.len() + self.failed_inserts.len(); let num_items = crds.len() + self.purged_values.len() + self.failed_inserts.len();
let num_items = MIN_NUM_BLOOM_ITEMS.max(num_items); let num_items = MIN_NUM_BLOOM_ITEMS.max(num_items);
@ -646,7 +649,7 @@ impl CrdsGossipPull {
} }
} }
#[cfg(test)] #[cfg(test)]
mod test { pub(crate) mod tests {
use super::*; use super::*;
use crate::cluster_info::MAX_BLOOM_SIZE; use crate::cluster_info::MAX_BLOOM_SIZE;
use crate::contact_info::ContactInfo; use crate::contact_info::ContactInfo;
@ -662,6 +665,11 @@ mod test {
}; };
use std::{iter::repeat_with, time::Duration}; use std::{iter::repeat_with, time::Duration};
#[cfg(debug_assertions)]
pub(crate) const MIN_NUM_BLOOM_FILTERS: usize = 1;
#[cfg(not(debug_assertions))]
pub(crate) const MIN_NUM_BLOOM_FILTERS: usize = 64;
#[test] #[test]
fn test_hash_as_u64() { fn test_hash_as_u64() {
let arr: Vec<u8> = (0..HASH_BYTES).map(|i| i as u8 + 1).collect(); let arr: Vec<u8> = (0..HASH_BYTES).map(|i| i as u8 + 1).collect();
@ -916,7 +924,7 @@ mod test {
} }
assert_eq!(num_inserts, 20_000); assert_eq!(num_inserts, 20_000);
let filters = crds_gossip_pull.build_crds_filters(&thread_pool, &crds, MAX_BLOOM_SIZE); let filters = crds_gossip_pull.build_crds_filters(&thread_pool, &crds, MAX_BLOOM_SIZE);
assert_eq!(filters.len(), 64); assert_eq!(filters.len(), MIN_NUM_BLOOM_FILTERS.max(32));
let hash_values: Vec<_> = crds let hash_values: Vec<_> = crds
.values() .values()
.map(|v| v.value_hash) .map(|v| v.value_hash)
@ -1171,7 +1179,7 @@ mod test {
CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS, CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS,
); );
assert_eq!(rsp[0].len(), 0); assert_eq!(rsp[0].len(), 0);
assert_eq!(filters.len(), 64); assert_eq!(filters.len(), MIN_NUM_BLOOM_FILTERS);
filters.extend({ filters.extend({
// Should return new value since caller is new. // Should return new value since caller is new.
let now = CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS + 1; let now = CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS + 1;
@ -1188,12 +1196,12 @@ mod test {
/*output_size_limit=*/ usize::MAX, /*output_size_limit=*/ usize::MAX,
CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS, CRDS_GOSSIP_PULL_MSG_TIMEOUT_MS,
); );
assert_eq!(rsp.len(), 128); assert_eq!(rsp.len(), 2 * MIN_NUM_BLOOM_FILTERS);
// There should be only one non-empty response in the 2nd half. // There should be only one non-empty response in the 2nd half.
// Orders are also preserved. // Orders are also preserved.
assert!(rsp.iter().take(64).all(|r| r.is_empty())); assert!(rsp.iter().take(MIN_NUM_BLOOM_FILTERS).all(|r| r.is_empty()));
assert_eq!(rsp.iter().filter(|r| r.is_empty()).count(), 127); assert_eq!(rsp.iter().filter(|r| r.is_empty()).count(), rsp.len() - 1);
assert_eq!(rsp.iter().find(|r| !r.is_empty()).unwrap().len(), 1); assert_eq!(rsp.iter().find(|r| r.len() == 1).unwrap().len(), 1);
} }
#[test] #[test]
@ -1338,7 +1346,7 @@ mod test {
if rsp.is_empty() { if rsp.is_empty() {
continue; continue;
} }
assert_eq!(rsp.len(), 64); assert_eq!(rsp.len(), MIN_NUM_BLOOM_FILTERS);
let failed = node let failed = node
.process_pull_response( .process_pull_response(
&mut node_crds, &mut node_crds,

View File

@ -265,6 +265,10 @@ impl CrdsGossipPush {
) { ) {
const BLOOM_FALSE_RATE: f64 = 0.1; const BLOOM_FALSE_RATE: f64 = 0.1;
const BLOOM_MAX_BITS: usize = 1024 * 8 * 4; const BLOOM_MAX_BITS: usize = 1024 * 8 * 4;
#[cfg(debug_assertions)]
const MIN_NUM_BLOOM_ITEMS: usize = 512;
#[cfg(not(debug_assertions))]
const MIN_NUM_BLOOM_ITEMS: usize = CRDS_UNIQUE_PUBKEY_CAPACITY;
let mut rng = rand::thread_rng(); let mut rng = rand::thread_rng();
let need = Self::compute_need(self.num_active, self.active_set.len(), ratio); let need = Self::compute_need(self.num_active, self.active_set.len(), ratio);
let mut new_items = HashMap::new(); let mut new_items = HashMap::new();
@ -281,7 +285,7 @@ impl CrdsGossipPush {
if peers.is_empty() { if peers.is_empty() {
return; return;
} }
let num_bloom_items = CRDS_UNIQUE_PUBKEY_CAPACITY.max(network_size); let num_bloom_items = MIN_NUM_BLOOM_ITEMS.max(network_size);
let shuffle = { let shuffle = {
let mut seed = [0; 32]; let mut seed = [0; 32];
rng.fill(&mut seed[..]); rng.fill(&mut seed[..]);