Skip to content

Commit

Permalink
conf: check storage or coprocessor readpool config when partially uni…
Browse files Browse the repository at this point in the history
…fied (tikv#7387) (tikv#7513)

Signed-off-by: Yilin Chen <[email protected]>
  • Loading branch information
sre-bot committed Apr 30, 2020
1 parent 9fb341a commit 049a066
Showing 1 changed file with 55 additions and 25 deletions.
80 changes: 55 additions & 25 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1486,6 +1486,9 @@ macro_rules! readpool_config {
}

pub fn validate(&self) -> Result<(), Box<dyn Error>> {
if self.use_unified_pool() {
return Ok(());
}
if self.high_concurrency == 0 {
return Err(format!(
"readpool.{}.high-concurrency should be > 0",
Expand Down Expand Up @@ -1588,6 +1591,11 @@ macro_rules! readpool_config {
assert!(invalid_cfg.validate().is_err());
invalid_cfg.max_tasks_per_worker_low = 100;
assert!(cfg.validate().is_ok());

let mut invalid_but_unified = cfg.clone();
invalid_but_unified.use_unified_pool = Some(true);
invalid_but_unified.low_concurrency = 0;
assert!(invalid_but_unified.validate().is_ok());
}
}
};
Expand Down Expand Up @@ -1684,10 +1692,9 @@ impl ReadPoolConfig {
pub fn validate(&self) -> Result<(), Box<dyn Error>> {
if self.is_unified_pool_enabled() {
self.unified.validate()?;
} else {
self.storage.validate()?;
self.coprocessor.validate()?;
}
self.storage.validate()?;
self.coprocessor.validate()?;
Ok(())
}
}
Expand Down Expand Up @@ -1748,28 +1755,6 @@ mod readpool_tests {

#[test]
fn test_unified_enabled() {
// Allow invalid storage and coprocessor config when yatp is used.
let unified = UnifiedReadPoolConfig::default();
assert!(unified.validate().is_ok());
let storage = StorageReadPoolConfig {
use_unified_pool: Some(true),
high_concurrency: 0,
..Default::default()
};
assert!(storage.validate().is_err());
let coprocessor = CoprReadPoolConfig {
high_concurrency: 0,
..Default::default()
};
assert!(coprocessor.validate().is_err());
let cfg = ReadPoolConfig {
unified,
storage,
coprocessor,
};
assert!(cfg.is_unified_pool_enabled());
assert!(cfg.validate().is_ok());

// Yatp config must be valid when yatp is used.
let unified = UnifiedReadPoolConfig {
min_thread_count: 0,
Expand Down Expand Up @@ -1810,6 +1795,51 @@ mod readpool_tests {
cfg.coprocessor.use_unified_pool = Some(false);
assert!(!cfg.is_unified_pool_enabled());
}

#[test]
fn test_partially_unified() {
let storage = StorageReadPoolConfig {
use_unified_pool: Some(false),
low_concurrency: 0,
..Default::default()
};
assert!(!storage.use_unified_pool());
let coprocessor = CoprReadPoolConfig {
use_unified_pool: Some(true),
..Default::default()
};
assert!(coprocessor.use_unified_pool());
let mut cfg = ReadPoolConfig {
storage,
coprocessor,
..Default::default()
};
assert!(cfg.is_unified_pool_enabled());
assert!(cfg.validate().is_err());
cfg.storage.low_concurrency = 1;
assert!(cfg.validate().is_ok());

let storage = StorageReadPoolConfig {
use_unified_pool: Some(true),
..Default::default()
};
assert!(storage.use_unified_pool());
let coprocessor = CoprReadPoolConfig {
use_unified_pool: Some(false),
low_concurrency: 0,
..Default::default()
};
assert!(!coprocessor.use_unified_pool());
let mut cfg = ReadPoolConfig {
storage,
coprocessor,
..Default::default()
};
assert!(cfg.is_unified_pool_enabled());
assert!(cfg.validate().is_err());
cfg.coprocessor.low_concurrency = 1;
assert!(cfg.validate().is_ok());
}
}

#[derive(Clone, Serialize, Deserialize, PartialEq, Debug, Configuration)]
Expand Down

0 comments on commit 049a066

Please sign in to comment.