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

@@ -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

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!