diff --git a/src/metabase/task.clj b/src/metabase/task.clj index f8af6d20837b56060819384d38eca10b94ed5581..34c12ff39426cc3f994b933dd899ea1563fda720 100644 --- a/src/metabase/task.clj +++ b/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)))) diff --git a/test/metabase/task_test.clj b/test/metabase/task_test.clj index 5e7e715b799c047ef2bd42dea5439e96ea538e70..f27379a90afa1bedaf3643b28e39b439f5a9d29a 100644 --- a/test/metabase/task_test.clj +++ b/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)))))))) diff --git a/test/metabase/test.clj b/test/metabase/test.clj index b99dbf538065ca42b21d8964ce724fc514eeda59..e210535009f55d2ce943197f6ece90066bdce988 100644 --- a/test/metabase/test.clj +++ b/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 diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index 8da203a75498267ea19088729fc77071ca7b57fd..dad403078e87019a6339b44eebc093de4a15a1fb 100644 --- a/test/metabase/test/util.clj +++ b/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.