diff --git a/docs/design/2026_04_26_implemented_raft_learner.md b/docs/design/2026_04_26_implemented_raft_learner.md index 011c5abbe..ffb280433 100644 --- a/docs/design/2026_04_26_implemented_raft_learner.md +++ b/docs/design/2026_04_26_implemented_raft_learner.md @@ -867,8 +867,9 @@ single-process 3-node demo cluster, attaches a learner via ### Milestone 3 — Hardening -- Jepsen workload that exercises learner attach during partition and - promote after heal. +- ~~Jepsen workload that exercises learner attach during partition and + promote after heal.~~ **Implemented** as + `jepsen/src/elastickv/learner_workload.clj`. - Monitoring: `suffrage` label on per-peer Prometheus labels (already exists in `monitoring/raft.go:355`; verify it survives the engine changes). @@ -883,9 +884,19 @@ handling, v2 peers-file suffrage persistence, admin RPC/CLI surface, join-as-learner alarm, monitoring suffrage labels, promotion precondition checks against leader `Progress.Match`, and the operator runbook (`docs/raft_learner_operations.md`). Remaining Milestone 3 hardening is not -claimed shipped here: the learner attach/promote-under-partition Jepsen -workload and a first-class `Status.PerPeer` progress field are still open, and -follower-served read routing remains a separate proposal. +claimed shipped here: a first-class `Status.PerPeer` progress field is still +open, and follower-served read routing remains a separate proposal. The learner +attach/promote-under-partition Jepsen workload has landed; its checker pins +three properties — promotion never outruns catch-up, no acknowledged write lost +across a promotion, and a learner never counted in the voter quorum. + +Note on the first: catch-up is measured against the LEADER's commit index, not +against `min-applied-index`. The engine's own test is +`Match >= min-applied-index`, so an operator who reads the learner's current +`Match` and passes it back satisfies it by construction — and both that broken +call and the correct one (pick a target, wait for `Match` to reach it) end with +`min-applied-index == Match`. That equality distinguishes nothing; only the +leader's position does. ## 7. Risks diff --git a/jepsen/src/elastickv/db.clj b/jepsen/src/elastickv/db.clj index 369d33bc9..3bd806138 100644 --- a/jepsen/src/elastickv/db.clj +++ b/jepsen/src/elastickv/db.clj @@ -62,6 +62,39 @@ (c/upload (str build-dir "/" bin) (str bin-dir "/" bin)) (c/exec :chmod "755" (str bin-dir "/" bin)))))) +(defn raftadmin-binary + "Path to the uploaded raftadmin helper. Exposed so workloads that drive + membership changes do not each hardcode it." + [] + raftadmin-bin) + +(defn parse-raft-status + "Parses `raftadmin status` output into a keyword map. + + Numeric fields come back as longs and quoted strings unquoted, so a caller + can ask for :commit_index or :applied_index without re-deriving the format." + [out] + (->> (clojure.string/split-lines (or out "")) + (keep (fn [line] + (when-let [[_ k v] (re-matches #"\s*([a-z_]+):\s+(.*)" line)] + (let [v (clojure.string/trim v)] + [(keyword k) + (cond + (re-matches #"-?\d+" v) (Long/parseLong v) + (and (> (count v) 1) + (clojure.string/starts-with? v "\"") + (clojure.string/ends-with? v "\"")) + (subs v 1 (dec (count v))) + :else v)])))) + (into {}))) + +(defn raft-status + "Runs `raftadmin status` from node against addr and returns the parsed map." + [node addr] + (parse-raft-status + (c/on node (c/su (c/exec :env "RAFTADMIN_ALLOW_INSECURE=true" + raftadmin-bin addr "status"))))) + (defn- node-addr "Returns host:port for the node and port." [node port] @@ -176,6 +209,13 @@ "for i in $(seq 1 60); do if nc -z -w 1 $1 $2; then exit 0; fi; sleep 1; done; echo \\\"Timed out waiting for $1:$2\\\"; exit 1" "--" (name node) (str p)))))) +(defn voter-peers + "The peers setup! joins as voters: every node after the bootstrap one, + minus any reserved learner candidate." + [nodes reserved] + (let [reserved (when reserved (name reserved))] + (vec (remove #(= reserved (name %)) (rest nodes))))) + (defn- join-node! "Join peer into cluster via raftadmin, executed on bootstrap node." [bootstrap-node leader-addr peer-id peer-addr] @@ -203,7 +243,11 @@ (let [raft-groups (:raft-groups opts) grpc-port (or (:grpc-port opts) 50051) group-ids (when (seq raft-groups) (group-ids raft-groups))] - (doseq [peer (rest (:nodes test))] + ;; A reserved node is deliberately NOT made a voter, so a learner + ;; workload has a non-member to attach. Without this every node is a + ;; voter before the workload starts and :add-learner has nothing to + ;; act on. + (doseq [peer (voter-peers (:nodes test) (:reserve-learner opts))] (util/await-fn (fn [] (try diff --git a/jepsen/src/elastickv/jepsen_test.clj b/jepsen/src/elastickv/jepsen_test.clj index 9de017df0..d3320e13c 100644 --- a/jepsen/src/elastickv/jepsen_test.clj +++ b/jepsen/src/elastickv/jepsen_test.clj @@ -1,6 +1,7 @@ (ns elastickv.jepsen-test (:gen-class) - (:require [elastickv.redis-workload :as redis-workload] + (:require [elastickv.learner-workload :as learner-workload] + [elastickv.redis-workload :as redis-workload] [elastickv.redis-zset-safety-workload :as zset-safety-workload] [elastickv.dynamodb-workload :as dynamodb-workload] [elastickv.dynamodb-types-workload :as dynamodb-types-workload] @@ -28,6 +29,10 @@ ([] (elastickv-zset-safety-test {})) ([opts] (zset-safety-workload/elastickv-zset-safety-test opts))) +(defn elastickv-learner-test + ([] (elastickv-learner-test {})) + ([opts] (learner-workload/elastickv-learner-test opts))) + (def ^:private test-fns "Map of user-facing test names to their constructor fns. The first positional CLI arg selects which workload runs; if absent or unknown, @@ -36,7 +41,8 @@ {"elastickv-test" elastickv-test "elastickv-zset-safety-test" elastickv-zset-safety-test "elastickv-dynamodb-test" elastickv-dynamodb-test - "elastickv-s3-test" elastickv-s3-test}) + "elastickv-s3-test" elastickv-s3-test + "elastickv-learner-test" elastickv-learner-test}) (defn elastickv-sqs-htfifo-test "HT-FIFO Jepsen test (PR 7b). Run via the workload's own -main: diff --git a/jepsen/src/elastickv/learner_workload.clj b/jepsen/src/elastickv/learner_workload.clj new file mode 100644 index 000000000..64072f320 --- /dev/null +++ b/jepsen/src/elastickv/learner_workload.clj @@ -0,0 +1,501 @@ +(ns elastickv.learner-workload + "Jepsen workload for the Raft learner primitive: attach a learner, promote + it under partition, and assert the safety properties the learner design + leaves as Milestone 3 hardening. + + Operations: + + {:f :write :value n} write n to the register + {:f :read :value {:lease? b + :value n}} read the register + {:f :add-learner :value node} attach node as a learner + {:f :promote-learner :value {:node n + :catch-up-target t + :target-source :leader-commit-index + :match m}} + promote, recording the + target the operator + SAMPLED, where it came + from, and the learner's + Match when the promotion + committed + + Properties checked (see `learner-safety-checker`): + + 1. **Promotion never outruns catch-up** — `match >= catch-up-target`, + against the immutable sampled target, plus two guards: a promotion with + no evidence fails closed, and a target not sampled from the leader is a + procedure violation. See `premature-promotions` for why neither + min-applied-index nor the leader's current commit index works alone. + + 2. **No acknowledged write is lost across a promotion** — established by + temporal ordering, not set subtraction. See + `lost-writes-across-promotions`. + + 3. **A learner never counts toward the lease** — isolating a non-voter must + not fail lease reads. Expressed on reads rather than writes because + `quorumAckTracker` gates `LastQuorumAck` and the lease-read fast path, + not the write-commit quorum. See `learner-partition-read-failures`." + (:gen-class) + (:require [clojure.tools.logging :refer [warn]] + [elastickv.cli :as cli] + [elastickv.db :as ekdb] + [jepsen.control :as c] + [jepsen.db :as jdb] + [jepsen.os.debian :as debian] + [jepsen [checker :as checker] + [client :as client] + [control :as control] + [generator :as gen] + [nemesis :as nemesis] + [net :as net] + [os :as os]] + [taoensso.carmine :as car :refer [wcar]])) + +(def default-nodes ["n1" "n2" "n3" "n4" "n5"]) + +;; The last node is reserved as the learner candidate: ElastickvDB's setup +;; otherwise runs `raftadmin add_voter` for every node after the bootstrap +;; one, which leaves no non-member to attach. See ekdb/db :reserve-learner. +(defn learner-candidate + "The node held out of the initial voter set, so :add-learner has something + to attach. Without one, every node is already a voter before the workload + starts and the operation the test is named for cannot run at all." + [nodes] + (last nodes)) + +(defn voter-nodes + "The nodes that join as voters during setup." + [nodes] + (vec (butlast nodes))) +;; --------------------------------------------------------------------------- +;; Pure history analysis +;; --------------------------------------------------------------------------- + +(defn completed-promotions + "Every :promote-learner COMPLETION. + + Completions only, not invocations: a Jepsen operation appears twice in a + history (an :invoke plus an :ok / :fail / :info), so selecting both counted + each promotion twice and reported an invocation with no completion as a + promotion that happened." + [history] + (->> history + (filter #(= :promote-learner (:f %))) + (remove #(= :invoke (:type %))) + vec)) + +(defn unmeasurable-promotions + "Promotions that reported :ok without the evidence needed to judge them. + + Fails closed. A successful promotion whose completion is missing :match or + :catch-up-target -- status collection failed, say -- used to be filtered out + by the numeric guards, so the checker could return :valid? true having + verified catch-up for nothing at all. An unmeasurable promotion is not a + safe one; it is one we cannot vouch for, and it must show up." + [history] + (->> (completed-promotions history) + (filter #(= :ok (:type %))) + (remove (fn [op] + (let [{:keys [match catch-up-target]} (:value op)] + (and (number? match) (number? catch-up-target))))) + vec)) + +(defn premature-promotions + "Promotions that committed while the learner was behind its catch-up target. + + Measured against the target the operator SAMPLED and passed as + min_applied_index, not against the leader's commit index at promotion time. + + Both alternatives are wrong in opposite directions: + + - Comparing min-applied-index with Match alone proves nothing, because the + engine's own test IS `Match >= min_applied_index`. An operator who reads + the learner's current Match and passes it back satisfies that by + construction, so the correct and the broken procedure are + indistinguishable from the pair. + - Comparing Match with the leader's CURRENT commit index rejects the + documented safe workflow. An operator samples commit index T, waits for + the learner to reach T, and promotes; if the leader commits more entries + while that happens the promotion legitimately has `match >= T` but + `match < leader-commit-index`, and the healthy run is marked invalid. + That also contradicts the \"within N entries\" policy in + docs/raft_learner_operations.md. + + The immutable sampled target is the only reference that distinguishes the + two procedures without rejecting the safe one, so the workload records where + its target came from and `promotions-without-a-sampled-target` rejects a + target derived from the learner." + [history] + (->> (completed-promotions history) + (filter #(= :ok (:type %))) + (filter (fn [op] + (let [{:keys [match catch-up-target]} (:value op)] + (and (number? match) + (number? catch-up-target) + (< match catch-up-target))))) + vec)) + +(defn promotions-without-a-sampled-target + "Promotions whose catch-up target did not come from the leader. + + This is what closes the loophole the Match comparison could not: a target + read off the learner's own Match makes the engine's check vacuous, so the + workload records :target-source and anything other than + :leader-commit-index is a procedure violation regardless of the outcome." + [history] + (->> (completed-promotions history) + (filter #(= :ok (:type %))) + (remove #(= :leader-commit-index (:target-source (:value %)))) + vec)) + +(defn- last-ok-write-before + [history t] + (->> history + (filter #(and (= :write (:f %)) (= :ok (:type %)) (< (:time %) t))) + (sort-by :time) + last)) + +(defn- first-ok-read-after + [history t] + (->> history + (filter #(and (= :read (:f %)) (= :ok (:type %)) (> (:time %) t))) + (sort-by :time) + first)) + +(defn- writes-invoked-between + [history from to] + (->> history + (filter #(and (= :write (:f %)) (= :invoke (:type %)) + (> (:time %) from) (< (:time %) to))) + vec)) + +(defn lost-writes-across-promotions + "Acknowledged writes that a promotion lost, by TEMPORAL ordering. + + Set subtraction cannot establish this. `write 1 :ok, write 2 :ok, promote + :ok, read 2 :ok` is a legal register history, but subtracting observed + values from acknowledged ones reports 1 as lost merely because it was + overwritten before anyone read it; and a read of 1 taken BEFORE its write + would mask a genuine later loss. + + So this pairs each promotion with the last write acknowledged before it and + the first read that succeeded after it, and reports a loss only when no + other write was in flight in between -- the case where the register's value + is pinned and the read is obliged to return it. Concurrency makes the + expected value ambiguous rather than wrong, so those cases are skipped + instead of guessed at." + [history] + (->> (completed-promotions history) + (filter #(= :ok (:type %))) + (keep (fn [promotion] + (let [t (:time promotion) + write (last-ok-write-before history t) + read (first-ok-read-after history t)] + ;; A read's :value is the map {:lease? b :value n}, so the + ;; register value has to be unwrapped before comparing it + ;; with the write's scalar. + (let [observed (get-in read [:value :value])] + (when (and write read + (empty? (writes-invoked-between + history (:time write) (:time read))) + (not= (:value write) observed)) + {:promotion (:value promotion) + :acked-write (:value write) + :observed-after observed}))))) + vec)) + +(defn learner-partition-windows + "Every learners-only partition interval, as [start stop] time pairs. + + Each start is paired with its OWN stop. Taking the first start and the + first later stop examined one window and silently ignored every subsequent + learner-isolation period, so a regression in the second or later window + could not fail the check." + [history] + (let [nemesis (->> history (filter #(= :nemesis (:process %))) (sort-by :time)) + starts (->> nemesis + (filter #(= :start-partition (:f %))) + (filter #(= :learners-only (get-in % [:value :scope])))) + stops (->> nemesis (filter #(= :stop-partition (:f %))) (map :time) vec)] + (->> starts + (map (fn [start] + (let [t (:time start)] + [t (or (first (filter #(> % t) stops)) Long/MAX_VALUE)]))) + vec))) + +(defn learner-partition-read-failures + "Lease reads that failed while only learners were partitioned. + + Reads, not writes. The learner is excluded from the write-commit quorum by + the voter set, but `quorumAckTracker` feeds `LastQuorumAck`, which gates the + leader-local LEASE-READ fast path. A learner wrongly counted there does not + stop writes committing -- so a write-failure check stays green through the + exact regression it claims to catch -- it stalls the lease the leader serves + fast reads from. + + So the property is expressed on lease reads: isolating a non-voter must not + make them fail." + [history] + (let [windows (learner-partition-windows history)] + (if (empty? windows) + [] + (->> history + (filter #(and (= :read (:f %)) + (= :fail (:type %)) + (true? (:lease? (:value %))))) + (filter (fn [op] + (some (fn [[start stop]] + (and (>= (:time op) start) (<= (:time op) stop))) + windows))) + vec)))) + +;; --------------------------------------------------------------------------- +;; Checker +;; --------------------------------------------------------------------------- + +(defn learner-safety-checker + "Checks the learner safety properties over a completed history. + + An EMPTY history is invalid. A run that emitted no operations proves + nothing, and reporting it valid is how a workload that cannot actually + drive the cluster still passes -- which is exactly what this workload did + before it had a client, a nemesis, or a generator that produced its + documented operations." + [] + (reify checker/Checker + (check [_ _test history _opts] + (let [promotions (completed-promotions history) + premature (premature-promotions history) + unmeasurable (unmeasurable-promotions history) + unsampled (promotions-without-a-sampled-target history) + lost (lost-writes-across-promotions history) + stalls (learner-partition-read-failures history) + writes (count (filter #(and (= :write (:f %)) (= :ok (:type %))) history)) + reads (count (filter #(and (= :read (:f %)) (= :ok (:type %))) history))] + (when (seq premature) + (warn "learner promoted before reaching its sampled target:" premature)) + (when (seq unmeasurable) + (warn "promotion reported ok without catch-up evidence:" unmeasurable)) + {:valid? (and (pos? (count promotions)) + (pos? writes) + (pos? reads) + (empty? premature) + (empty? unmeasurable) + (empty? unsampled) + (empty? lost) + (empty? stalls)) + :promotions (count promotions) + :ok-writes writes + :ok-reads reads + :premature-promotions premature + :unmeasurable-promotions unmeasurable + :unsampled-targets unsampled + :lost-writes lost + :learner-read-failures stalls})))) + +;; --------------------------------------------------------------------------- +;; Client +;; --------------------------------------------------------------------------- + +(def ^:private register-key "learner-register") + +(defn- raftadmin! + "Runs raftadmin on node against the leader address." + [node & args] + (c/on node (c/su (apply c/exec :env "RAFTADMIN_ALLOW_INSECURE=true" + (ekdb/raftadmin-binary) args)))) + +(defn- leader-commit-index + "Samples the leader's commit index: the catch-up target. + + Sampled from the LEADER because a target read off the learner is what makes + the engine's `Match >= min_applied_index` test vacuous." + [node leader-addr] + (:commit_index (ekdb/raft-status node leader-addr))) + +(defn- learner-applied-index + "The learner's OWN applied index, read from the learner. + + Deliberately not the leader's Match for that peer: an independently observed + measure cannot be satisfied by the leader's own bookkeeping, so it is the + stronger evidence of catch-up. (Per-peer Match is not available from + `raftadmin status` on this branch in any case.)" + [learner-node learner-addr] + (:applied_index (ekdb/raft-status learner-node learner-addr))) + +(def ^:private catch-up-poll-ms 200) +(def ^:private catch-up-timeout-ms 60000) + +(defn- await-catch-up! + "Polls the learner until its applied index reaches target." + [learner-node learner-addr target] + (let [deadline (+ (System/currentTimeMillis) catch-up-timeout-ms)] + (loop [] + (let [applied (or (learner-applied-index learner-node learner-addr) 0)] + (cond + (>= applied target) applied + (> (System/currentTimeMillis) deadline) + (throw (ex-info "learner did not reach the catch-up target" + {:target target :applied applied})) + :else (do (Thread/sleep (long catch-up-poll-ms)) (recur))))))) + +(defrecord LearnerClient [node->port leader-addr conn] + client/Client + (open! [this test node] + (let [port (get node->port node 6379) + host (or (:redis-host test) (name node))] + (assoc this :conn {:pool {} :spec {:host host :port port :timeout-ms 10000}}))) + + (close! [this _test] this) + (setup! [_this _test]) + (teardown! [_this _test]) + + (invoke! [this test op] + (let [conn (:conn this) + nodes (:nodes test) + leader (first nodes) + addr (or leader-addr (str leader ":50051"))] + (try + (case (:f op) + :write (do (wcar conn (car/set register-key (:value op))) + (assoc op :type :ok)) + + ;; :lease? marks the read as one the leader may serve from its + ;; lease, which is the path a learner wrongly counted in + ;; quorumAckTracker would break. + :read (let [v (wcar conn (car/get register-key))] + (assoc op :type :ok + :value {:lease? true + :value (when v (Long/parseLong (str v)))})) + + :add-learner + (let [candidate (name (:value op))] + (raftadmin! leader addr "add_learner" candidate + (str candidate ":" (:grpc-port test 50051)) "0") + (assoc op :type :ok)) + + :promote-learner + (let [candidate (name (:value op)) + candidate-addr (str candidate ":" (:grpc-port test 50051)) + ;; Sample the target FIRST, then wait for the learner to reach + ;; it, then promote against that same immutable value. + ;; Recording where the target came from is what lets the + ;; checker reject the vacuous procedure. + target (leader-commit-index leader addr) + applied (await-catch-up! candidate candidate-addr target)] + (raftadmin! leader addr "promote_learner" candidate + "0" (str target)) + (assoc op :type :ok + :value {:node candidate + :catch-up-target target + :target-source :leader-commit-index + :match applied}))) + (catch Exception e + (assoc op :type :fail :error (.getMessage e))))))) + +;; --------------------------------------------------------------------------- +;; Nemesis +;; --------------------------------------------------------------------------- + +(defn learner-partition-nemesis + "Isolates ONLY the learner candidate, leaving every voter connected. + + That is the shape the quorum property needs: voters retain quorum among + themselves, so anything that degrades must be attributable to the learner + being counted where it should not be." + [nodes] + (let [learner (learner-candidate nodes)] + (nemesis/partitioner + (fn [_test _nodes] + (nemesis/complete-grudge [[learner] (voter-nodes nodes)]))))) + +(defn learner-nemesis-generator + "start-partition / stop-partition pairs, each start tagged :learners-only so + the checker can pair it with its own stop." + [] + ;; A seq is a generator in Jepsen 0.3.x; gen/seq was removed. + (cycle [(gen/sleep 5) + {:type :info :f :start-partition :value {:scope :learners-only}} + (gen/sleep 10) + {:type :info :f :stop-partition :value {:scope :learners-only}}])) + +;; --------------------------------------------------------------------------- +;; Generator +;; --------------------------------------------------------------------------- + +(defn client-generator + "Register traffic plus one attach/promote cycle for the reserved candidate. + + The previous generator was `gen/nemesis` applied to nil with no :client at + all, so a run emitted NOTHING: none of the documented :write, :read, + :add-learner or :promote-learner operations could appear, and the checker + reported the resulting empty history as valid." + [nodes] + (let [candidate (learner-candidate nodes) + register (gen/mix [(fn [] {:f :write :value (rand-int 1000000)}) + (fn [] {:f :read})])] + (gen/phases + ;; Some traffic first, so the promotion has acknowledged writes to + ;; preserve across it. + (gen/time-limit 5 register) + (gen/once {:f :add-learner :value candidate}) + (gen/time-limit 5 register) + (gen/once {:f :promote-learner :value candidate}) + register))) + +(defn elastickv-learner-test + "Builds a Jepsen test map exercising learner attach and promotion." + ([] (elastickv-learner-test {})) + ([opts] + (let [nodes (or (:nodes opts) default-nodes) + local? (:local opts) + grpc-port (or (:grpc-port opts) 50051) + redis-port (or (:redis-port opts) 6379) + db (if local? + jdb/noop + (ekdb/db {:grpc-port grpc-port + :redis-port redis-port + :encryption (:encryption opts) + ;; Held out of the voter set so :add-learner + ;; has a non-member to attach. + :reserve-learner (learner-candidate nodes)})) + time-limit (or (:time-limit opts) 30) + ports (or (:node->port opts) + (cli/ports->node-map + (repeat (count nodes) redis-port) nodes))] + {:name "elastickv-learner" + :nodes nodes + :db db + :os (if local? os/noop debian/os) + :net (if local? net/noop net/iptables) + :ssh (merge {:username "vagrant" + :private-key-path "/home/vagrant/.ssh/id_rsa" + :strict-host-key-checking false} + (when local? {:dummy true}) + (:ssh opts)) + :remote control/ssh + :client (->LearnerClient ports nil nil) + :nemesis (if local? nemesis/noop (learner-partition-nemesis nodes)) + ;; Jepsen 0.3.x cannot fressian-serialize some final generators. + :final-generator nil + :concurrency (or (:concurrency opts) 10) + :time-limit time-limit + :rate (double (or (:rate opts) 5)) + :checker (learner-safety-checker) + :generator (->> (client-generator nodes) + (gen/nemesis (if local? + (gen/once {:type :info :f :noop}) + (learner-nemesis-generator))) + (gen/stagger (/ (double (or (:rate opts) 5)))) + (gen/time-limit time-limit)) + :grpc-port grpc-port + :ports ports}))) + +(defn -main + "Runnable entry point. Without one, neither invocation form could select + this workload: the namespace had no -main, and the shared dispatcher in + elastickv.jepsen-test neither required it nor listed it, so passing its + name fell through to the Redis test." + [& args] + (cli/run-workload! args cli/common-cli-opts identity elastickv-learner-test)) diff --git a/jepsen/test/elastickv/learner_workload_test.clj b/jepsen/test/elastickv/learner_workload_test.clj new file mode 100644 index 000000000..eb989657d --- /dev/null +++ b/jepsen/test/elastickv/learner_workload_test.clj @@ -0,0 +1,247 @@ +(ns elastickv.learner-workload-test + "Unit tests for the learner workload's checker and wiring. + + The checker tests matter more than usual here: the workload's whole value is + that it can FAIL when the learner primitive misbehaves, and an earlier + revision could not — it had no client, no nemesis and a generator that + emitted nothing, so every run produced an empty history the checker called + valid. Several properties were also measuring the wrong thing. Each test + below names the way it used to pass wrongly." + (:require [clojure.test :refer :all] + [elastickv.db :as ekdb] + [elastickv.jepsen-test :as jt] + [elastickv.learner-workload :as lw] + [jepsen.checker :as checker])) + +(defn- check [history] + (checker/check (lw/learner-safety-checker) {} history {})) + +(def ^:private healthy-prefix + [{:type :invoke :f :write :value 1 :time 10 :process 0} + {:type :ok :f :write :value 1 :time 20 :process 0} + {:type :invoke :f :read :time 30 :process 0} + {:type :ok :f :read :value {:lease? true :value 1} :time 40 :process 0}]) + +(defn- promotion + [m time] + {:type :ok :f :promote-learner :time time + :value (merge {:node "n5" :catch-up-target 100 + :target-source :leader-commit-index :match 100} + m)}) + +(defn- healthy-history [] + (concat healthy-prefix + [{:type :invoke :f :promote-learner :value "n5" :time 50 :process 0} + (promotion {} 60) + {:type :invoke :f :read :time 70 :process 0} + {:type :ok :f :read :value {:lease? true :value 1} :time 80 :process 0}])) + +(deftest healthy-history-is-valid + (is (:valid? (check (healthy-history))))) + +;; --------------------------------------------------------------------------- +;; 1. Promotion never outruns catch-up +;; --------------------------------------------------------------------------- + +(deftest premature-promotion-is-rejected + (let [r (check (concat healthy-prefix [(promotion {:match 90 :catch-up-target 100} 60)]))] + (is (false? (:valid? r))) + (is (= 1 (count (:premature-promotions r)))))) + +(deftest catch-up-is-measured-against-the-sampled-target-not-a-moving-leader + ;; The documented safe workflow: sample target T, wait for the learner to + ;; reach T, promote. The leader keeps committing, so at promotion time the + ;; learner is at T while the leader is well past it. Comparing Match with + ;; the leader's CURRENT commit index rejected this healthy run. + (let [r (check (concat healthy-prefix + [(promotion {:catch-up-target 100 + :match 100 + :leader-commit-index 100000} 60)]))] + (is (:valid? r) + "a learner that reached its sampled target is caught up, whatever the leader did since"))) + +(deftest a-target-read-off-the-learner-is-rejected + ;; The loophole the Match comparison cannot close: the engine's own test is + ;; Match >= min_applied_index, so passing the learner's current Match + ;; satisfies it by construction. The procedure, not the arithmetic, has to + ;; be checked. + (let [r (check (concat healthy-prefix + [(promotion {:target-source :learner-match} 60)]))] + (is (false? (:valid? r))) + (is (= 1 (count (:unsampled-targets r)))))) + +(deftest a-promotion-without-evidence-fails-closed + ;; Previously the numeric guards simply skipped these, so a successful but + ;; unmeasurable promotion left the checker reporting valid having verified + ;; nothing. + (doseq [missing [{:match nil} {:catch-up-target nil}]] + (let [r (check (concat healthy-prefix [(promotion missing 60)]))] + (is (false? (:valid? r)) (str "missing " (keys missing))) + (is (= 1 (count (:unmeasurable-promotions r))))))) + +(deftest promotions-are-counted-once-per-completed-call + ;; An op appears as :invoke plus a completion, so counting both reported two + ;; promotions per call and counted a bare invocation as one. + (let [r (check (concat healthy-prefix + [{:type :invoke :f :promote-learner :value "n5" :time 50} + (promotion {} 60)]))] + (is (= 1 (:promotions r))))) + +;; --------------------------------------------------------------------------- +;; 2. No acknowledged write is lost across a promotion +;; --------------------------------------------------------------------------- + +(deftest a-write-lost-across-a-promotion-is-rejected + (let [r (check [{:type :invoke :f :write :value 7 :time 10} + {:type :ok :f :write :value 7 :time 20} + {:type :invoke :f :promote-learner :value "n5" :time 30} + (promotion {} 40) + {:type :invoke :f :read :time 50} + {:type :ok :f :read :value {:lease? true :value 3} :time 60}])] + (is (false? (:valid? r))) + (is (= 1 (count (:lost-writes r)))))) + +(deftest an-overwritten-value-is-not-a-lost-write + ;; Set subtraction reported 1 as lost in this legal register history merely + ;; because it was overwritten before anyone read it. + (let [r (check [{:type :invoke :f :write :value 1 :time 10} + {:type :ok :f :write :value 1 :time 20} + {:type :invoke :f :write :value 2 :time 30} + {:type :ok :f :write :value 2 :time 40} + {:type :invoke :f :promote-learner :value "n5" :time 50} + (promotion {} 60) + {:type :invoke :f :read :time 70} + {:type :ok :f :read :value {:lease? true :value 2} :time 80}])] + (is (:valid? r) (str "lost-writes=" (:lost-writes r))))) + +(deftest a-concurrent-write-makes-the-expected-value-ambiguous-not-wrong + ;; With another write in flight between the acked write and the read, the + ;; register's value is not pinned, so no conclusion is drawn rather than a + ;; false loss being reported. + (let [r (check [{:type :invoke :f :write :value 1 :time 10} + {:type :ok :f :write :value 1 :time 20} + {:type :invoke :f :promote-learner :value "n5" :time 30} + (promotion {} 40) + {:type :invoke :f :write :value 9 :time 50} + {:type :invoke :f :read :time 60} + {:type :ok :f :read :value {:lease? true :value 9} :time 70}])] + (is (:valid? r) (str "lost-writes=" (:lost-writes r))))) + +;; --------------------------------------------------------------------------- +;; 3. A learner never counts toward the lease +;; --------------------------------------------------------------------------- + +(defn- partition-window [start stop] + [{:type :info :process :nemesis :f :start-partition + :value {:scope :learners-only} :time start} + {:type :info :process :nemesis :f :stop-partition + :value {:scope :learners-only} :time stop}]) + +(deftest a-lease-read-failing-under-learner-isolation-is-rejected + ;; Reads, not writes: quorumAckTracker feeds LastQuorumAck and the + ;; lease-read fast path, not the write-commit quorum, so a write-failure + ;; check stayed green through the exact regression it claimed to detect. + (let [r (check (concat healthy-prefix + [(promotion {} 50)] + (partition-window 100 200) + [{:type :invoke :f :read :time 120} + {:type :fail :f :read :value {:lease? true} :time 130}]))] + (is (false? (:valid? r))) + (is (= 1 (count (:learner-read-failures r)))))) + +(deftest every-learner-partition-window-is-checked-not-just-the-first + ;; Taking the first start and the first later stop ignored every subsequent + ;; isolation window, so a regression in the second one could not fail. + (let [r (check (concat healthy-prefix + [(promotion {} 50)] + (partition-window 100 200) + (partition-window 300 400) + [{:type :invoke :f :read :time 320} + {:type :fail :f :read :value {:lease? true} :time 330}]))] + (is (false? (:valid? r))) + (is (= 1 (count (:learner-read-failures r))) + "a failure in the SECOND window must still be caught"))) + +(deftest a-read-failure-outside-any-window-is-not-attributed-to-the-learner + (let [r (check (concat healthy-prefix + [(promotion {} 50)] + (partition-window 100 200) + [{:type :invoke :f :read :time 500} + {:type :fail :f :read :value {:lease? true} :time 510}]))] + (is (:valid? r) (str "failures=" (:learner-read-failures r))))) + +;; --------------------------------------------------------------------------- +;; The checker must not pass a run that proved nothing +;; --------------------------------------------------------------------------- + +(deftest an-empty-history-is-invalid + ;; THE load-bearing test. Before the workload had a client, a nemesis and a + ;; generator that emitted its documented operations, a real run produced an + ;; empty history — and the checker called it valid, so the gate could never + ;; fail. + (is (false? (:valid? (check []))))) + +(deftest a-history-with-no-promotion-is-invalid + (is (false? (:valid? (check healthy-prefix))) + "a learner test that never promoted has not tested promotion")) + +(deftest a-history-with-no-reads-is-invalid + (is (false? (:valid? (check [{:type :invoke :f :write :value 1 :time 10} + {:type :ok :f :write :value 1 :time 20} + (promotion {} 30)]))) + "the lease property is unobservable without reads")) + +;; --------------------------------------------------------------------------- +;; Wiring: the workload has to be able to run at all +;; --------------------------------------------------------------------------- + +(deftest the-test-map-has-a-client-and-a-nemesis + ;; It had neither, so nothing could drive the cluster. + (let [t (lw/elastickv-learner-test {:nodes ["n1" "n2" "n3" "n4" "n5"]})] + (is (some? (:client t))) + (is (some? (:nemesis t))) + (is (some? (:generator t))))) + +(deftest the-generator-emits-every-documented-operation + ;; The old generator was gen/nemesis applied to nil, so none of :write, + ;; :read, :add-learner or :promote-learner could ever appear. + (let [ops (->> (lw/client-generator ["n1" "n2" "n3" "n4" "n5"]) + (tree-seq coll? seq) + (keep #(when (map? %) (:f %))) + set)] + (is (contains? ops :add-learner)) + (is (contains? ops :promote-learner)))) + +(deftest a-node-is-reserved-outside-the-initial-voter-set + ;; ElastickvDB's setup adds every node after the bootstrap one as a voter, + ;; so without a reservation there is no non-member left to attach and + ;; :add-learner cannot run. + (let [nodes ["n1" "n2" "n3" "n4" "n5"]] + (is (= "n5" (lw/learner-candidate nodes))) + (is (= ["n2" "n3" "n4"] (ekdb/voter-peers nodes "n5")) + "the reserved candidate must not be joined as a voter") + (is (= ["n2" "n3" "n4" "n5"] (ekdb/voter-peers nodes nil)) + "with no reservation the existing behaviour is unchanged") + (is (= "n5" (get-in (lw/elastickv-learner-test {:nodes nodes}) + [:db :opts :reserve-learner]))))) + +(deftest the-workload-is-reachable-from-the-shared-dispatcher + ;; Neither invocation form could select it: the namespace had no -main and + ;; the dispatcher neither required nor listed it, so the name fell through + ;; to the Redis test. + (is (fn? (deref (resolve 'elastickv.learner-workload/-main))) + "the namespace needs a -main to be runnable directly") + (is (= "elastickv-learner" + (:name (jt/elastickv-learner-test {:nodes ["n1" "n2" "n3"]}))))) + +(deftest raft-status-parsing-reads-the-indices-the-promotion-needs + (let [out (str "state: Leader\n" + "leader_id: \"n1\"\n" + "term: 3\n" + "commit_index: 4211\n" + "applied_index: 4207\n" + "pending_conf_change: false\n") + got (ekdb/parse-raft-status out)] + (is (= 4211 (:commit_index got))) + (is (= 4207 (:applied_index got))) + (is (= "n1" (:leader_id got)))))