Skip to content
Snippets Groups Projects
Unverified Commit c7e2a007 authored by Alexander Solovyov's avatar Alexander Solovyov Committed by GitHub
Browse files

[cache] surface last run time and make sure "nocache" won't use cache (#44679)

parent 87914fd8
No related branches found
No related tags found
No related merge requests found
......@@ -154,37 +154,37 @@
:model "database")))
(testing "making a query will cache it"
(is (=? {:cached false :data some?}
(is (=? {:cached nil :data some?}
(run-query! card1-id)))
(is (=? {:cached true :data some?}
(is (=? {:cached some? :data some?}
(run-query! card1-id)))
(is (=? {:cached true :data some?}
(is (=? {:cached some? :data some?}
(run-query! card2-id))))
(testing "invalidation drops cache only for affected card"
(is (=? {:count 1}
(invalidate! 200 :question card2-id :include :overrides)))
(is (=? {:cached true :data some?}
(is (=? {:cached some? :data some?}
(run-query! card1-id)))
(is (=? {:cached false :data some?}
(is (=? {:cached nil :data some?}
(run-query! card2-id))))
(testing "but invalidating a whole config drops cache for any affected card"
(doseq [card-id [card1-id card2-id]]
(is (=? {:count 1}
(invalidate! 200 :database (mt/id))))
(is (=? {:cached false :data some?}
(is (=? {:cached nil :data some?}
(run-query! card-id {:ignore_cache true})))))
(testing "when invalidating database config directly, dashboard-related queries are still cached"
(is (=? {:count 1}
(invalidate! 200 :database (mt/id))))
(is (=? {:cached true :data some?}
(is (=? {:cached some? :data some?}
(run-query! card1-id {:dashboard_id (:id dash)}))))
(testing "but with overrides - will go through every card and mark cache invalidated"
;; not a concrete number here since (mt/id) can have a bit more than 2 cards we've currently defined
(is (=? {:count pos-int?}
(invalidate! 200 :include :overrides :database (mt/id))))
(is (=? {:cached false :data some?}
(is (=? {:cached nil :data some?}
(run-query! card1-id {:dashboard_id (:id dash)}))))))))))
......@@ -11,7 +11,7 @@ export default function QuestionLastUpdated({ result, ...props }) {
return result ? (
<SectionRoot {...props}>
<Icon name="clock" className={CS.mr1} />
{t`Updated ${moment(result.updated_at).fromNow()}`}
{t`Updated ${moment(result.cached).fromNow()}`}
</SectionRoot>
) : null;
}
......
......@@ -220,7 +220,8 @@
(save-results-xform start-time-ns metadata query-hash cache-strategy (rff metadata)))))))))
(defn- is-cacheable? {:arglists '([query])} [{:keys [cache-strategy]}]
(some? cache-strategy))
(and (some? cache-strategy)
(not= (:type cache-strategy) :nocache)))
(defn maybe-return-cached-results
"Middleware for caching results of a query if applicable.
......
......@@ -94,7 +94,7 @@
add-running-time
(dissoc :error :hash :executor_id :action_id :is_sandboxed :card_id :dashboard_id :pulse_id :result_rows :native))
(dissoc result :cache/details)
{:cached (boolean (:cached cache))
{:cached (when (:cached cache) (:updated_at cache))
:status :completed
:average_execution_time (when (:cached cache)
(query/average-execution-time-ms query-hash))}))
......
......@@ -28,7 +28,8 @@
[metabase.util :as u]
[metabase.util.log :as log]
[pretty.core :as pretty]
[toucan2.core :as t2]))
[toucan2.core :as t2])
(:import [java.time ZonedDateTime]))
(set! *warn-on-reflection* true)
......@@ -159,10 +160,11 @@
(boolean (#'cache/is-cacheable? (merge {:cache-strategy (ttl-strategy), :query :abc} query-kvs))))
(deftest is-cacheable-test
(testing "something is-cacheable? if it includes `:cache-strategy`"
(testing "something is-cacheable? if it includes `:cache-strategy` and it is not `:nocache`"
(with-mock-cache []
(doseq [cache-strategy [(ttl-strategy) nil]
:let [expected (boolean cache-strategy)]]
(doseq [[cache-strategy expected] {(ttl-strategy) true
{:type :nocache} false
nil false}]
(testing (format "cache strategy = %s" (pr-str cache-strategy))
(is (= expected
(boolean (#'cache/is-cacheable? {:cache-strategy cache-strategy})))))))))
......@@ -349,9 +351,10 @@
(is (= 1 @save-execution-metadata-count))
(is (= 1 @update-avg-execution-count))
(let [avg-execution-time (query/average-execution-time-ms q-hash)]
(is (number? avg-execution-time))
(is (pos? avg-execution-time))
;; rerun query getting cached results
(is (:cached (qp/process-query (qp/userland-query query))))
(is (instance? ZonedDateTime
(:cached (qp/process-query (qp/userland-query query)))))
(mt/wait-for-result save-chan)
(is (= 2 @save-execution-metadata-count)
"Saving execution times of a cache lookup")
......
......@@ -83,7 +83,7 @@
:average_execution_time nil
:context nil
:running_time int?
:cached false}
:cached nil}
(process-userland-query query))
"Result should have query execution info")
(is (=? {:hash "58af781ea2ba252ce3131462bdc7c54bc57538ed965d55beec62928ce8b32635"
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment