Introduce property tests for stable throttle (#660)

* Introduce property tests for stable throttle We have, lately, wondered if the stable throttle was giving too much capacity to its clients. This PR is an attempt to answer that question through property testing. Note I have separated out the async and non-async bits of `Stable` and am property testing the non-async bit, making it feasible to do this kind of examination. I suspect I can do the same for the predictive throttle. Signed-off-by: Brian L. Troutwine <[email protected]> * add test config Signed-off-by: Brian L. Troutwine <[email protected]> * always check granted requests Signed-off-by: Brian L. Troutwine <[email protected]> * Shift throttle code to lading_throttle, kani I would really like to establish that the stable throttle behaves the way that we expect. This commit does a few things. Firstly we move the throttle code to a crate with minimal dependencies, allowing us to run the kani proof tool on that new crate. Second, I introduce a kani experiment for the major property test. This takes... some time to run and as of this writing I'm not sure of the results. Finally, the predictive throttle is gone. In practice it wasn't used because its results were too surprising. Signed-off-by: Brian L. Troutwine <[email protected]> * PR review Signed-off-by: Brian L. Troutwine <[email protected]> * CI fixes Signed-off-by: Brian L. Troutwine <[email protected]> * property ding Signed-off-by: Brian L. Troutwine <[email protected]> * checkpoint Signed-off-by: Brian L. Troutwine <[email protected]> * checkpoint Signed-off-by: Brian L. Troutwine <[email protected]> * pass static Signed-off-by: Brian L. Troutwine <[email protected]> * checkpoint Signed-off-by: Brian L. Troutwine <[email protected]> * further shrink Signed-off-by: Brian L. Troutwine <[email protected]> * checkpoint Signed-off-by: Brian L. Troutwine <[email protected]> * draw down on the interval cell, not add to Signed-off-by: Brian L. Troutwine <[email protected]> * re-expand experiments, fix kani proof Signed-off-by: Brian L. Troutwine <[email protected]> * 0.17.5-rc1 Signed-off-by: Brian L. Troutwine <[email protected]> * comment fix Signed-off-by: Brian L. Troutwine <[email protected]> * update README Signed-off-by: Brian L. Troutwine <[email protected]> * 0.18.0 Signed-off-by: Brian L. Troutwine <[email protected]> * install protobuf in release Signed-off-by: Brian L. Troutwine <[email protected]> --------- Signed-off-by: Brian L. Troutwine <[email protected]>
DataDog · Aug 7, 2023 · 0c3707b · 0c3707b
1 parent 1a5c025
commit 0c3707b
Show file tree

Hide file tree

Showing 28 changed files with 469 additions and 671 deletions.
diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml
@@ -110,6 +110,9 @@ jobs:
     steps:
       - uses: actions/checkout@v2
 
+      - name: Install protobuf
+        run: sudo apt-get update && sudo apt-get install -y protobuf-compiler
+
       - name: Publish to crates.io
         run: |
           cargo publish \

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+## [0.18.0]
+### Changed
+- The predictive throttle no longer exists. The only options are stable and
+  all-out.
+
 ## [0.17.4]
 ### Added
 - `lading` and `lading-capture` releases are now published on crates.io.

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -3,13 +3,19 @@ members = [
         "integration/sheepdog",
         "integration/ducks",
         "lading",
-        "lading_capture"
+        "lading_capture",
+        "lading_throttle"
 ]
 
 [workspace.dependencies]
-uuid =  { version = "1.2", default-features = false, features = ["v4", "serde"] }
+metrics = { version = "0.21" }
 serde = { version = "1.0", features = ["std", "derive"] }
 serde_json = { version = "1.0", features = ["std"] }
+thiserror = { version = "1.0" }
+tokio = { version = "1.29" }
+tracing = { version = "0.1" }
+uuid =  { version = "1.2", default-features = false, features = ["v4", "serde"] }
+
 
 [profile.release]
 lto = true        # Optimize our binary at link stage.

diff --git a/lading/Cargo.toml b/lading/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "lading"
-version = "0.17.4"
+version = "0.18.0"
 authors = ["Brian L. Troutwine <[email protected]>", "George Hahn <[email protected]"]
 edition = "2021"
 license = "MIT"
@@ -11,8 +11,8 @@ description = "A tool for load testing daemons."
 
 [dependencies]
 lading-capture = { version = "0.1", path = "../lading_capture" }
+lading-throttle = { path = "../lading_throttle" }
 
-async-trait = { version = "0.1", default-features = false, features = [] }
 byte-unit = { version = "4.0", features = ["serde"] }
 bytes = { version = "1.4.0", default-features = false, features = ["std"] }
 clap = { version = "3.2", default-features = false, features = ["std", "color", "suggestions", "derive"] }
@@ -22,7 +22,7 @@ http = "0.2"
 http-serde = "1.1"
 hyper = { version = "0.14", features = ["client"] }
 is_executable = "1.0.1"
-metrics = { version = "0.21", default-features = false }
+metrics = { workspace = true }
 metrics-exporter-prometheus = { version = "0.12.1", default-features = false, features = ["http-listener"]}
 metrics-util = { version = "0.15" }
 nix = { version = "0.26" }
@@ -39,9 +39,9 @@ serde_json = {workspace = true }
 serde_qs = "0.12"
 serde_tuple = { version = "0.5", default-features = false }
 serde_yaml = "0.9"
-thiserror = { version = "1.0" }
+thiserror = { workspace = true }
 time = { version = "0.3", features = ["formatting"] }
-tokio = { version = "1.29", features = ["rt", "rt-multi-thread", "macros", "fs", "io-util", "process", "signal", "time", "net"] }
+tokio = { workspace = true, features = ["rt", "rt-multi-thread", "macros", "fs", "io-util", "process", "signal", "time", "net"] }
 tokio-util = { version = "0.7", features = ["io"] }
 tonic = { version = "0.9" }
 tower = { version = "0.4", default-features = false, features = ["timeout", "limit", "load-shed"] }

diff --git a/lading/proptest-regressions/payload/dogstatsd.txt b/lading/proptest-regressions/payload/dogstatsd.txt
@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc d9a4fb20b7328d6983ce4d71a10a02073955cae69a9f75c6d6dd60a959a6e756 # shrinks to seed = 0, max_bytes = 0
diff --git a/lading/proptest-regressions/throttle/stable.txt b/lading/proptest-regressions/throttle/stable.txt
@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc 08df0e78646a65c4d6f6d21db705b6cd3e675060117d2045a79c9488f39a8ff0 # shrinks to maximum_capacity = 1, last_tick = 0, mut ticks_requests = [(0, 1)]
diff --git a/lading/src/config.rs b/lading/src/config.rs
@@ -73,8 +73,6 @@ mod tests {
 
     use http::HeaderMap;
 
-    use crate::throttle;
-
     use super::*;
 
     #[test]
@@ -126,7 +124,7 @@ blackhole:
                         .unwrap(),
                         block_sizes: Option::default(),
                         parallel_connections: 5,
-                        throttle: throttle::Config::default(),
+                        throttle: lading_throttle::Config::default(),
                     }),
                 }],
                 blackhole: Some(vec![

diff --git a/lading/src/generator/file_gen.rs b/lading/src/generator/file_gen.rs
@@ -24,6 +24,7 @@ use std::{
 
 use byte_unit::{Byte, ByteUnit};
 use futures::future::join_all;
+use lading_throttle::Throttle;
 use metrics::{gauge, register_counter};
 use rand::{prelude::StdRng, SeedableRng};
 use serde::Deserialize;
@@ -38,7 +39,6 @@ use crate::{
     block::{self, chunk_bytes, construct_block_cache, Block},
     payload,
     signals::Shutdown,
-    throttle::{self, Throttle},
 };
 
 use super::General;
@@ -98,7 +98,7 @@ pub struct Config {
     rotate: bool,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 #[derive(Debug)]
@@ -178,8 +178,7 @@ impl FileGen {
         let block_cache = Arc::new(block_cache);
 
         for _ in 0..config.duplicates {
-            let throttle =
-                Throttle::new_with_config(config.throttle, bytes_per_second, labels.clone());
+            let throttle = Throttle::new_with_config(config.throttle, bytes_per_second);
 
             let child = Child {
                 path_template: config.path_template.clone(),

diff --git a/lading/src/generator/file_tree.rs b/lading/src/generator/file_tree.rs
@@ -10,6 +10,7 @@
 //! configured [throttle].
 //!
 
+use lading_throttle::Throttle;
 use rand::{
     distributions::{Alphanumeric, DistString},
     seq::SliceRandom,
@@ -28,10 +29,7 @@ use serde::Deserialize;
 use tokio::{fs::create_dir, fs::rename, fs::File};
 use tracing::info;
 
-use crate::{
-    signals::Shutdown,
-    throttle::{self, Throttle},
-};
+use crate::signals::Shutdown;
 
 static FILE_EXTENSION: &str = "txt";
 
@@ -107,7 +105,7 @@ pub struct Config {
     pub rename_per_second: NonZeroU32,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 #[derive(Debug)]
@@ -136,15 +134,13 @@ impl FileTree {
         let mut rng = StdRng::from_seed(config.seed);
         let (nodes, _total_files, total_folder) = generate_tree(&mut rng, config);
 
-        let labels = vec![
+        let _labels = vec![
             ("component".to_string(), "generator".to_string()),
             ("component_name".to_string(), "file_tree".to_string()),
         ];
 
-        let open_throttle =
-            Throttle::new_with_config(config.throttle, config.open_per_second, labels.clone());
-        let rename_throttle =
-            Throttle::new_with_config(config.throttle, config.rename_per_second, labels);
+        let open_throttle = Throttle::new_with_config(config.throttle, config.open_per_second);
+        let rename_throttle = Throttle::new_with_config(config.throttle, config.rename_per_second);
         Ok(Self {
             name_len: config.name_len,
             open_throttle,

diff --git a/lading/src/generator/grpc.rs b/lading/src/generator/grpc.rs
@@ -20,6 +20,7 @@ use std::{
 
 use bytes::{Buf, BufMut, Bytes};
 use http::{uri::PathAndQuery, Uri};
+use lading_throttle::Throttle;
 use metrics::{counter, gauge, register_counter};
 use rand::rngs::StdRng;
 use rand::SeedableRng;
@@ -34,7 +35,6 @@ use crate::{
     block::{self, chunk_bytes, construct_block_cache, Block},
     payload,
     signals::Shutdown,
-    throttle::{self, Throttle},
 };
 
 use super::General;
@@ -73,7 +73,7 @@ pub struct Config {
     pub parallel_connections: u16,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 /// No-op tonic codec. Sends raw bytes and returns the number of bytes received.
@@ -202,7 +202,7 @@ impl Grpc {
             .cloned()
             .expect("target_uri should have an RPC path");
 
-        let throttle = Throttle::new_with_config(config.throttle, bytes_per_second, labels.clone());
+        let throttle = Throttle::new_with_config(config.throttle, bytes_per_second);
         Ok(Self {
             target_uri,
             rpc_path,

diff --git a/lading/src/generator/http.rs b/lading/src/generator/http.rs
@@ -19,6 +19,7 @@ use hyper::{
     header::CONTENT_LENGTH,
     Body, HeaderMap, Request, Uri,
 };
+use lading_throttle::Throttle;
 use metrics::{counter, gauge};
 use once_cell::sync::OnceCell;
 use rand::{prelude::StdRng, SeedableRng};
@@ -30,7 +31,6 @@ use crate::{
     block::{self, chunk_bytes, construct_block_cache, Block},
     payload,
     signals::Shutdown,
-    throttle::{self, Throttle},
 };
 
 use super::General;
@@ -71,7 +71,7 @@ pub struct Config {
     pub parallel_connections: u16,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 #[derive(thiserror::Error, Debug)]
@@ -174,11 +174,7 @@ impl Http {
                     method: hyper::Method::POST,
                     headers: config.headers,
                     block_cache,
-                    throttle: Throttle::new_with_config(
-                        config.throttle,
-                        bytes_per_second,
-                        labels.clone(),
-                    ),
+                    throttle: Throttle::new_with_config(config.throttle, bytes_per_second),
                     metric_labels: labels,
                     shutdown,
                 })

diff --git a/lading/src/generator/process_tree.rs b/lading/src/generator/process_tree.rs
@@ -10,11 +10,9 @@
 //! configured [throttle].
 //!
 
-use crate::{
-    signals::Shutdown,
-    throttle::{self, Throttle},
-};
+use crate::signals::Shutdown;
 use is_executable::IsExecutable;
+use lading_throttle::Throttle;
 use nix::{
     sys::wait::{waitpid, WaitPidFlag, WaitStatus},
     unistd::{fork, ForkResult, Pid},
@@ -220,7 +218,7 @@ pub struct Config {
     pub executables: Vec<Executable>,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 impl Config {
@@ -268,13 +266,12 @@ impl ProcessTree {
             Err(e) => return Err(Error::from(e)),
         };
 
-        let labels = vec![
+        let _labels = vec![
             ("component".to_string(), "generator".to_string()),
             ("component_name".to_string(), "process_tree".to_string()),
         ];
 
-        let throttle =
-            Throttle::new_with_config(config.throttle, config.max_tree_per_second, labels);
+        let throttle = Throttle::new_with_config(config.throttle, config.max_tree_per_second);
         match serde_yaml::to_string(config) {
             Ok(serialized) => Ok(Self {
                 lading_path,

diff --git a/lading/src/generator/splunk_hec.rs b/lading/src/generator/splunk_hec.rs
@@ -28,6 +28,7 @@ use http::{
     Method, Request, Uri,
 };
 use hyper::{client::HttpConnector, Body, Client};
+use lading_throttle::Throttle;
 use metrics::{counter, gauge};
 use once_cell::sync::OnceCell;
 use rand::{prelude::StdRng, SeedableRng};
@@ -44,7 +45,6 @@ use crate::{
     payload,
     payload::SplunkHecEncoding,
     signals::Shutdown,
-    throttle::{self, Throttle},
 };
 
 use super::General;
@@ -89,7 +89,7 @@ pub struct Config {
     pub parallel_connections: u16,
     /// The load throttle configuration
     #[serde(default)]
-    pub throttle: throttle::Config,
+    pub throttle: lading_throttle::Config,
 }
 
 #[derive(thiserror::Error, Debug, Clone, Copy)]
@@ -214,7 +214,7 @@ impl SplunkHec {
             uri,
             token: config.token,
             block_cache,
-            throttle: Throttle::new_with_config(config.throttle, bytes_per_second, labels.clone()),
+            throttle: Throttle::new_with_config(config.throttle, bytes_per_second),
             metric_labels: labels,
             shutdown,
         })