Support starting Metabase without starting Quartz scheduler (#25349)

* Support starting Metabase without initializing the Quartz scheduler

* Fix

src/metabase/task.clj

@@ -15,33 +15,28 @@
             [clojure.string :as str]
             [clojure.tools.logging :as log]
             [clojurewerkz.quartzite.scheduler :as qs]
+            [environ.core :as env]
             [metabase.db :as mdb]
             [metabase.plugins.classloader :as classloader]
             [metabase.util :as u]
             [metabase.util.i18n :refer [trs]]
             [schema.core :as s]
             [toucan.db :as db])
-  (:import [org.quartz CronTrigger JobDetail JobKey Scheduler Trigger TriggerKey]))
+  (:import
+   (org.quartz CronTrigger JobDetail JobKey Scheduler Trigger TriggerKey)))
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
 ;;; |                                               SCHEDULER INSTANCE                                               |
 ;;; +----------------------------------------------------------------------------------------------------------------+
 
-(def ^:dynamic *quartz-scheduler*
-  "Override the global Quartz scheduler by binding this var."
-  nil)
-
-(defonce ^:private quartz-scheduler
+(defonce ^:dynamic ^{:doc "Override the global Quartz scheduler by binding this var."}
+  *quartz-scheduler*
   (atom nil))
 
-;; TODO - maybe we should make this a delay instead!
 (defn- scheduler
-  "Fetch the instance of our Quartz scheduler. Call this function rather than dereffing the atom directly because there
-  are a few places (e.g., in tests) where we swap the instance out."
-  ;; TODO - why can't we just swap the atom out in the tests?
+  "Fetch the instance of our Quartz scheduler."
   ^Scheduler []
-  (or *quartz-scheduler*
-      @quartz-scheduler))
+  @*quartz-scheduler*)
 
 
 ;;; +----------------------------------------------------------------------------------------------------------------+
@@ -151,28 +146,34 @@
   standby mode. Call [[start-scheduler!]] to begin running scheduled tasks."
   []
   (classloader/the-classloader)
-  (when-not @quartz-scheduler
+  (when-not @*quartz-scheduler*
     (set-jdbc-backend-properties!)
     (let [new-scheduler (qs/initialize)]
-      (when (compare-and-set! quartz-scheduler nil new-scheduler)
+      (when (compare-and-set! *quartz-scheduler* nil new-scheduler)
         (find-and-load-task-namespaces!)
         (qs/standby new-scheduler)
         (log/info (trs "Task scheduler initialized into standby mode."))
         (init-tasks!)))))
 
+;;; this is a function mostly to facilitate testing.
+(defn- disable-scheduler? []
+  (some-> (env/env :mb-disable-scheduler) Boolean/parseBoolean))
+
 (defn start-scheduler!
   "Start an initialized scheduler. Tasks do not run before calling this function. It is an error to call this function
-  when [[quartz-scheduler]] has not been set. The function [[init-scheduler!]] will initialize this correctly."
+  when [[*quartz-scheduler*]] has not been set. The function [[init-scheduler!]] will initialize this correctly."
   []
-  (if-let [scheduler @quartz-scheduler]
-    (do (qs/start scheduler)
-        (log/info (trs "Task scheduler started")))
-    (throw (trs "Scheduler not initialized but `start-scheduler!` called. Please call `init-scheduler!` before attempting to start."))))
+  (if (disable-scheduler?)
+    (log/warn (trs "Metabase task scheduler disabled. Scheduled tasks will not be ran."))
+    (if-let [scheduler (scheduler)]
+      (do (qs/start scheduler)
+          (log/info (trs "Task scheduler started")))
+      (throw (trs "Scheduler not initialized but `start-scheduler!` called. Please call `init-scheduler!` before attempting to start.")))))
 
 (defn stop-scheduler!
   "Stop our Quartzite scheduler and shutdown any running executions."
   []
-  (let [[old-scheduler] (reset-vals! quartz-scheduler nil)]
+  (let [[old-scheduler] (reset-vals! *quartz-scheduler* nil)]
     (when old-scheduler
       (qs/shutdown old-scheduler))))
 

test/metabase/task_test.clj

@@ -7,9 +7,11 @@
             [metabase.task :as task]
             [metabase.test :as mt]
             [metabase.test.fixtures :as fixtures]
+            [metabase.test.util :as tu]
             [metabase.util.schema :as su]
             [schema.core :as s])
-  (:import [org.quartz CronTrigger JobDetail]))
+  (:import
+   (org.quartz CronTrigger JobDetail)))
 
 (use-fixtures :once (fixtures/initialize :db))
 
@@ -95,3 +97,18 @@
                                                 s/Keyword            s/Any}]
                                  s/Keyword    s/Any}]}
                    (task/scheduler-info))))))
+
+(deftest start-scheduler-no-op-with-env-var-test
+  (tu/do-with-unstarted-temp-scheduler
+   (^:once fn* []
+    (testing (format "task/start-scheduler! should no-op When MB_DISABLE_SCHEDULER is set")
+      (testing "Sanity check"
+        (is (not (qs/started? (#'task/scheduler)))))
+      (mt/with-temp-env-var-value ["MB_DISABLE_SCHEDULER" "TRUE"]
+        (task/start-scheduler!)
+        (is (not (qs/started? (#'task/scheduler)))))
+      (testing "Should still be able to 'schedule' tasks even if scheduler is unstarted"
+        (is (some? (task/schedule-task! (job) (trigger-1)))))
+      (mt/with-temp-env-var-value ["MB_DISABLE_SCHEDULER" "FALSE"]
+        (task/start-scheduler!)
+        (is (qs/started? (#'task/scheduler))))))))

test/metabase/test.clj

@@ -204,7 +204,6 @@
   with-model-cleanup
   with-non-admin-groups-no-root-collection-for-namespace-perms
   with-non-admin-groups-no-root-collection-perms
-  with-scheduler
   with-temp-env-var-value
   with-temp-file
   with-temp-scheduler

test/metabase/test/util.clj

@@ -36,11 +36,13 @@
             [toucan.db :as db]
             [toucan.models :as models]
             [toucan.util.test :as tt])
-  (:import [java.io File FileInputStream]
-           java.net.ServerSocket
-           java.util.concurrent.TimeoutException
-           java.util.Locale
-           [org.quartz CronTrigger JobDetail JobKey Scheduler Trigger]))
+  (:import
+   (java.io File FileInputStream)
+   (java.net ServerSocket)
+   (java.util Locale)
+   (java.util.concurrent TimeoutException)
+   (org.quartz CronTrigger JobDetail JobKey Scheduler Trigger)
+   (org.quartz.impl StdSchedulerFactory)))
 
 (comment tu.log/keep-me
          test-runner.assert-exprs/keep-me)
@@ -600,29 +602,39 @@
 ;; Various functions for letting us check that things get scheduled properly. Use these to put a temporary scheduler
 ;; in place and then check the tasks that get scheduled
 
-(defn do-with-scheduler [scheduler thunk]
-  (binding [task/*quartz-scheduler* scheduler]
-    (thunk)))
-
-(defmacro with-scheduler
-  "Temporarily bind the Metabase Quartzite scheduler to `scheulder` and run `body`."
-  {:style/indent 1}
-  [scheduler & body]
-  `(do-with-scheduler ~scheduler (fn [] ~@body)))
-
-(defn do-with-temp-scheduler [f]
-  (classloader/the-classloader)
-  (initialize/initialize-if-needed! :db)
-  (let [temp-scheduler        (qs/start (qs/initialize))
-        is-default-scheduler? (identical? temp-scheduler (#'metabase.task/scheduler))]
-    (if is-default-scheduler?
-      (f)
-      (with-scheduler temp-scheduler
+(defn- in-memory-scheduler
+  "An in-memory Quartz Scheduler separate from the usual database-backend one we normally use. Every time you call this
+  it returns the same scheduler! So make sure you shut it down when you're done using it."
+  ^org.quartz.impl.StdScheduler []
+  (.getScheduler
+   (StdSchedulerFactory.
+    (doto (java.util.Properties.)
+      (.setProperty StdSchedulerFactory/PROP_SCHED_INSTANCE_NAME (str `in-memory-scheduler))
+      (.setProperty StdSchedulerFactory/PROP_JOB_STORE_CLASS (.getCanonicalName org.quartz.simpl.RAMJobStore))
+      (.setProperty (str StdSchedulerFactory/PROP_THREAD_POOL_PREFIX ".threadCount") "1")))))
+
+(defn do-with-unstarted-temp-scheduler [thunk]
+  (let [temp-scheduler (in-memory-scheduler)
+        already-bound? (identical? @task/*quartz-scheduler* temp-scheduler)]
+    (if already-bound?
+      (thunk)
+      (binding [task/*quartz-scheduler* (atom temp-scheduler)]
         (try
-          (f)
+          (assert (not (qs/started? temp-scheduler))
+                  "temp in-memory scheduler already started: did you use it elsewhere without shutting it down?")
+          (thunk)
           (finally
             (qs/shutdown temp-scheduler)))))))
 
+(defn do-with-temp-scheduler [thunk]
+  ;; not 100% sure we need to initialize the DB anymore since the temp scheduler is in-memory-only now.
+  (classloader/the-classloader)
+  (initialize/initialize-if-needed! :db)
+  (do-with-unstarted-temp-scheduler
+   (^:once fn* []
+    (qs/start @task/*quartz-scheduler*)
+    (thunk))))
+
 (defmacro with-temp-scheduler
   "Execute `body` with a temporary scheduler in place.