Refactoring

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]

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))))
 

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   <n>                ; optional; the workload picks otherwise
-      :time-limit    <seconds>          ; optional; otherwise the run ends after
-                                        ; the workload's op count
+      :time-limit    <seconds>          ; optional; 1 second is the default
       :name          \"...\"            ; optional
-      :target        {:type :in-process} ; optional; :in-process is the default
-                                        ; see lite.target.<type> for its config
+      :target        {:type :in-process} ; see lite.target.<type> for its config
       :nemesis       [:crash]           ; optional; faults to inject
       :nemesis-opts  {...}}             ; optional; :in-process takes :crashes
                                         ; and :crash-interval, the others

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

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?

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

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!

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)

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])}))

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)})