From ef50df59e6f22738292c7aa18c021c9adca1c53e Mon Sep 17 00:00:00 2001
From: Ryan Senior <ryan@metabase.com>
Date: Tue, 12 Dec 2017 14:15:26 -0600
Subject: [PATCH] Fix async-test transient failures
Depending on the order in which the threads were running, we were
looking for the job in the running-jobs atom BEFORE we put the data in
the atom. If that happens, we might not ever update the job with the
correct status, it's also possible that we'll never remove the job
from the running-jobs atom, just depending on unlucky we are with the
ordering of the threads.
This commit introduces coordination between the code adding the new
data to the running-jobs atom and the work done by the job.
---
src/metabase/feature_extraction/async.clj | 17 ++++++++++++-----
test/metabase/feature_extraction/async_test.clj | 4 ++--
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/metabase/feature_extraction/async.clj b/src/metabase/feature_extraction/async.clj
index 625d5a35e66..8c82265b268 100644
--- a/src/metabase/feature_extraction/async.clj
+++ b/src/metabase/feature_extraction/async.clj
@@ -113,17 +113,24 @@
(or (when-let [job (cached-job ctx)]
(callback job (:result (result job)))
(:id job))
- (let [{:keys [id] :as job} (db/insert! ComputationJob
- :creator_id api/*current-user-id*
- :status :running
- :type :simple-job
- :context ctx)]
+ (let [{:keys [id] :as job} (db/insert! ComputationJob
+ :creator_id api/*current-user-id*
+ :status :running
+ :type :simple-job
+ :context ctx)
+ added-to-running-jobs? (promise)]
(log/info (format "Async job %s started." id))
(swap! running-jobs assoc id (future
(try
+ ;; This argument is getting evaluated BEFORE the swap! associates the id with
+ ;; the future in the atom. If we're unlucky, that means `save-result` will
+ ;; look for the id in that same atom before we've put it there. If that
+ ;; happens we'll never acknowledge that the job completed
+ @added-to-running-jobs?
(save-result job (f) callback)
(catch Throwable e
(save-error job e callback)))))
+ (deliver added-to-running-jobs? true)
id)))
(defmacro with-async
diff --git a/test/metabase/feature_extraction/async_test.clj b/test/metabase/feature_extraction/async_test.clj
index fd05a9962c6..b9e1c6b7003 100644
--- a/test/metabase/feature_extraction/async_test.clj
+++ b/test/metabase/feature_extraction/async_test.clj
@@ -5,7 +5,7 @@
[metabase.test.async :refer :all]))
;; DISABLED due to constant failures 12/6/17. Fix soon!
-#_(expect
+(expect
true
(let [job-id (compute (gensym) (constantly 42))]
(result! job-id)
@@ -27,7 +27,7 @@
:result))
;; DISABLED due to constant failures 12/6/17. Fix soon!
-#_(expect
+(expect
"foo"
(-> (compute (gensym) #(throw (Throwable. "foo")))
result!
--
GitLab