diff --git a/src/metabase/feature_extraction/async.clj b/src/metabase/feature_extraction/async.clj index 625d5a35e66c5c337dd5cb67acb7bb2dee67ea36..8c82265b26875db960996c102209c17629034497 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 fd05a9962c6f5b66113e5acf9178a8113289d59a..b9e1c6b7003ef3cd52a567b846e90d37a4016d10 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!