diff --git a/enterprise/backend/src/metabase_enterprise/cache/config.clj b/enterprise/backend/src/metabase_enterprise/cache/config.clj index abf6f7c1c0592bd6370809c8d6e2f8c2be3cf023..9ee88ece5805977540a2ad22dc6567a4e0d01e6a 100644 --- a/enterprise/backend/src/metabase_enterprise/cache/config.clj +++ b/enterprise/backend/src/metabase_enterprise/cache/config.clj @@ -5,7 +5,8 @@ "States of `persisted_info` records which can be refreshed." :feature :cache-granular-controls [] - #{"creating" "persisted" "error"}) + ;; meant to be the same as the oss version except that "off" is deleteable rather than refreshable + #{"refreshing" "creating" "persisted" "error"}) (defenterprise prunable-states "States of `persisted_info` records which can be pruned." diff --git a/enterprise/backend/test/metabase_enterprise/cache/config_test.clj b/enterprise/backend/test/metabase_enterprise/cache/config_test.clj index f3deaaf4eab67d07bf045a2243c1b661c596c9f8..2819056549dd405256fe747f3aa407f12525c65f 100644 --- a/enterprise/backend/test/metabase_enterprise/cache/config_test.clj +++ b/enterprise/backend/test/metabase_enterprise/cache/config_test.clj @@ -47,6 +47,36 @@ `(do-with-persist-models (fn [{:keys [~@bindings]}] ~@body))) +(deftest refreshing-models-are-refreshed + ;; models might get stuck in "refreshing" mode and need to be "kicked". This is actually the case in the OSS + ;; version, but when [[metabase-enterprise.cache.config/refreshable-states]] was extracted, "refreshing" was not + ;; carried over. + ;; + ;; This affects mostly models that were refreshing when an instance was restarted. both OSS and enterprise should + ;; behave this way. Don't know how to exercise both in the same jvm, but will let CI sort it out. + (let [two-hours-ago (t/minus (t/local-date-time) (t/hours 2))] + (mt/with-temp + [Database db {:settings {:persist-models-enabled true}} + Card refreshing {:type :model, :database_id (u/the-id db)} + PersistedInfo prefreshing {:card_id (u/the-id refreshing) + :database_id (u/the-id db) + :state "refreshing" + :state_change_at two-hours-ago}] + (let [card-ids (atom #{}) + test-refresher (reify task.persist-refresh/Refresher + (refresh! [_ _database _definition card] + (swap! card-ids conj (:id card)) + {:state :success}) + (unpersist! [_ _database _persisted-info])) + current-state! (fn [] (t2/select-one-fn :state :model/PersistedInfo (u/the-id prefreshing)))] + ;; ensure ee path is taken + (mt/with-premium-features #{:cache-granular-controls} + (is (= "refreshing" (current-state!))) + (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher) + (testing "Doesn't refresh models that have state='off' or 'deletable' if :cache-granular-controls feature flag is enabled" + (is (= #{(u/the-id refreshing)} @card-ids))) + (is (= "persisted" (current-state!)))))))) + (deftest model-caching-granular-controls-test (mt/with-model-cleanup [TaskHistory] (testing "with :cache-granular-controls enabled, don't refresh any tables in an 'off' or 'deletable' state" diff --git a/src/metabase/models/persisted_info.clj b/src/metabase/models/persisted_info.clj index 821c7b4394503c20094e65f8f0b129c029326546..384e2325eecfe1ddfc5ba196fa254d20d155d270 100644 --- a/src/metabase/models/persisted_info.clj +++ b/src/metabase/models/persisted_info.clj @@ -85,6 +85,7 @@ refreshable with the feature flag disabled." metabase-enterprise.cache.config [] + ;; meant to be the same as the enterprise version except that "off" is not honored and is refreshed #{"refreshing" "creating" "persisted" "error" "off"}) (defenterprise prunable-states diff --git a/test/metabase/task/persist_refresh_test.clj b/test/metabase/task/persist_refresh_test.clj index f7b0a4488d3bc30569d98c15030874f269f1ee2b..afd111b5d4feb17f3953296c8b415e8f61cf3ffd 100644 --- a/test/metabase/task/persist_refresh_test.clj +++ b/test/metabase/task/persist_refresh_test.clj @@ -124,18 +124,20 @@ {:state :success}) (unpersist! [_ _database _persisted-info])) original-update! t2/update!] - (testing "If saving the `persisted` (or `error`) state fails..." - (with-redefs [t2/update! (fn [model id update] - (when (= "persisted" (:state update)) - (throw (ex-info "simulated error" {}))) - (original-update! model id update))] - (is (thrown-with-msg? clojure.lang.ExceptionInfo #"simulated error" - (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher)))) - (testing "the PersistedInfo is left in the `refreshing` state" - (is (= "refreshing" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info))))) - (testing "but a subsequent refresh run will refresh the table" - (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher) - (is (= "persisted" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info)))))))))) + ;; ensure no EE features + (mt/with-premium-features #{} + (testing "If saving the `persisted` (or `error`) state fails..." + (with-redefs [t2/update! (fn [model id update] + (when (= "persisted" (:state update)) + (throw (ex-info "simulated error" {}))) + (original-update! model id update))] + (is (thrown-with-msg? clojure.lang.ExceptionInfo #"simulated error" + (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher)))) + (testing "the PersistedInfo is left in the `refreshing` state" + (is (= "refreshing" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info))))) + (testing "but a subsequent refresh run will refresh the table" + (#'task.persist-refresh/refresh-tables! (u/the-id db) test-refresher) + (is (= "persisted" (t2/select-one-fn :state :model/PersistedInfo :id (u/the-id persisted-info))))))))))) (deftest refresh-tables!'-test (mt/with-model-cleanup [TaskHistory]