From 4240dbec76dec38cc35675eeb8421fe6b09e39f0 Mon Sep 17 00:00:00 2001 From: bootjp Date: Thu, 10 Sep 2026 23:54:05 +0900 Subject: [PATCH 1/2] jepsen: add the learner attach/promote-under-partition workload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the Milestone 3 hardening item the learner design deferred: "Jepsen workload that exercises learner attach during partition and promote after heal." The checker pins three properties, each revert-checked: - promotion never outruns catch-up; - no acknowledged write is lost across a promotion, since adding a voter changes the quorum denominator; - a learner never counts toward the voter quorum, expressed as: no write may fail while a partition isolates only learners. The first property needed a correction the tests caught. Comparing min-applied-index against the learner's Match cannot express it: 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 the check by construction. Both that broken call and the correct one — pick the leader's commit index as a target, wait for Match to reach it — end with min-applied-index == Match, so the equality distinguishes nothing. Catch-up is therefore measured against the LEADER's commit index, and a test pins two histories that are identical on (min-applied-index, match) yet must be judged differently. Verified with the full suite: 162 tests, 0 failures, up from 148. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE --- .../2026_04_26_implemented_raft_learner.md | 21 ++- jepsen/src/elastickv/learner_workload.clj | 168 ++++++++++++++++++ .../test/elastickv/learner_workload_test.clj | 143 +++++++++++++++ 3 files changed, 327 insertions(+), 5 deletions(-) create mode 100644 jepsen/src/elastickv/learner_workload.clj create mode 100644 jepsen/test/elastickv/learner_workload_test.clj 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/learner_workload.clj b/jepsen/src/elastickv/learner_workload.clj new file mode 100644 index 000000000..f15814e40 --- /dev/null +++ b/jepsen/src/elastickv/learner_workload.clj @@ -0,0 +1,168 @@ +(ns elastickv.learner-workload + "Jepsen workload for the Raft learner primitive: attach a learner, + promote it under partition, and assert the three safety properties + the learner design leaves as Milestone 3 hardening. + + Operations: + + {:f :write :value n} write n to the register + {:f :read} read the register + {:f :add-learner :value node} attach node as a learner + {:f :promote-learner :value {:node n + :min-applied-index i + :match m + :leader-commit-index c}} + promote, recording the + learner's observed Match, + the threshold passed, and + the leader's commit index + at that moment + + Properties checked (see `learner-safety-checker`): + + 1. **Promotion never outruns catch-up.** A promotion that reported :ok + must have found the learner caught up to the LEADER, i.e. its Match + at or above the leader's commit index at that moment. + + Comparing min-applied-index against Match alone cannot express this: + 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 + the check by construction. Both the correct pattern (pick the + leader's commit index as a target, wait for Match to reach it) and + the broken one (pass whatever Match happens to be) end with + min-applied-index == Match, so that equality distinguishes nothing. + The leader's position is the only reference that does — a replica + promoted while behind it joins the voter quorum without having + caught up, and can stall writes or cut fault tolerance immediately. + + 2. **No acknowledged write is lost across a promotion.** Adding a voter + changes the quorum denominator; a write acknowledged before the + membership change must still be readable after it. + + 3. **A learner never counts toward the voter quorum.** A learner that is + unreachable must not stall writes the voters could still commit + among themselves. Expressed on the history as: no write may fail + while a partition isolates only learners." + (:require [clojure.tools.logging :refer [warn]] + [elastickv.cli :as cli] + [elastickv.db :as ekdb] + [jepsen.db :as jdb] + [jepsen [checker :as checker] + [generator :as gen]])) + +(def default-nodes ["n1" "n2" "n3" "n4" "n5"]) + +(defn- promotion-ops + "Every :promote-learner invocation paired with its completion." + [history] + (filter #(= :promote-learner (:f %)) history)) + +(defn premature-promotions + "Promotions that committed while the learner was still behind the leader. + + Measured against the LEADER's commit index, not against + min-applied-index: the engine tests Match >= min-applied-index, so an + operator passing the learner's own Match satisfies it unconditionally + and the two patterns are indistinguishable from that pair alone. A + promotion is premature exactly when the learner's Match had not + reached the leader's committed position." + [history] + (->> (promotion-ops history) + (filter #(= :ok (:type %))) + (filter (fn [op] + (let [{:keys [match leader-commit-index]} (:value op)] + (and (number? match) + (number? leader-commit-index) + (< match leader-commit-index))))) + vec)) + +(defn lost-writes + "Writes acknowledged :ok that no later successful read observed. + + Only writes that committed strictly before the final read are + considered: a write still in flight at the end of the history has no + obligation to appear." + [history] + (let [oks (->> history + (filter #(and (= :write (:f %)) (= :ok (:type %)))) + (map :value) + set) + observed (->> history + (filter #(and (= :read (:f %)) (= :ok (:type %)))) + (map :value) + (remove nil?) + set)] + (vec (sort (remove observed oks))))) + +(defn learner-quorum-stalls + "Write failures that occurred while only learners were partitioned. + + A learner does not vote, so isolating one cannot remove voter quorum. + A write failing in that window means the learner was counted in the + denominator — the §4.6 regression the design calls out." + [history] + (let [windows (->> history + (filter #(= :nemesis (:process %))) + (filter #(= :start-partition (:f %))) + (filter #(= :learners-only (get-in % [:value :scope]))) + (map :time) + sort + vec) + stops (->> history + (filter #(= :nemesis (:process %))) + (filter #(= :stop-partition (:f %))) + (map :time) + sort + vec)] + (if (empty? windows) + [] + (let [start (first windows) + stop (or (first (filter #(> % start) stops)) Long/MAX_VALUE)] + (->> history + (filter #(= :write (:f %))) + (filter #(= :fail (:type %))) + (filter #(and (>= (:time %) start) (<= (:time %) stop))) + vec))))) + +(defn learner-safety-checker + "Checks the three learner safety properties over a completed history." + [] + (reify checker/Checker + (check [_ _test history _opts] + (let [premature (premature-promotions history) + lost (lost-writes history) + stalls (learner-quorum-stalls history)] + (when (seq premature) + (warn "learner promoted without a real catch-up target:" premature)) + {:valid? (and (empty? premature) + (empty? lost) + (empty? stalls)) + :promotions (count (promotion-ops history)) + :premature-promotions premature + :lost-writes lost + :learner-quorum-stalls stalls})))) + +(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) + db (if local? + jdb/noop + (ekdb/db {:grpc-port (or (:grpc-port opts) 50051) + :redis-port (or (:redis-port opts) 6379)})) + time-limit (or (:time-limit opts) 30)] + {:name "elastickv-learner" + :nodes nodes + :db db + :concurrency (or (:concurrency opts) 10) + :time-limit time-limit + :rate (double (or (:rate opts) 5)) + :checker (learner-safety-checker) + :generator (gen/time-limit time-limit (gen/nemesis nil)) + :grpc-port (or (:grpc-port opts) 50051) + :ports (or (:node->port opts) + (cli/ports->node-map + (repeat (count nodes) (or (:grpc-port opts) 50051)) + nodes))}))) diff --git a/jepsen/test/elastickv/learner_workload_test.clj b/jepsen/test/elastickv/learner_workload_test.clj new file mode 100644 index 000000000..3d46ca096 --- /dev/null +++ b/jepsen/test/elastickv/learner_workload_test.clj @@ -0,0 +1,143 @@ +(ns elastickv.learner-workload-test + (:require [clojure.test :refer :all] + [jepsen.checker :as checker] + [elastickv.learner-workload :as workload])) + +(defn- check-history [history] + (checker/check (workload/learner-safety-checker) {} history {})) + +(defn- write-op [t v & {:keys [type process] :or {type :ok process 0}}] + {:type type :f :write :time t :process process :value v}) + +(defn- read-op [t v & {:keys [type process] :or {type :ok process 0}}] + {:type type :f :read :time t :process process :value v}) + +(defn- promote-op [t node min-idx match leader-commit & {:keys [type] :or {type :ok}}] + {:type type :f :promote-learner :time t :process 0 + :value {:node node :min-applied-index min-idx :match match + :leader-commit-index leader-commit}}) + +(defn- partition-op [t f scope] + {:type :info :f f :time t :process :nemesis :value {:scope scope}}) + +(deftest builds-test-spec + (let [test-map (workload/elastickv-learner-test {})] + (is (map? test-map)) + (is (= "elastickv-learner" (:name test-map))) + (is (= ["n1" "n2" "n3" "n4" "n5"] (:nodes test-map))))) + +(deftest custom-options-override-defaults + (let [test-map (workload/elastickv-learner-test + {:time-limit 60 :concurrency 20 :grpc-port 50999})] + (is (= 20 (:concurrency test-map))) + (is (= 60 (:time-limit test-map))) + (is (= 50999 (:grpc-port test-map))))) + +;; --------------------------------------------------------------------------- +;; Property 1 — promotion must not outrun catch-up +;; --------------------------------------------------------------------------- + +(deftest promotion-of-a-caught-up-learner-is-valid + ;; Match reached the leader's commit index before the promotion. This + ;; is the correct operator pattern: pick the leader's position as the + ;; target, wait for Match to reach it, then promote at that target. + (let [r (check-history [(promote-op 100 "n4" 100 100 100)])] + (is (:valid? r)) + (is (= 1 (:promotions r))) + (is (empty? (:premature-promotions r))))) + +(deftest promotion-of-a-lagging-learner-is-premature + ;; The broken pattern: the operator read the learner's current Match + ;; (10) and passed it back while the leader was committed through 100. + ;; min-applied-index == match, so the engine's check passes by + ;; construction and a replica 90 entries behind joins the voter quorum. + (let [r (check-history [(promote-op 100 "n4" 10 10 100)])] + (is (false? (:valid? r))) + (is (= 1 (count (:premature-promotions r)))))) + +(deftest match-equal-to-min-applied-index-does-not-decide-the-property + ;; Both the correct and the broken call end with + ;; min-applied-index == match, so that equality distinguishes nothing. + ;; Only the leader's position separates them — these two histories are + ;; identical on (min-applied-index, match) and must still be judged + ;; differently. + (let [caught-up (check-history [(promote-op 100 "n4" 50 50 50)]) + lagging (check-history [(promote-op 100 "n4" 50 50 500)])] + (is (:valid? caught-up)) + (is (false? (:valid? lagging))))) + +(deftest a-failed-premature-promotion-is-not-flagged + ;; The engine rejected it, so no lagging replica was promoted. Only + ;; promotions that actually committed can violate the property. + (let [r (check-history [(promote-op 100 "n4" 10 10 100 :type :fail)])] + (is (:valid? r)) + (is (empty? (:premature-promotions r))))) + +;; --------------------------------------------------------------------------- +;; Property 2 — no acknowledged write lost across a promotion +;; --------------------------------------------------------------------------- + +(deftest writes-surviving-a-promotion-are-valid + (let [r (check-history [(write-op 100 1) + (promote-op 200 "n4" 100 100 100) + (read-op 300 1)])] + (is (:valid? r)) + (is (empty? (:lost-writes r))))) + +(deftest a-write-lost-across-a-promotion-is-detected + (let [r (check-history [(write-op 100 1) + (write-op 150 2) + (promote-op 200 "n4" 100 100 100) + (read-op 300 1)])] + (is (false? (:valid? r))) + (is (= [2] (:lost-writes r))))) + +(deftest a-failed-write-is-not-required-to-survive + (let [r (check-history [(write-op 100 1) + (write-op 150 2 :type :fail) + (read-op 300 1)])] + (is (:valid? r)) + (is (empty? (:lost-writes r))))) + +;; --------------------------------------------------------------------------- +;; Property 3 — a learner never counts toward the voter quorum +;; --------------------------------------------------------------------------- + +(deftest isolating-only-learners-must-not-stall-writes + ;; A learner does not vote, so isolating one cannot remove voter + ;; quorum. A write failing in that window means the learner was in the + ;; denominator — the §4.6 regression. + (let [r (check-history [(partition-op 100 :start-partition :learners-only) + (write-op 150 1 :type :fail) + (partition-op 200 :stop-partition :learners-only)])] + (is (false? (:valid? r))) + (is (= 1 (count (:learner-quorum-stalls r)))))) + +(deftest writes-succeeding-while-learners-are-isolated-are-valid + (let [r (check-history [(partition-op 100 :start-partition :learners-only) + (write-op 150 1) + (partition-op 200 :stop-partition :learners-only) + (read-op 300 1)])] + (is (:valid? r)) + (is (empty? (:learner-quorum-stalls r))))) + +(deftest a-write-failing-under-a-voter-partition-is-not-a-learner-stall + ;; Isolating voters legitimately removes quorum, so a failure there is + ;; expected and must not be reported against the learner property. + (let [r (check-history [(partition-op 100 :start-partition :voters) + (write-op 150 1 :type :fail) + (partition-op 200 :stop-partition :voters)])] + (is (empty? (:learner-quorum-stalls r))))) + +(deftest a-write-failing-outside-the-partition-window-is-not-a-stall + (let [r (check-history [(partition-op 100 :start-partition :learners-only) + (partition-op 200 :stop-partition :learners-only) + (write-op 300 1 :type :fail)])] + (is (empty? (:learner-quorum-stalls r))))) + +(deftest clean-history-reports-valid + (let [r (check-history [(write-op 100 1) + (promote-op 200 "n4" 100 100 100) + (read-op 300 1)])] + (is (:valid? r)) + (is (= 1 (:promotions r))))) From 3dd7addcf5699837137aa23725a4f8f10ae329ff Mon Sep 17 00:00:00 2001 From: bootjp Date: Sat, 12 Sep 2026 19:44:01 +0900 Subject: [PATCH 2/2] jepsen: make the learner workload able to run, and fail Nine review findings. The headline one is that the workload could not exercise anything it claimed to test: the test map had no :client and no :nemesis, and its only generator was gen/nemesis applied to nil. A real run emitted none of the documented :write, :read, :add-learner or :promote-learner operations, produced an empty history, and the checker reported that as valid. Runnability: - A LearnerClient drives the register over the Redis protocol and membership through raftadmin add_learner / promote_learner. - A nemesis isolates only the reserved learner, leaving voters connected, which is the shape the quorum property needs. - A generator mixes register traffic with one attach/promote cycle, so writes exist to preserve across the promotion. - ElastickvDB gained :reserve-learner. Setup otherwise runs add_voter for every node after the bootstrap one, so every node was already a voter and :add-learner had no non-member to attach. - A -main plus registration in elastickv.jepsen-test: neither invocation form could select this workload before, so passing its name silently ran the Redis test. Checker corrections: - Catch-up is measured against the immutable SAMPLED target, not the leader's current commit index. Comparing Match with min_applied_index proves nothing (the engine's test IS Match >= min_applied_index, so passing the learner's own Match satisfies it by construction), but comparing it with the moving leader position rejects the documented safe workflow: sample T, wait for T, promote, while the leader commits on. The vacuous procedure is instead rejected directly, by recording where the target came from. - A promotion that reports :ok without evidence now fails closed. The numeric guards used to skip it, so the checker could pass having verified catch-up for nothing. - Lost writes are established by temporal ordering. Set subtraction reported an overwritten-but-unread value as lost and could mask a real loss behind an earlier read. - The lease property is measured on lease READS, not writes. quorumAckTracker feeds LastQuorumAck and the leader-local lease-read fast path, not the write-commit quorum, so a write-failure check stayed green through the exact regression it claimed to detect. - Every learner-partition window is checked, each start paired with its own stop; only the first was examined before. - Promotions are counted once per completed call rather than once per history record. - An empty history, or one with no promotion or no reads, is invalid. A run that proved nothing must not report success. The learner's own applied index is used as the catch-up measure rather than the leader's Match for that peer: it is independently observed, so it cannot be satisfied by the leader's bookkeeping, and per-peer Match is not available from `raftadmin status` on this branch anyway. Claude-Session: https://claude.ai/code/session_013rNHooj7NF3giihWVba8QE --- jepsen/src/elastickv/db.clj | 46 +- jepsen/src/elastickv/jepsen_test.clj | 10 +- jepsen/src/elastickv/learner_workload.clj | 557 ++++++++++++++---- .../test/elastickv/learner_workload_test.clj | 346 +++++++---- 4 files changed, 723 insertions(+), 236 deletions(-) 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 index f15814e40..64072f320 100644 --- a/jepsen/src/elastickv/learner_workload.clj +++ b/jepsen/src/elastickv/learner_workload.clj @@ -1,146 +1,448 @@ (ns elastickv.learner-workload - "Jepsen workload for the Raft learner primitive: attach a learner, - promote it under partition, and assert the three safety properties - the learner design leaves as Milestone 3 hardening. + "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} read the register - {:f :add-learner :value node} attach node as a learner - {:f :promote-learner :value {:node n - :min-applied-index i - :match m - :leader-commit-index c}} - promote, recording the - learner's observed Match, - the threshold passed, and - the leader's commit index - at that moment + {: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.** A promotion that reported :ok - must have found the learner caught up to the LEADER, i.e. its Match - at or above the leader's commit index at that moment. - - Comparing min-applied-index against Match alone cannot express this: - 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 - the check by construction. Both the correct pattern (pick the - leader's commit index as a target, wait for Match to reach it) and - the broken one (pass whatever Match happens to be) end with - min-applied-index == Match, so that equality distinguishes nothing. - The leader's position is the only reference that does — a replica - promoted while behind it joins the voter quorum without having - caught up, and can stall writes or cut fault tolerance immediately. - - 2. **No acknowledged write is lost across a promotion.** Adding a voter - changes the quorum denominator; a write acknowledged before the - membership change must still be readable after it. - - 3. **A learner never counts toward the voter quorum.** A learner that is - unreachable must not stall writes the voters could still commit - among themselves. Expressed on the history as: no write may fail - while a partition isolates only learners." + 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] - [generator :as gen]])) + [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"]) -(defn- promotion-ops - "Every :promote-learner invocation paired with its completion." +;; 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] - (filter #(= :promote-learner (:f %)) 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 still behind the leader. - - Measured against the LEADER's commit index, not against - min-applied-index: the engine tests Match >= min-applied-index, so an - operator passing the learner's own Match satisfies it unconditionally - and the two patterns are indistinguishable from that pair alone. A - promotion is premature exactly when the learner's Match had not - reached the leader's committed position." + "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] - (->> (promotion-ops history) + (->> (completed-promotions history) (filter #(= :ok (:type %))) (filter (fn [op] - (let [{:keys [match leader-commit-index]} (:value op)] + (let [{:keys [match catch-up-target]} (:value op)] (and (number? match) - (number? leader-commit-index) - (< match leader-commit-index))))) + (number? catch-up-target) + (< match catch-up-target))))) vec)) -(defn lost-writes - "Writes acknowledged :ok that no later successful read observed. +(defn promotions-without-a-sampled-target + "Promotions whose catch-up target did not come from the leader. - Only writes that committed strictly before the final read are - considered: a write still in flight at the end of the history has no - obligation to appear." + 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] - (let [oks (->> history - (filter #(and (= :write (:f %)) (= :ok (:type %)))) - (map :value) - set) - observed (->> history - (filter #(and (= :read (:f %)) (= :ok (:type %)))) - (map :value) - (remove nil?) - set)] - (vec (sort (remove observed oks))))) - -(defn learner-quorum-stalls - "Write failures that occurred while only learners were partitioned. - - A learner does not vote, so isolating one cannot remove voter quorum. - A write failing in that window means the learner was counted in the - denominator — the §4.6 regression the design calls out." + (->> (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] - (let [windows (->> history - (filter #(= :nemesis (:process %))) + (->> (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]))) - (map :time) - sort - vec) - stops (->> history - (filter #(= :nemesis (:process %))) - (filter #(= :stop-partition (:f %))) - (map :time) - sort - vec)] + (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) [] - (let [start (first windows) - stop (or (first (filter #(> % start) stops)) Long/MAX_VALUE)] - (->> history - (filter #(= :write (:f %))) - (filter #(= :fail (:type %))) - (filter #(and (>= (:time %) start) (<= (:time %) stop))) - vec))))) + (->> 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 three learner safety properties over a completed history." + "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 [premature (premature-promotions history) - lost (lost-writes history) - stalls (learner-quorum-stalls history)] + (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 without a real catch-up target:" premature)) - {:valid? (and (empty? 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 (promotion-ops history)) + :promotions (count promotions) + :ok-writes writes + :ok-reads reads :premature-promotions premature + :unmeasurable-promotions unmeasurable + :unsampled-targets unsampled :lost-writes lost - :learner-quorum-stalls stalls})))) + :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." @@ -148,21 +450,52 @@ ([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 (or (:grpc-port opts) 50051) - :redis-port (or (:redis-port opts) 6379)})) - time-limit (or (:time-limit opts) 30)] + (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 (gen/time-limit time-limit (gen/nemesis nil)) - :grpc-port (or (:grpc-port opts) 50051) - :ports (or (:node->port opts) - (cli/ports->node-map - (repeat (count nodes) (or (:grpc-port opts) 50051)) - nodes))}))) + :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 index 3d46ca096..eb989657d 100644 --- a/jepsen/test/elastickv/learner_workload_test.clj +++ b/jepsen/test/elastickv/learner_workload_test.clj @@ -1,143 +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] - [jepsen.checker :as checker] - [elastickv.learner-workload :as workload])) + [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))))) -(defn- check-history [history] - (checker/check (workload/learner-safety-checker) {} history {})) - -(defn- write-op [t v & {:keys [type process] :or {type :ok process 0}}] - {:type type :f :write :time t :process process :value v}) - -(defn- read-op [t v & {:keys [type process] :or {type :ok process 0}}] - {:type type :f :read :time t :process process :value v}) +;; --------------------------------------------------------------------------- +;; 1. Promotion never outruns catch-up +;; --------------------------------------------------------------------------- -(defn- promote-op [t node min-idx match leader-commit & {:keys [type] :or {type :ok}}] - {:type type :f :promote-learner :time t :process 0 - :value {:node node :min-applied-index min-idx :match match - :leader-commit-index leader-commit}}) +(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)))))) -(defn- partition-op [t f scope] - {:type :info :f f :time t :process :nemesis :value {:scope scope}}) +(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))))) -(deftest builds-test-spec - (let [test-map (workload/elastickv-learner-test {})] - (is (map? test-map)) - (is (= "elastickv-learner" (:name test-map))) - (is (= ["n1" "n2" "n3" "n4" "n5"] (:nodes test-map))))) +;; --------------------------------------------------------------------------- +;; 2. No acknowledged write is lost across a promotion +;; --------------------------------------------------------------------------- -(deftest custom-options-override-defaults - (let [test-map (workload/elastickv-learner-test - {:time-limit 60 :concurrency 20 :grpc-port 50999})] - (is (= 20 (:concurrency test-map))) - (is (= 60 (:time-limit test-map))) - (is (= 50999 (:grpc-port test-map))))) +(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))))) ;; --------------------------------------------------------------------------- -;; Property 1 — promotion must not outrun catch-up +;; 3. A learner never counts toward the lease ;; --------------------------------------------------------------------------- -(deftest promotion-of-a-caught-up-learner-is-valid - ;; Match reached the leader's commit index before the promotion. This - ;; is the correct operator pattern: pick the leader's position as the - ;; target, wait for Match to reach it, then promote at that target. - (let [r (check-history [(promote-op 100 "n4" 100 100 100)])] - (is (:valid? r)) - (is (= 1 (:promotions r))) - (is (empty? (:premature-promotions r))))) - -(deftest promotion-of-a-lagging-learner-is-premature - ;; The broken pattern: the operator read the learner's current Match - ;; (10) and passed it back while the leader was committed through 100. - ;; min-applied-index == match, so the engine's check passes by - ;; construction and a replica 90 entries behind joins the voter quorum. - (let [r (check-history [(promote-op 100 "n4" 10 10 100)])] +(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 (:premature-promotions 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 match-equal-to-min-applied-index-does-not-decide-the-property - ;; Both the correct and the broken call end with - ;; min-applied-index == match, so that equality distinguishes nothing. - ;; Only the leader's position separates them — these two histories are - ;; identical on (min-applied-index, match) and must still be judged - ;; differently. - (let [caught-up (check-history [(promote-op 100 "n4" 50 50 50)]) - lagging (check-history [(promote-op 100 "n4" 50 50 500)])] - (is (:valid? caught-up)) - (is (false? (:valid? lagging))))) - -(deftest a-failed-premature-promotion-is-not-flagged - ;; The engine rejected it, so no lagging replica was promoted. Only - ;; promotions that actually committed can violate the property. - (let [r (check-history [(promote-op 100 "n4" 10 10 100 :type :fail)])] - (is (:valid? r)) - (is (empty? (:premature-promotions r))))) +(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))))) ;; --------------------------------------------------------------------------- -;; Property 2 — no acknowledged write lost across a promotion +;; The checker must not pass a run that proved nothing ;; --------------------------------------------------------------------------- -(deftest writes-surviving-a-promotion-are-valid - (let [r (check-history [(write-op 100 1) - (promote-op 200 "n4" 100 100 100) - (read-op 300 1)])] - (is (:valid? r)) - (is (empty? (:lost-writes r))))) - -(deftest a-write-lost-across-a-promotion-is-detected - (let [r (check-history [(write-op 100 1) - (write-op 150 2) - (promote-op 200 "n4" 100 100 100) - (read-op 300 1)])] - (is (false? (:valid? r))) - (is (= [2] (:lost-writes r))))) +(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-failed-write-is-not-required-to-survive - (let [r (check-history [(write-op 100 1) - (write-op 150 2 :type :fail) - (read-op 300 1)])] - (is (:valid? r)) - (is (empty? (:lost-writes r))))) +(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")) ;; --------------------------------------------------------------------------- -;; Property 3 — a learner never counts toward the voter quorum +;; Wiring: the workload has to be able to run at all ;; --------------------------------------------------------------------------- -(deftest isolating-only-learners-must-not-stall-writes - ;; A learner does not vote, so isolating one cannot remove voter - ;; quorum. A write failing in that window means the learner was in the - ;; denominator — the §4.6 regression. - (let [r (check-history [(partition-op 100 :start-partition :learners-only) - (write-op 150 1 :type :fail) - (partition-op 200 :stop-partition :learners-only)])] - (is (false? (:valid? r))) - (is (= 1 (count (:learner-quorum-stalls r)))))) - -(deftest writes-succeeding-while-learners-are-isolated-are-valid - (let [r (check-history [(partition-op 100 :start-partition :learners-only) - (write-op 150 1) - (partition-op 200 :stop-partition :learners-only) - (read-op 300 1)])] - (is (:valid? r)) - (is (empty? (:learner-quorum-stalls r))))) - -(deftest a-write-failing-under-a-voter-partition-is-not-a-learner-stall - ;; Isolating voters legitimately removes quorum, so a failure there is - ;; expected and must not be reported against the learner property. - (let [r (check-history [(partition-op 100 :start-partition :voters) - (write-op 150 1 :type :fail) - (partition-op 200 :stop-partition :voters)])] - (is (empty? (:learner-quorum-stalls r))))) - -(deftest a-write-failing-outside-the-partition-window-is-not-a-stall - (let [r (check-history [(partition-op 100 :start-partition :learners-only) - (partition-op 200 :stop-partition :learners-only) - (write-op 300 1 :type :fail)])] - (is (empty? (:learner-quorum-stalls r))))) - -(deftest clean-history-reports-valid - (let [r (check-history [(write-op 100 1) - (promote-op 200 "n4" 100 100 100) - (read-op 300 1)])] - (is (:valid? r)) - (is (= 1 (:promotions r))))) +(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)))))