Improve retry for scalardb test (#131)

* Add upper limit to `exponential-backoff` * Increase max retry * Fix unit test * Use the constants in core_test.cli
scalar-labs · Oct 29, 2024 · 8b2757d · 8b2757d
1 parent 9ff6675
commit 8b2757d
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 11 deletions.
diff --git a/scalardb/src/scalardb/core.clj b/scalardb/src/scalardb/core.clj
@@ -12,18 +12,23 @@
                                   StorageFactory)
            (com.scalar.db.transaction.consensuscommit Coordinator)))
 
-(def ^:const RETRIES 8)
+(def ^:const RETRIES 20)
 (def ^:const RETRIES_FOR_RECONNECTION 3)
 (def ^:private ^:const NUM_FAILURES_FOR_RECONNECTION 1000)
+(def ^:private ^:const MAX_WAIT_MILLIS 32000)
 (def ^:const INITIAL_TABLE_ID 1)
 (def ^:const DEFAULT_TABLE_COUNT 3)
 
 (def ^:const KEYSPACE "jepsen")
 (def ^:const VERSION "tx_version")
 
+(defn compute-exponential-backoff
+  [r]
+  (min MAX_WAIT_MILLIS (reduce * 1000 (repeat r 2))))
+
 (defn exponential-backoff
   [r]
-  (Thread/sleep (reduce * 1000 (repeat r 2))))
+  (Thread/sleep (compute-exponential-backoff r)))
 
 (defn- get-cassandra-schema
   "Only the current test schemata are covered

diff --git a/scalardb/test/scalardb/core_test.clj b/scalardb/test/scalardb/core_test.clj
@@ -103,7 +103,7 @@
                 scalar/create-service-instance (spy/stub nil)]
     (let [test {:db mock-db :storage (atom nil)}]
       (scalar/prepare-storage-service! test)
-      (is (spy/called-n-times? scalar/exponential-backoff 8))
+      (is (spy/called-n-times? scalar/exponential-backoff scalardb.core/RETRIES))
       (is (nil? @(:storage test))))))
 
 (deftest check-connection-test
@@ -150,5 +150,12 @@
     (let [test {:storage (atom mock-storage)}]
       (is (thrown? clojure.lang.ExceptionInfo
                    (scalar/check-transaction-states test #{"tx"})))
-      (is (spy/called-n-times? scalar/exponential-backoff 8))
-      (is (spy/called-n-times? scalar/prepare-storage-service! 3)))))
+      (is (spy/called-n-times? scalar/exponential-backoff scalardb.core/RETRIES))
+      (is (spy/called-n-times? scalar/prepare-storage-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1))))))
+
+(deftest compute-exponential-backoff-test
+  (is (= 2000 (scalar/compute-exponential-backoff 1)))
+  (is (= 4000 (scalar/compute-exponential-backoff 2)))
+  (is (= 32000 (scalar/compute-exponential-backoff 5)))
+  (is (= 32000 (scalar/compute-exponential-backoff 10))))
diff --git a/scalardb/test/scalardb/transfer_2pc_test.clj b/scalardb/test/scalardb/transfer_2pc_test.clj
@@ -235,8 +235,10 @@
                                    (#'transfer/get-all {:client client}
                                                        nil))))
       (is (spy/called-n-times? scalar/exponential-backoff scalar/RETRIES))
-      (is (spy/called-n-times? scalar/prepare-transaction-service! scalar/RETRIES_FOR_RECONNECTION))
-      (is (spy/called-n-times? scalar/prepare-storage-service! scalar/RETRIES_FOR_RECONNECTION)))))
+      (is (spy/called-n-times? scalar/prepare-transaction-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1)))
+      (is (spy/called-n-times? scalar/prepare-storage-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1))))))
 
 (deftest transfer-client-check-tx-test
   (with-redefs [scalar/check-transaction-states (spy/stub 1)]

diff --git a/scalardb/test/scalardb/transfer_append_2pc_test.clj b/scalardb/test/scalardb/transfer_append_2pc_test.clj
@@ -242,7 +242,8 @@
                    (client/invoke! client {:db mock-db}
                                    (transfer/get-all {:client client} nil))))
       (is (spy/called-n-times? scalar/exponential-backoff scalar/RETRIES))
-      (is (spy/called-n-times? scalar/prepare-transaction-service! scalar/RETRIES_FOR_RECONNECTION)))))
+      (is (spy/called-n-times? scalar/prepare-transaction-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1))))))
 
 (deftest transfer-client-check-tx-test
   (with-redefs [scalar/check-transaction-states (spy/stub 1)]

diff --git a/scalardb/test/scalardb/transfer_append_test.clj b/scalardb/test/scalardb/transfer_append_test.clj
@@ -212,7 +212,8 @@
                    (client/invoke! client {:db mock-db}
                                    (transfer/get-all {:client client} nil))))
       (is (spy/called-n-times? scalar/exponential-backoff scalar/RETRIES))
-      (is (spy/called-n-times? scalar/prepare-transaction-service! scalar/RETRIES_FOR_RECONNECTION)))))
+      (is (spy/called-n-times? scalar/prepare-transaction-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1))))))
 
 (deftest transfer-client-check-tx-test
   (with-redefs [scalar/check-transaction-states (spy/stub 1)]

diff --git a/scalardb/test/scalardb/transfer_test.clj b/scalardb/test/scalardb/transfer_test.clj
@@ -205,8 +205,10 @@
                                    (#'transfer/get-all {:client client}
                                                        nil))))
       (is (spy/called-n-times? scalar/exponential-backoff scalar/RETRIES))
-      (is (spy/called-n-times? scalar/prepare-transaction-service! scalar/RETRIES_FOR_RECONNECTION))
-      (is (spy/called-n-times? scalar/prepare-storage-service! scalar/RETRIES_FOR_RECONNECTION)))))
+      (is (spy/called-n-times? scalar/prepare-transaction-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1)))
+      (is (spy/called-n-times? scalar/prepare-storage-service!
+                               (+ (quot scalar/RETRIES scalar/RETRIES_FOR_RECONNECTION) 1))))))
 
 (deftest transfer-client-check-tx-test
   (with-redefs [scalar/check-transaction-states (spy/stub 1)]