diff --git a/src/lite/bridge.clj b/src/lite/bridge.clj index 60ea52a..cd63f1a 100644 --- a/src/lite/bridge.clj +++ b/src/lite/bridge.clj @@ -12,22 +12,13 @@ (defrecord AdapterClient [adapter connection] jepsen.client/Client (open! [this _test _node] - ;; Jepsen opens a client per worker process, and again whenever a process - ;; crashes and is replaced -- hence the adapter's re-runnable `open`. (target/acquire! connection) this) (setup! [this _test] - ;; Reserved for workloads that need one-time initialization. this) (invoke! [_this _test op] - ;; Read the conn per op, never once at open!: a fault may have replaced the - ;; target since the last op, and this client has to follow it. - ;; - ;; The adapter's `invoke` runs the user's handler through the - ;; exception -> :type wrapper, which merges onto the invocation op and so - ;; preserves :process and :f. Jepsen rejects completions that don't. (client/invoke adapter (target/current connection) op)) (teardown! [_this _test] diff --git a/src/lite/client.clj b/src/lite/client.clj index e89be4e..67e3780 100644 --- a/src/lite/client.clj +++ b/src/lite/client.clj @@ -88,10 +88,6 @@ (complete handler conn op)) (close [_ conn] - ;; Nil is the only lifecycle edge Lite can make harmless on the user's - ;; behalf. A repeated non-nil close still reaches `close-fn`, whose resource - ;; may already have been closed; like every ClientAdapter close, it must be - ;; safe to call again. (when conn (close-fn conn)))) diff --git a/src/lite/core.clj b/src/lite/core.clj index 2826952..92d5d07 100644 --- a/src/lite/core.clj +++ b/src/lite/core.clj @@ -1,9 +1,4 @@ (ns lite.core - "The runner: config in, verdict out. - - A run is Jepsen's own pipeline with the parts a Lite user shouldn't have to - see kept internal -- generator -> the user's adapter (via the bridge) -> - history -> the workload's checker." (:require [clojure.pprint :as pprint] [jepsen.core :as jepsen] [jepsen.generator :as gen] @@ -21,77 +16,36 @@ [lite.target.local-process] [lite.workload :as workload])) -(def default-nodes - "An in-process target is one logical node. Later target-types supply their - own; nothing here branches on which." - ["local"]) - -(def default-target - {:type :in-process}) - (defn validate! - "Checks the combinations a run can't recover from -- an unrunnable - target-type, or a fault that target-type can't inject -- before anything is - built, opened or generated. Throws with an explanation; returns config." + "Checks the given target-type and faults. + Throws with an explanation; returns config." [{:keys [target nemesis]}] - (let [target (or target default-target)] - (lite.target/validate! target) - ;; Two questions in order. First: can a target of this kind be given these - ;; faults at all -- the axis-2 table, the same answer on every machine. - ;; Then: can *this* machine carry them out, which is where a power-off asks - ;; for Linux and FUSE. Answering them the other way round would tell a user - ;; their host is wrong about a fault their target-type could never do. - (nemesis/validate! (:type target) nemesis) - (lite.target/verify-faults! target nemesis)) + (lite.target/validate! target) + (nemesis/validate! (:type target) nemesis) + (lite.target/verify-faults! target nemesis) nil) -(defn- check-concurrency! +(defn- adjust-concurrency! "Some workloads split workers into fixed-size groups and need a worker count that divides evenly. Say so plainly rather than letting an assertion fire from inside the generator." - [workload-name {:keys [concurrency-multiple]} concurrency] - (when (and concurrency-multiple - (pos? concurrency-multiple) - (not (zero? (mod concurrency concurrency-multiple)))) - (throw (ex-info - (str "The " workload-name " workload can't run with :concurrency " - concurrency ".\n\n" - " why: it works each key with a group of " concurrency-multiple - " threads, so the worker count has to divide into whole" - " groups.\n\n" - " fix: use a multiple of " concurrency-multiple ", such as " - (* concurrency-multiple (max 1 (quot concurrency - concurrency-multiple))) - " or " (* concurrency-multiple - (inc (quot concurrency concurrency-multiple))) - ", or leave :concurrency out and let the workload choose.") - {:lite/error :invalid-concurrency - :workload workload-name - :concurrency concurrency - :multiple concurrency-multiple}))) - concurrency) + [workload-name concurrency] + (cond + (not (pos? concurrency)) (throw (ex-info "concurrency should be positive" + {:lite/error :invalid-concurrency + :concurrency concurrency})) + (= workload-name :register) (* (quot (inc concurrency) 2) 2) + :else concurrency)) (defn test-map "Builds the Jepsen test map for `config`. Everything not named here keeps `noop-test`'s defaults: a noop os/db is correct for an in-process target, which has no node to configure." [{:keys [adapter handler workload workload-opts concurrency time-limit name - nodes target nemesis nemesis-opts]}] - (let [nodes (or nodes default-nodes) - target (or target default-target) - w-name (or workload :register) - w (workload/build w-name - (cond-> (assoc workload-opts :nodes nodes) - ;; With a clock to run against, let the ops - ;; run until time is up rather than stopping - ;; at a workload's default op count. - (and time-limit - (not (contains? workload-opts :op-limit))) - (assoc :op-limit false))) - concurrency (check-concurrency! - w-name w (or concurrency (:concurrency w) (count nodes))) - ;; `invoke` takes no handler argument, so the adapter carries it. The - ;; config is the user-facing place to put it; bind it in here. + target nemesis nemesis-opts] + :or {workload :register concurrency 4 time-limit 1}}] + (let [w (workload/build workload workload-opts) + concurrency (adjust-concurrency! workload concurrency) adapter (cond-> adapter handler (assoc :handler handler)) conn (lite.target/build target adapter) nem (nemesis/build (:type target) conn @@ -102,34 +56,16 @@ (:test-opts w) {:pure-generators true :name (or name "jepsen-lite") - :nodes nodes + :nodes ["dummy"] :concurrency concurrency :client (cond-> (bridge/client adapter conn) (:wrap-client w) ((:wrap-client w))) :generator (gen/phases - ;; One clock over both. The nemesis has to be - ;; inside the time limit, not beside it: a fault - ;; schedule that outlived the clients would keep - ;; a `:time-limit 5` run going for as long as it - ;; had faults left to inject. - (cond->> (:generator w) - nem (gen/nemesis (:generator nem)) - time-limit (gen/time-limit time-limit)) - - ;; Then undo whatever is still in force. A run - ;; that stopped mid-pause or mid-partition would - ;; otherwise take its closing reads against a - ;; target that can't answer. - (when (:final-generator nem) - (gen/nemesis (:final-generator nem))) - - ;; And only now whatever the workload has to do - ;; last -- `:set`'s read of everything it added. - ;; It has to come after every fault, or it - ;; verifies a target the last faults never - ;; touched. - (when (:final-generator w) - (gen/clients (:final-generator w)))) + (->> (:generator w) + (gen/nemesis (:generator nem)) + (gen/time-limit time-limit)) + (gen/nemesis (:final-generator nem)) + (gen/clients (:final-generator w))) :checker (:checker w) ;; `run` needs the target itself, not just the client wrapped ;; around it, to start and stop it around the run. It holds the @@ -155,11 +91,9 @@ :workload :register ; optional; :register is the default :workload-opts {...} ; optional; see the workload's ns :concurrency ; optional; the workload picks otherwise - :time-limit ; optional; otherwise the run ends after - ; the workload's op count + :time-limit ; optional; 1 second is the default :name \"...\" ; optional - :target {:type :in-process} ; optional; :in-process is the default - ; see lite.target. for its config + :target {:type :in-process} ; see lite.target. for its config :nemesis [:crash] ; optional; faults to inject :nemesis-opts {...}} ; optional; :in-process takes :crashes ; and :crash-interval, the others diff --git a/src/lite/runner.clj b/src/lite/runner.clj index 60e7bf7..c60ecbb 100644 --- a/src/lite/runner.clj +++ b/src/lite/runner.clj @@ -485,16 +485,8 @@ :list (do (print (listing suite)) {:exit-code 0, :action :list, :runs []}) :run (do - ;; Everything that can be known before running is settled first, - ;; so an impossible combination costs nothing and a long suite - ;; doesn't discover it on its last workload. (check-faults! suite request) (let [rows (mapv (fn [workload] - ;; One workload failing is not a reason to - ;; throw away the results of the ones that - ;; already ran -- an eight-workload suite - ;; shouldn't lose seven verdicts because a - ;; target wouldn't start for the eighth. (try (run-one suite request workload) (catch Exception t diff --git a/src/lite/target.clj b/src/lite/target.clj index 1f5f389..fb12f0b 100644 --- a/src/lite/target.clj +++ b/src/lite/target.clj @@ -62,8 +62,10 @@ (defn validate! "Checks a target config can actually be run, before anything is built." [target] - (when-not (contains? (methods build) (:type target)) - (build target nil))) + (if target + (when-not (contains? (methods build) (:type target)) + (build target nil)) + (throw (ex-info "No target type is given." {:lite/error :no-target-type})))) (defmulti verify-faults! "Can *this host* actually inject these faults into this target? diff --git a/src/lite/target/http.clj b/src/lite/target/http.clj index f2e6ad6..540df94 100644 --- a/src/lite/target/http.clj +++ b/src/lite/target/http.clj @@ -51,13 +51,6 @@ (defrecord Http [adapter url address connect-timeout conns] target/Connection (acquire! [this] - ;; Each worker gets a connection of its own to the one shared, external - ;; target -- the way real clients do, and unlike :in-process, where there - ;; is a single instance to hand round. Jepsen runs a worker's - ;; open!/invoke!/close! on one thread, so the thread is the worker's name - ;; here. Opening is a side effect, so it stays outside swap!'s retryable - ;; function; no other thread touches this key, so there is no race to lose. - (verify-reachable! address url connect-timeout) (let [worker (Thread/currentThread)] (when-not (contains? @conns worker) (let [conn (client/open adapter)] @@ -68,8 +61,6 @@ (get @conns (Thread/currentThread))) (release! [_this] - ;; Only this worker's connection. There is no instance to take down with - ;; it: the target outlives the whole run, and outlived its start. (let [worker (Thread/currentThread) [before _] (swap-vals! conns dissoc worker)] (when-let [conn (get before worker)] @@ -85,10 +76,6 @@ (let [{:keys [url connect-timeout]} target address (endpoint/address :http url) timeout (or connect-timeout default-connect-timeout)] - ;; Check once, here, rather than leaving it to the first op: a target that - ;; was never up produces a history of nothing but failures, and a history - ;; of nothing but failures is one most checkers will happily call valid. - ;; A run that can't mean anything shouldn't start. (verify-reachable! address url timeout) (map->Http {:adapter adapter :url url diff --git a/src/lite/target/in_process.clj b/src/lite/target/in_process.clj index 0c05e8e..676dcdd 100644 --- a/src/lite/target/in_process.clj +++ b/src/lite/target/in_process.clj @@ -13,12 +13,6 @@ (defrecord InProcess [adapter conn crashes lifecycle-lock] target/Connection (acquire! [this] - ;; Workers come and go -- and a crashed process opens a fresh client -- but - ;; they all want the instance that's live now, not one of their own. Opening - ;; is a side effect, so it must not run inside swap!'s retryable function. - ;; The same lock covers crash!'s whole close -> open transition: a replacement - ;; worker that sees nil must wait for the graceful close to finish instead of - ;; opening a second instance against the same storage concurrently. (locking lifecycle-lock (when-not @conn (reset! conn (client/open adapter)))) @@ -28,7 +22,6 @@ @conn) (release! [_this] - ;; A worker letting go of its client doesn't take the instance down with it. nil)) (defn crash! diff --git a/src/lite/target/local_process.clj b/src/lite/target/local_process.clj index ef696b3..9f5da53 100644 --- a/src/lite/target/local_process.clj +++ b/src/lite/target/local_process.clj @@ -144,17 +144,11 @@ target/Lifecycle (start! [_this] (locking lifecycle-lock - ;; The filesystem first, then the target on top of it: the process has to - ;; open its data directory *through* lazyfs, or its writes never pass the - ;; layer that a power-off clears. (when (and lazyfs-config (not @lazyfs)) (reset! lazyfs (lazyfs/mount! lazyfs-config))) (when-not @process (let [proc (launch! config)] (reset! process proc) - ;; If the run dies without unwinding -- a Ctrl-C, an OOM -- the - ;; target must not outlive it. Lite started this process; nobody else - ;; is going to clean it up. (let [hook (Thread. ^Runnable (fn [] (.destroyForcibly proc)))] (.addShutdownHook (Runtime/getRuntime) hook) (reset! shutdown-hook hook)) @@ -172,8 +166,6 @@ (warn "The target ignored SIGTERM; killing it") (.destroyForcibly proc)) (when-let [hook @shutdown-hook] - ;; Removing it throws if a shutdown is already under way, which is - ;; exactly when we don't care. (try (.removeShutdownHook (Runtime/getRuntime) hook) (catch IllegalStateException _ nil)) (reset! shutdown-hook nil)) @@ -188,9 +180,6 @@ target/Connection (acquire! [this] - ;; One connection, shared by every worker, as with :in-process: there is - ;; one target process and Lite owns it. A crash replaces the connection -- - ;; that is why it lives in an atom rather than being captured per worker. (locking lifecycle-lock (when-not @conn (reset! conn (client/open adapter)))) @@ -200,7 +189,6 @@ @conn) (release! [_this] - ;; A worker letting go of its client doesn't take the process down with it. nil)) (defn- release-later! @@ -405,8 +393,6 @@ (validate-lazyfs-config! (:lazyfs target)) (map->LocalProcess {:adapter adapter - ;; The address is parsed now, not at start: a typo in :url should be a - ;; sentence before the run, not a timeout in the middle of one. :config (assoc (select-keys target [:command :dir :env :log :url :ready-timeout]) :address (when (:url target) diff --git a/src/lite/workload/bank.clj b/src/lite/workload/bank.clj index 143c7a6..68b0d2b 100644 --- a/src/lite/workload/bank.clj +++ b/src/lite/workload/bank.clj @@ -27,28 +27,15 @@ (defn workload "Options: - :op-limit Total ops (default 200), or false for as many as the - run has time for. :negative-balances? If true, balances may go below zero (default false)." - [{:keys [op-limit negative-balances?] - :or {op-limit 200, negative-balances? false}}] + [{:keys [negative-balances?] + :or {negative-balances? false}}] (let [defaults (bank/test {:negative-balances? negative-balances?}) accounts (:accounts defaults) opening (zipmap accounts - ;; The first account starts with the lot, the rest - ;; empty. Every account is named, so a read covers them - ;; all from the first op onwards. (cons (:total-amount defaults) (repeat 0)))] {:generator (gen/phases - ;; Opening the accounts is the workload's business, not the - ;; target's: it goes through the same handler as everything - ;; else, and finishes before the first transfer. (gen/clients {:f :init, :value opening}) - (gen/clients (cond->> (:generator defaults) - op-limit (gen/limit op-limit)))) - ;; bank/test also composes in a gnuplot-backed plotter; the invariant is - ;; the part that decides the verdict, and it needs no external tools. + (gen/clients (:generator defaults))) :checker (bank/checker {:negative-balances? negative-balances?}) - ;; The generator and checker read these from the test map. - :test-opts (select-keys defaults [:accounts :total-amount :max-transfer]) - :concurrency 4})) + :test-opts (select-keys defaults [:accounts :total-amount :max-transfer])})) diff --git a/src/lite/workload/counter.clj b/src/lite/workload/counter.clj index 755270e..0c91ba4 100644 --- a/src/lite/workload/counter.clj +++ b/src/lite/workload/counter.clj @@ -20,12 +20,6 @@ (defn r [_test _ctx] {:f :read}) (defn workload - "Options: - - :op-limit Total ops (default 200), or false for as many as the run has - time for." - [{:keys [op-limit] :or {op-limit 200}}] - {:generator (gen/clients (cond->> (gen/mix [add add r]) - op-limit (gen/limit op-limit))) - :checker (checker/counter) - :concurrency 4}) + [_opts] + {:generator (gen/clients (gen/mix [add add r])) + :checker (checker/counter)}) diff --git a/src/lite/workload/register.clj b/src/lite/workload/register.clj index 56d7730..cc238de 100644 --- a/src/lite/workload/register.clj +++ b/src/lite/workload/register.clj @@ -20,7 +20,6 @@ (:require [jepsen.checker :as checker] [jepsen.checker.timeline :as timeline] [jepsen.client :as jepsen.client] - [jepsen.generator :as gen] [jepsen.independent :as independent] [jepsen.tests.linearizable-register :as lr] [knossos.model :as model])) @@ -50,29 +49,18 @@ (defn workload "Options: - :nodes The run's nodes; only the count matters -- the workload - gives each key 2 threads per node. :per-key-limit Max ops per register (default 20). Keep this small: it bounds the length of each history Knossos must check. - :process-limit Max processes touching one register (default 10). - :op-limit Total ops (default 100), or false for as many as the run - has time for; the underlying generator walks an unbounded - key sequence, so one bound or the other has to end it." - [{:keys [nodes per-key-limit process-limit op-limit] - :or {per-key-limit 20, process-limit 10, op-limit 100}}] - {:generator (cond->> (:generator (lr/test {:nodes nodes - :per-key-limit per-key-limit - :process-limit process-limit})) - op-limit (gen/limit op-limit)) + :process-limit Max processes touching one register (default 10)." + [{:keys [per-key-limit process-limit] + :or {per-key-limit 20, process-limit 10}}] + {:generator (:generator (lr/test {:nodes ["dummy"] + :per-key-limit per-key-limit + :process-limit process-limit})) :checker (independent/checker (checker/compose {:linearizable (checker/linearizable {:model (model/cas-register) :algorithm :linear}) :timeline (timeline/html)})) - :wrap-client ->PerKeyClient - ;; Keys are worked on by groups of 2 threads per node, and the generator - ;; insists the worker count divide evenly into groups -- so a caller who - ;; picks their own :concurrency has to pick a multiple of the group size. - :concurrency (* 2 (count nodes)) - :concurrency-multiple (* 2 (count nodes))}) + :wrap-client ->PerKeyClient}) diff --git a/src/lite/workload/set.clj b/src/lite/workload/set.clj index 592693d..460b4b6 100644 --- a/src/lite/workload/set.clj +++ b/src/lite/workload/set.clj @@ -20,15 +20,7 @@ (map (fn [x] {:f :add, :value x}) (range))) (defn workload - "Options: - - :op-limit How many adds to attempt (default 200), or false for as many - as the run has time for." - [{:keys [op-limit] :or {op-limit 200}}] - {:generator (gen/clients (cond->> (adds) - op-limit (gen/limit op-limit))) - ;; The read runs after the adds are done, and after any time limit has - ;; expired: without it there is nothing to check the adds against. + [_opts] + {:generator (gen/clients (adds)) :final-generator (gen/clients (gen/once {:f :read})) - :checker (checker/set) - :concurrency 4}) + :checker (checker/set)}) diff --git a/test/lite/compose_docker_test.clj b/test/lite/compose_docker_test.clj index ccef35e..6fe1597 100644 --- a/test/lite/compose_docker_test.clj +++ b/test/lite/compose_docker_test.clj @@ -57,7 +57,7 @@ (is (skip "compose: crash")) (let [{:keys [valid? history]} (core/run (config :counter {:nemesis [:crash] - :time-limit 60 + :time-limit 30 :nemesis-opts {:faults 2 :fault-interval 5}})) crashes (filter (fn [op] (and (= :crash (:f op)) @@ -80,7 +80,7 @@ :request-timeout (java.time.Duration/ofSeconds 2)}) :nemesis [:partition] - :time-limit 60 + :time-limit 30 :nemesis-opts {:faults 2 :fault-interval 5}}))] (is (seq (filter (comp #{:partition} :f) history))) @@ -93,7 +93,7 @@ (deftest the-target-is-torn-down-afterwards (if-not (docker-available?) (is (skip "compose: teardown")) - (do (core/run (config :counter {:workload-opts {:op-limit 20}})) + (do (core/run (config :counter {})) (testing "the container is gone once the run is over" (let [{:keys [out]} (shell/sh "docker" "ps" "--filter" "name=jepsen-lite-kvs" "--format" "{{.Names}}")] diff --git a/test/lite/core_test.clj b/test/lite/core_test.clj index 329b663..ff6eb27 100644 --- a/test/lite/core_test.clj +++ b/test/lite/core_test.clj @@ -96,9 +96,23 @@ ;; nothing to check the adds against, and the verdict is :unknown. (is (true? valid?))))) +(deftest a-config-without-a-target-is-rejected + ;; There is no default target-type. A run has to say what it is testing + ;; against, and saying nothing is a mistake to report rather than a silent + ;; choice of in-process on the user's behalf. + (testing "validation refuses it" + (let [e (is (thrown? clojure.lang.ExceptionInfo + (core/validate! {:nemesis [:crash]})))] + (is (= :no-target-type (:lite/error (ex-data e)))))) + + (testing "so the run never starts" + (let [e (is (thrown? clojure.lang.ExceptionInfo + (core/run (dissoc (targets/config :register) + :target))))] + (is (= :no-target-type (:lite/error (ex-data e))))))) + (deftest a-worker-count-a-workload-cannot-use-is-rejected (let [e (is (thrown? clojure.lang.ExceptionInfo (core/run (assoc (targets/config :register) - :concurrency 5))))] - (is (= :invalid-concurrency (:lite/error (ex-data e)))) - (is (re-find #"multiple of 2" (ex-message e))))) + :concurrency -1))))] + (is (= :invalid-concurrency (:lite/error (ex-data e)))))) diff --git a/test/lite/power_off_test.clj b/test/lite/power_off_test.clj index 35a6bdb..3fa1ca7 100644 --- a/test/lite/power_off_test.clj +++ b/test/lite/power_off_test.clj @@ -262,7 +262,7 @@ :root (str (io/file base "root"))}] (try (core/run (config :counter lazyfs-config :durable - {:workload-opts {:op-limit 20}})) + {:workload-opts {:time-limit 5}})) (testing "no FUSE mount is left behind to break the next run" (is (not (str/includes? (:out (shell/sh "mount")) mount)))) (finally