Merge 'Improve simulator cli' from bit-aloo

1. Moving manual CLI validation into Clap for safer argument handling. 2. Remove deprecated `with_ascii` flag from `PrettyFields` in logger initialization. 3. Remove `log` and `env_logger` dependencies in favor of `tracing` from simulator. Reviewed-by: Jussi Saurio <jussi.saurio@gmail.com> Closes #3533
2026-01-31 13:54:27 +01:00 · 2025-10-05 18:33:55 +03:00
parent b52d6ab056 ff381c1036
commit 90632ad883
11 changed files with 62 additions and 72 deletions
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -2351,13 +2351,11 @@ dependencies = [
 "clap",
 "dirs 6.0.0",
 "either",
- "env_logger 0.11.7",
 "garde",
 "hex",
 "indexmap 2.11.1",
 "itertools 0.14.0",
 "json5",
- "log",
 "notify",
 "parking_lot",
 "rand 0.9.2",
--- a/simulator/Cargo.toml
+++ b/simulator/Cargo.toml
@@ -18,8 +18,6 @@ path = "main.rs"
 turso_core = { workspace = true, features = ["simulator"]}
 rand = { workspace = true }
 rand_chacha = { workspace = true }
-log = "0.4.20"
-env_logger = { workspace = true }
 regex = { workspace = true }
 regex-syntax = { workspace = true, default-features = false, features = [
    "unicode",
--- a/simulator/generation/plan.rs
+++ b/simulator/generation/plan.rs
@@ -21,6 +21,7 @@ use sql_generation::{
        table::SimValue,
    },
 };
+use tracing::error;
 use turso_core::{Connection, Result, StepResult};

 use crate::{
@@ -121,7 +122,7 @@ impl InteractionPlan {
                    let _ = plan[j].split_off(k);
                    break;
                }
-                log::error!("Comparing '{}' with '{}'", interactions[i], plan[j][k]);
+                error!("Comparing '{}' with '{}'", interactions[i], plan[j][k]);
                if interactions[i].contains(plan[j][k].to_string().as_str()) {
                    i += 1;
                    k += 1;
--- a/simulator/main.rs
+++ b/simulator/main.rs
@@ -34,7 +34,7 @@ mod runner;
 mod shrink;

 fn main() -> anyhow::Result<()> {
-    init_logger();
+    init_logger()?;
    let mut cli_opts = SimulatorCLI::parse();
    cli_opts.validate()?;

@@ -622,17 +622,16 @@ fn run_simulation_default(
    result
 }

-fn init_logger() {
+fn init_logger() -> anyhow::Result<()> {
    let file = OpenOptions::new()
        .create(true)
        .write(true)
        .truncate(true)
-        .open("simulator.log")
-        .unwrap();
+        .open("simulator.log")?;

    let requires_ansi = std::io::stdout().is_terminal();

-    let _ = tracing_subscriber::registry()
+    tracing_subscriber::registry()
        .with(
            tracing_subscriber::fmt::layer()
                .with_ansi(requires_ansi)
@@ -642,17 +641,17 @@ fn init_logger() {
        )
        .with(EnvFilter::try_from_default_env().unwrap_or_else(|_| EnvFilter::new("info")))
        .with(
-            #[allow(deprecated)]
            tracing_subscriber::fmt::layer()
                .with_writer(file)
                .with_ansi(false)
-                .fmt_fields(format::PrettyFields::new().with_ansi(false)) // with_ansi is deprecated, but I cannot find another way to remove ansi codes
+                .fmt_fields(format::PrettyFields::new())
                .with_line_number(true)
                .without_time()
                .with_thread_ids(false)
                .map_fmt_fields(|f| f.debug_alt()),
        )
-        .try_init();
+        .try_init()?;
+    Ok(())
 }

 fn banner() {
--- a/simulator/profiles/io.rs
+++ b/simulator/profiles/io.rs
@@ -33,7 +33,7 @@ pub struct LatencyProfile {
    pub enable: bool,
    #[garde(range(min = 0, max = 100))]
    /// Added IO latency probability
-    pub latency_probability: usize,
+    pub latency_probability: u8,
    #[garde(custom(max_dependent(&self.max_tick)))]
    /// Minimum tick time in microseconds for simulated time
    pub min_tick: u64,
--- a/simulator/runner/cli.rs
+++ b/simulator/runner/cli.rs
@@ -12,28 +12,36 @@ use crate::profiles::ProfileType;
 #[command(name = "limbo-simulator")]
 #[command(author, version, about, long_about = None)]
 pub struct SimulatorCLI {
-    #[clap(short, long, help = "set seed for reproducible runs", default_value = None)]
+    #[clap(
+        short,
+        long,
+        help = "set seed for reproducible runs",
+        conflicts_with = "load"
+    )]
    pub seed: Option<u64>,
    #[clap(
        short,
        long,
-        help = "enable doublechecking, run the simulator with the plan twice and check output equality"
+        help = "enable doublechecking, run the simulator with the plan twice and check output equality",
+        conflicts_with = "differential"
    )]
    pub doublecheck: bool,
    #[clap(
        short = 'n',
        long,
        help = "change the maximum size of the randomly generated sequence of interactions",
-        default_value_t = 5000
+        default_value_t = 5000,
+        value_parser = clap::value_parser!(u32).range(1..)
    )]
-    pub maximum_tests: usize,
+    pub maximum_tests: u32,
    #[clap(
        short = 'k',
        long,
        help = "change the minimum size of the randomly generated sequence of interactions",
-        default_value_t = 1000
+        default_value_t = 1000,
+        value_parser = clap::value_parser!(u32).range(1..)
    )]
-    pub minimum_tests: usize,
+    pub minimum_tests: u32,
    #[clap(
        short = 't',
        long,
@@ -41,7 +49,12 @@ pub struct SimulatorCLI {
        default_value_t = 60 * 60 // default to 1 hour
    )]
    pub maximum_time: usize,
-    #[clap(short = 'l', long, help = "load plan from the bug base")]
+    #[clap(
+        short = 'l',
+        long,
+        help = "load plan from the bug base",
+        conflicts_with = "seed"
+    )]
    pub load: Option<String>,
    #[clap(
        short = 'w',
@@ -49,7 +62,11 @@ pub struct SimulatorCLI {
        help = "enable watch mode that reruns the simulation on file changes"
    )]
    pub watch: bool,
-    #[clap(long, help = "run differential testing between sqlite and Limbo")]
+    #[clap(
+        long,
+        help = "run differential testing between sqlite and Limbo",
+        conflicts_with = "doublecheck"
+    )]
    pub differential: bool,
    #[clap(
        long,
@@ -58,19 +75,19 @@ pub struct SimulatorCLI {
    pub enable_brute_force_shrinking: bool,
    #[clap(subcommand)]
    pub subcommand: Option<SimulatorCommand>,
-    #[clap(long, help = "disable BugBase", default_value_t = false)]
+    #[clap(long, help = "disable BugBase")]
    pub disable_bugbase: bool,
-    #[clap(long, help = "disable heuristic shrinking", default_value_t = false)]
+    #[clap(long, help = "disable heuristic shrinking")]
    pub disable_heuristic_shrinking: bool,
-    #[clap(long, help = "disable UPDATE Statement", default_value_t = false)]
+    #[clap(long, help = "disable UPDATE Statement")]
    pub disable_update: bool,
-    #[clap(long, help = "disable DELETE Statement", default_value_t = false)]
+    #[clap(long, help = "disable DELETE Statement")]
    pub disable_delete: bool,
-    #[clap(long, help = "disable CREATE Statement", default_value_t = false)]
+    #[clap(long, help = "disable CREATE Statement")]
    pub disable_create: bool,
-    #[clap(long, help = "disable CREATE INDEX Statement", default_value_t = false)]
+    #[clap(long, help = "disable CREATE INDEX Statement")]
    pub disable_create_index: bool,
-    #[clap(long, help = "disable DROP Statement", default_value_t = false)]
+    #[clap(long, help = "disable DROP Statement")]
    pub disable_drop: bool,
    #[clap(
        long,
@@ -84,11 +101,11 @@ pub struct SimulatorCLI {
        default_value_t = false
    )]
    pub disable_double_create_failure: bool,
-    #[clap(long, help = "disable Select-Limit Property", default_value_t = false)]
+    #[clap(long, help = "disable Select-Limit Property")]
    pub disable_select_limit: bool,
-    #[clap(long, help = "disable Delete-Select Property", default_value_t = false)]
+    #[clap(long, help = "disable Delete-Select Property")]
    pub disable_delete_select: bool,
-    #[clap(long, help = "disable Drop-Select Property", default_value_t = false)]
+    #[clap(long, help = "disable Drop-Select Property")]
    pub disable_drop_select: bool,
    #[clap(
        long,
@@ -110,12 +127,12 @@ pub struct SimulatorCLI {
    pub disable_union_all_preserves_cardinality: bool,
    #[clap(long, help = "disable FsyncNoWait Property", default_value_t = true)]
    pub disable_fsync_no_wait: bool,
-    #[clap(long, help = "disable FaultyQuery Property", default_value_t = false)]
+    #[clap(long, help = "disable FaultyQuery Property")]
    pub disable_faulty_query: bool,
-    #[clap(long, help = "disable Reopen-Database fault", default_value_t = false)]
+    #[clap(long, help = "disable Reopen-Database fault")]
    pub disable_reopen_database: bool,
-    #[clap(long = "latency-prob", help = "added IO latency probability")]
-    pub latency_probability: Option<usize>,
+    #[clap(long = "latency-prob", help = "added IO latency probability", value_parser = clap::value_parser!(u8).range(0..=100))]
+    pub latency_probability: Option<u8>,
    #[clap(long, help = "Minimum tick time in microseconds for simulated time")]
    pub min_tick: Option<u64>,
    #[clap(long, help = "Maximum tick time in microseconds for simulated time")]
@@ -177,13 +194,6 @@ pub enum SimulatorCommand {

 impl SimulatorCLI {
    pub fn validate(&mut self) -> anyhow::Result<()> {
-        if self.minimum_tests < 1 {
-            anyhow::bail!("minimum size must be at least 1");
-        }
-        if self.maximum_tests < 1 {
-            anyhow::bail!("maximum size must be at least 1");
-        }
-
        if self.minimum_tests > self.maximum_tests {
            tracing::warn!(
                "minimum size '{}' is greater than '{}' maximum size, setting both to '{}'",
@@ -191,22 +201,7 @@ impl SimulatorCLI {
                self.maximum_tests,
                self.maximum_tests
            );
-            self.minimum_tests = self.maximum_tests - 1;
-        }
-
-        if self.seed.is_some() && self.load.is_some() {
-            anyhow::bail!("Cannot set seed and load plan at the same time");
-        }
-
-        if self.latency_probability.is_some_and(|prob| prob > 100) {
-            anyhow::bail!(
-                "latency probability must be a number between 0 and 100. Got `{}`",
-                self.latency_probability.unwrap()
-            );
-        }
-
-        if self.doublecheck && self.differential {
-            anyhow::bail!("Cannot run doublecheck and differential testing at the same time");
+            self.minimum_tests = self.maximum_tests;
        }

        Ok(())
--- a/simulator/runner/env.rs
+++ b/simulator/runner/env.rs
@@ -12,6 +12,7 @@ use rand_chacha::ChaCha8Rng;
 use sql_generation::generation::GenerationContext;
 use sql_generation::model::query::transaction::Rollback;
 use sql_generation::model::table::Table;
+use tracing::trace;
 use turso_core::Database;

 use crate::generation::Shadow;
@@ -297,7 +298,8 @@ impl SimulatorEnv {

        let mut opts = SimulatorOpts {
            seed,
-            ticks: rng.random_range(cli_opts.minimum_tests..=cli_opts.maximum_tests),
+            ticks: rng
+                .random_range(cli_opts.minimum_tests as usize..=cli_opts.maximum_tests as usize),
            max_tables: rng.random_range(0..128),
            disable_select_optimizer: cli_opts.disable_select_optimizer,
            disable_insert_values_select: cli_opts.disable_insert_values_select,
@@ -311,8 +313,7 @@ impl SimulatorEnv {
            disable_fsync_no_wait: cli_opts.disable_fsync_no_wait,
            disable_faulty_query: cli_opts.disable_faulty_query,
            page_size: 4096, // TODO: randomize this too
-            max_interactions: rng.random_range(cli_opts.minimum_tests..=cli_opts.maximum_tests)
-                as u32,
+            max_interactions: rng.random_range(cli_opts.minimum_tests..=cli_opts.maximum_tests),
            max_time_simulation: cli_opts.maximum_time,
            disable_reopen_database: cli_opts.disable_reopen_database,
        };
@@ -418,9 +419,7 @@ impl SimulatorEnv {
        }

        if self.connections[connection_index].is_connected() {
-            log::trace!(
-                "Connection {connection_index} is already connected, skipping reconnection"
-            );
+            trace!("Connection {connection_index} is already connected, skipping reconnection");
            return;
        }

--- a/simulator/runner/file.rs
+++ b/simulator/runner/file.rs
@@ -37,7 +37,7 @@ pub(crate) struct SimulatorFile {

    pub(crate) rng: RefCell<ChaCha8Rng>,

-    pub latency_probability: usize,
+    pub latency_probability: u8,

    pub sync_completion: RefCell<Option<turso_core::Completion>>,
    pub queued_io: RefCell<Vec<DelayedIo>>,
--- a/simulator/runner/io.rs
+++ b/simulator/runner/io.rs
@@ -16,7 +16,7 @@ pub(crate) struct SimulatorIO {
    pub(crate) rng: RefCell<ChaCha8Rng>,
    pub(crate) page_size: usize,
    seed: u64,
-    latency_probability: usize,
+    latency_probability: u8,
    clock: Arc<SimulatorClock>,
 }

@@ -27,7 +27,7 @@ impl SimulatorIO {
    pub(crate) fn new(
        seed: u64,
        page_size: usize,
-        latency_probability: usize,
+        latency_probability: u8,
        min_tick: u64,
        max_tick: u64,
    ) -> Result<Self> {
--- a/simulator/runner/memory/file.rs
+++ b/simulator/runner/memory/file.rs
@@ -52,7 +52,7 @@ pub struct MemorySimFile {
    pub closed: Cell<bool>,
    io_tracker: RefCell<IOTracker>,
    pub rng: RefCell<ChaCha8Rng>,
-    pub latency_probability: usize,
+    pub latency_probability: u8,
    clock: Arc<SimulatorClock>,
    fault: Cell<bool>,
 }
@@ -65,7 +65,7 @@ impl MemorySimFile {
        callbacks: CallbackQueue,
        fd: Fd,
        seed: u64,
-        latency_probability: usize,
+        latency_probability: u8,
        clock: Arc<SimulatorClock>,
    ) -> Self {
        Self {
--- a/simulator/runner/memory/io.rs
+++ b/simulator/runner/memory/io.rs
@@ -124,7 +124,7 @@ pub struct MemorySimIO {
    pub nr_run_once_faults: Cell<usize>,
    pub page_size: usize,
    seed: u64,
-    latency_probability: usize,
+    latency_probability: u8,
    clock: Arc<SimulatorClock>,
 }

@@ -135,7 +135,7 @@ impl MemorySimIO {
    pub fn new(
        seed: u64,
        page_size: usize,
-        latency_probability: usize,
+        latency_probability: u8,
        min_tick: u64,
        max_tick: u64,
    ) -> Self {