Skip to content
Snippets Groups Projects
Unverified Commit 5209d6dc authored by Cam Saul's avatar Cam Saul
Browse files

Horizontal Scalability: invalidate Settings cache when appropriate :scream_cat:

parent 9ba5521b
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/models/setting.clj
+ 153
− 43
View file @ 5209d6dc
......@@ -30,13 +30,18 @@
(setting/all)"
(:refer-clojure :exclude [get])
(:require [cheshire.core :as json]
[clojure.string :as str]
[clojure
[core :as core]
[string :as str]]
[clojure.core.memoize :as memoize]
[clojure.tools.logging :as log]
[environ.core :as env]
[honeysql.core :as hsql]
[metabase
[events :as events]
[util :as u]]
[puppetlabs.i18n.core :refer [tru]]
[metabase.util.honeysql-extensions :as hx]
[puppetlabs.i18n.core :refer [trs tru]]
[schema.core :as s]
[toucan
[db :as db]
......@@ -74,7 +79,8 @@
setting-or-name
(let [k (keyword setting-or-name)]
(or (@registered-settings k)
(throw (Exception. (str (tru "Setting {0} does not exist.\nFound: {1}" k (sort (keys @registered-settings))))))))))
(throw (Exception.
(str (tru "Setting {0} does not exist.\nFound: {1}" k (sort (keys @registered-settings))))))))))
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -84,12 +90,109 @@
;; Cache is a 1:1 mapping of what's in the DB
;; Cached lookup time is ~60µs, compared to ~1800µs for DB lookup
(defonce ^:private cache
(def ^:private cache
"Settings cache. Map of Setting key (string) -> Setting value (string)."
(atom nil))
(defn- restore-cache-if-needed! []
(when-not @cache
(reset! cache (db/select-field->field :key :value Setting))))
;; CACHE SYNCHRONIZATION
;;
;; When running multiple Metabase instances (horizontal scaling), it is of course possible for one instance to update
;; a Setting, and, since Settings are cached (to avoid tons of DB calls), for the other instances to then have an
;; out-of-date cache. Thus we need a way for instances to know when their caches are out of date, so they can update
;; them accordingly. Here is our solution:
;;
;; We will record the last time *any* Setting was updated in a special Setting called `settings-last-updated`.
;;
;; Since `settings-last-updated` itself is a Setting, it will get fetched as part of each instance's local cache; we
;; can then periodically compare the locally cached value of `settings-last-updated` with the value in the DB. If our
;; locally cached value is older than the one in the DB, we will flush our cache. When the cache is fetched again, it
;; will have the up-to-date value.
;;
;; Because different machines can have out-of-sync clocks, we'll rely entirely on the application DB for caclulating
;; and comparing values of `settings-last-updated`. Because the Setting table itself only stores text values, we'll
;; need to cast it between TEXT and TIMESTAMP SQL types as needed.
(def ^:private ^String settings-last-updated-key "settings-last-updated")
(defn- update-settings-last-updated!
"Update the value of `settings-last-updated` in the DB; if the row does not exist, insert one."
[]
(log/debug (trs "Updating value of settings-last-updated in DB..."))
;; attempt to UPDATE the existing row. If no row exists, `update-where!` will return false...
(or (db/update-where! Setting {:key settings-last-updated-key} :value (hx/cast :text (hsql/raw "current_timestamp")))
;; ...at which point we will try to INSERT a new row. Note that it is entirely possible two instances can both
;; try to INSERT it at the same time; one instance would fail because it would violate the PK constraint on
;; `key`, and throw a SQLException. As long as one instance updates the value, we are fine, so we can go ahead
;; and ignore that Exception if one is thrown.
(try
(db/insert! Setting :key settings-last-updated-key, :value (hx/cast :text (hsql/raw "current_timestamp")))
(catch java.sql.SQLException _)))
;; Now that we updated the value in the DB, go ahead and update our cached value as well, because we know about the
;; changes
(swap! cache assoc settings-last-updated-key (db/select-one-field :value Setting :key settings-last-updated-key)))
(defn- cache-out-of-date?
"Check whether our Settings cache is out of date. We know the cache is out of date if either of the following
conditions is true:
* The cache is empty (the `cache` atom is `nil`), which of course means it needs to be updated
* There is a value of `settings-last-updated` in the cache, and it is older than the value of in the DB. (There
will be no value until the first time a normal Setting is updated; thus if it is not yet set, we do not yet need
to invalidate our cache.)"
[]
(log/debug (trs "Checking whether settings cache is out of date (requires DB call)..."))
(boolean
(or
;; is the cache empty?
(not @cache)
;; if not, get the cached value of `settings-last-updated`, and if it exists...
(when-let [last-known-update (core/get @cache settings-last-updated-key)]
;; compare it to the value in the DB. This is done be seeing whether a row exists
;; WHERE CAST(value AS TIMESTAMP) > CAST(<local-value> AS TIMESTAMP)
(db/select-one Setting
{:where [:and
[:= :key settings-last-updated-key]
[:> (hx/cast :timestamp :value) (hx/cast :timestamp last-known-update)]]})))))
(def ^:private cache-update-check-interval-ms
"How often we should check whether the Settings cache is out of date (which requires a DB call)?"
;; once a minute
(* 60 1000))
(def ^:private ^{:arglists '([])} should-restore-cache?
"TTL-memoized version of `cache-out-of-date?`. Call this function to see whether we need to repopulate the cache with
values from the DB."
(memoize/ttl cache-out-of-date? :ttl/threshold cache-update-check-interval-ms))
(def ^:private restore-cache-if-needed-lock (Object.))
(defn- restore-cache-if-needed!
"Check whether we need to repopulate the cache with fresh values from the DB (because the cache is either empty or
known to be out-of-date), and do so if needed. This is intended to be called every time a Setting value is
retrieved, so it should be efficient; thus the calculation (`should-restore-cache?`) is itself TTL-memoized."
[]
;; There's a potential race condition here where two threads both call this at the exact same moment, and both get
;; `true` when they call `should-restore-cache`, and then both simultaneously try to update the cache (or, one
;; updates the cache, but the other calls `should-restore-cache?` and gets `true` before the other calls
;; `memo-swap!` (see below))
;;
;; This is not desirable, since either situation would result in duplicate work. Better to just add a quick lock
;; here so only one of them does it, since at any rate waiting for the other thread to finish the task in progress is
;; certainly quicker than starting the task ourselves from scratch
(locking restore-cache-if-needed-lock
(when (should-restore-cache?)
(log/debug (trs "Refreshing Settings cache..."))
(reset! cache (db/select-field->field :key :value Setting))
;; Now the cache is up-to-date. That is all good, but if we call `should-restore-cache?` again in a second it
;; will still return `true`, because its result is memoized, and we would be on the hook to (again) update the
;; cache. So go ahead and clear the memozied results for `should-restore-cache?`. The next time around when
;; someone calls this it will cache the latest value (which should be `false`)
;;
;; NOTE: I tried using `memo-swap!` instead to set the cached response to `false` here, avoiding the extra DB
;; call the next fn call would make, but it didn't seem to work correctly (I think it was still discarding the
;; new value because of the TTL). So we will just stick with `memo-clear!` for now. (One extra DB call whenever
;; the cache gets invalidated shouldn't be a huge deal)
(memoize/memo-clear! should-restore-cache?))))
;;; +----------------------------------------------------------------------------------------------------------------+
......@@ -100,14 +203,14 @@
(name (:name (resolve-setting setting-or-name))))
(defn- env-var-name
"Get the env var corresponding to SETTING-OR-NAME.
"Get the env var corresponding to `setting-or-name`.
(This is used primarily for documentation purposes)."
^String [setting-or-name]
(let [setting (resolve-setting setting-or-name)]
(str "MB_" (str/upper-case (str/replace (setting-name setting) "-" "_")))))
(defn env-var-value
"Get the value of SETTING-OR-NAME from the corresponding env var, if any.
"Get the value of `setting-or-name` from the corresponding env var, if any.
The name of the Setting is converted to uppercase and dashes to underscores;
for example, a setting named `default-domain` can be set with the env var `MB_DEFAULT_DOMAIN`."
^String [setting-or-name]
......@@ -117,14 +220,14 @@
v)))
(defn- db-value
"Get the value, if any, of SETTING-OR-NAME from the DB (using / restoring the cache as needed)."
"Get the value, if any, of `setting-or-name` from the DB (using / restoring the cache as needed)."
^String [setting-or-name]
(restore-cache-if-needed!)
(clojure.core/get @cache (setting-name setting-or-name)))
(defn get-string
"Get string value of SETTING-OR-NAME. This is the default getter for `String` settings; valuBis fetched as follows:
"Get string value of `setting-or-name`. This is the default getter for `String` settings; valuBis fetched as follows:
1. From the database (i.e., set via the admin panel), if a value is present;
2. From corresponding env var, if any;
......@@ -144,32 +247,33 @@
(case (str/lower-case string-value)
"true" true
"false" false
(throw (Exception. (str (tru "Invalid value for string: must be either \"true\" or \"false\" (case-insensitive).")))))))
(throw (Exception.
(str (tru "Invalid value for string: must be either \"true\" or \"false\" (case-insensitive).")))))))
(defn get-boolean
"Get boolean value of (presumably `:boolean`) SETTING-OR-NAME. This is the default getter for `:boolean` settings.
"Get boolean value of (presumably `:boolean`) `setting-or-name`. This is the default getter for `:boolean` settings.
Returns one of the following values:
* `nil` if string value of SETTING-OR-NAME is unset (or empty)
* `true` if *lowercased* string value of SETTING-OR-NAME is `true`
* `false` if *lowercased* string value of SETTING-OR-NAME is `false`."
* `nil` if string value of `setting-or-name` is unset (or empty)
* `true` if *lowercased* string value of `setting-or-name` is `true`
* `false` if *lowercased* string value of `setting-or-name` is `false`."
^Boolean [setting-or-name]
(string->boolean (get-string setting-or-name)))
(defn get-integer
"Get integer value of (presumably `:integer`) SETTING-OR-NAME. This is the default getter for `:integer` settings."
"Get integer value of (presumably `:integer`) `setting-or-name`. This is the default getter for `:integer` settings."
^Integer [setting-or-name]
(when-let [s (get-string setting-or-name)]
(Integer/parseInt s)))
(defn get-double
"Get double value of (presumably `:double`) SETTING-OR-NAME. This is the default getter for `:double` settings."
"Get double value of (presumably `:double`) `setting-or-name`. This is the default getter for `:double` settings."
^Double [setting-or-name]
(when-let [s (get-string setting-or-name)]
(Double/parseDouble s)))
(defn get-json
"Get the string value of SETTING-OR-NAME and parse it as JSON."
"Get the string value of `setting-or-name` and parse it as JSON."
[setting-or-name]
(json/parse-string (get-string setting-or-name) keyword))
......@@ -181,7 +285,7 @@
:double get-double})
(defn get
"Fetch the value of SETTING-OR-NAME. What this means depends on the Setting's `:getter`; by default, this looks for
"Fetch the value of `setting-or-name`. What this means depends on the Setting's `:getter`; by default, this looks for
first for a corresponding env var, then checks the cache, then returns the default value of the Setting, if any."
[setting-or-name]
((:getter (resolve-setting setting-or-name))))
......@@ -191,13 +295,14 @@
;;; | set! |
;;; +----------------------------------------------------------------------------------------------------------------+
(defn- update-setting! [setting-name new-value]
(defn- update-setting!
"Update an existing Setting. Used internally by `set-string!` below; do not use directly."
[setting-name new-value]
(db/update-where! Setting {:key setting-name}
:value new-value))
(defn- set-new-setting!
"Insert a new row for a Setting with SETTING-NAME and SETTING-VALUE.
Takes care of resetting the cache if the insert fails for some reason."
"Insert a new row for a Setting. Used internally by `set-string!` below; do not use directly."
[setting-name new-value]
(try (db/insert! Setting
:key setting-name
......@@ -206,13 +311,15 @@
;; and there's actually a row in the DB that's not in the cache for some reason. Go ahead and update the
;; existing value and log a warning
(catch Throwable e
(log/warn "Error INSERTing a new Setting:" (.getMessage e)
"\nAssuming Setting already exists in DB and updating existing value.")
(log/warn (tru "Error inserting a new Setting:") "\n"
(.getMessage e) "\n"
(tru "Assuming Setting already exists in DB and updating existing value."))
(update-setting! setting-name new-value))))
(s/defn set-string!
"Set string value of SETTING-OR-NAME. A `nil` or empty NEW-VALUE can be passed to unset (i.e., delete)
SETTING-OR-NAME."
"Set string value of `setting-or-name`. A `nil` or empty `new-value` can be passed to unset (i.e., delete)
`setting-or-name`. String-type settings use this function directly; all other types ultimately call this (e.g.
`set-boolean!` eventually calls `set-string!`). Returns the `new-value`."
[setting-or-name, new-value :- (s/maybe s/Str)]
(let [new-value (when (seq new-value)
new-value)
......@@ -230,11 +337,14 @@
(if new-value
(swap! cache assoc setting-name new-value)
(swap! cache dissoc setting-name))
;; Record the fact that a Setting has been updated so eventaully other instances (if applicable) find out about it
(update-settings-last-updated!)
;; Now return the `new-value`.
new-value))
(defn set-boolean!
"Set the value of boolean SETTING-OR-NAME. NEW-VALUE can be nil, a boolean, or a string representation of one, such
as `\"true\"` or `\"false\"` (these strings are case-insensitive)."
"Set the value of boolean `setting-or-name`. `new-value` can be nil, a boolean, or a string representation of one,
such as `\"true\"` or `\"false\"` (these strings are case-insensitive)."
[setting-or-name new-value]
(set-string! setting-or-name (if (string? new-value)
(set-boolean! setting-or-name (string->boolean new-value))
......@@ -244,7 +354,7 @@
nil nil))))
(defn set-integer!
"Set the value of integer SETTING-OR-NAME."
"Set the value of integer `setting-or-name`."
[setting-or-name new-value]
(set-string! setting-or-name (when new-value
(assert (or (integer? new-value)
......@@ -253,7 +363,7 @@
(str new-value))))
(defn set-double!
"Set the value of double SETTING-OR-NAME."
"Set the value of double `setting-or-name`."
[setting-or-name new-value]
(set-string! setting-or-name (when new-value
(assert (or (float? new-value)
......@@ -262,7 +372,7 @@
(str new-value))))
(defn set-json!
"Serialize NEW-VALUE for SETTING-OR-NAME as a JSON string and save it."
"Serialize `new-value` for `setting-or-name` as a JSON string and save it."
[setting-or-name new-value]
(set-string! setting-or-name (when new-value
(json/generate-string new-value))))
......@@ -275,7 +385,7 @@
:double set-double!})
(defn set!
"Set the value of SETTING-OR-NAME. What this means depends on the Setting's `:setter`; by default, this just updates
"Set the value of `setting-or-name`. What this means depends on the Setting's `:setter`; by default, this just updates
the Settings cache and writes its value to the DB.
(set :mandrill-api-key \"xyz123\")
......@@ -292,7 +402,7 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defn register-setting!
"Register a new `Setting` with a map of `SettingDefinition` attributes.
"Register a new Setting with a map of `SettingDefinition` attributes.
This is used internally be `defsetting`; you shouldn't need to use it yourself."
[{setting-name :name, setting-type :type, default :default, :as setting}]
(u/prog1 (let [setting-type (s/validate Type (or setting-type :string))]
......@@ -321,21 +431,21 @@
;; Turn on auto-complete-mode in Emacs and see for yourself!
:doc (str/join "\n" [ description
""
(format "`%s` is a %s `Setting`. You can get its value by calling:" (setting-name setting) (name setting-type))
(format "`%s` is a %s Setting. You can get its value by calling:" (setting-name setting) (name setting-type))
""
(format " (%s)" (setting-name setting))
(format " (%s)" (setting-name setting))
""
"and set its value by calling:"
""
(format " (%s <new-value>)" (setting-name setting))
(format " (%s <new-value>)" (setting-name setting))
""
(format "You can also set its value with the env var `%s`." (env-var-name setting))
(format "You can also set its value with the env var `%s`." (env-var-name setting))
""
"Clear its value by calling:"
""
(format " (%s nil)" (setting-name setting))
(format " (%s nil)" (setting-name setting))
""
(format "Its default value is `%s`." (if (nil? default) "nil" default))])})
(format "Its default value is `%s`." (if (nil? default) "nil" default))])})
......@@ -352,7 +462,7 @@
(metabase.models.setting/set! setting new-value))))
(defmacro defsetting
"Defines a new `Setting` that will be added to the DB at some point in the future.
"Defines a new Setting that will be added to the DB at some point in the future.
Conveniently can be used as a getter/setter as well:
(defsetting mandrill-api-key \"API key for Mandrill.\")
......@@ -368,7 +478,7 @@
* `:default` - The default value of the setting. (default: `nil`)
* `:type` - `:string` (default), `:boolean`, `:integer`, or `:json`. Non-`:string` settings have special
default getters and setters that automatically coerce values to the correct types.
* `:internal?` - This `Setting` is for internal use and shouldn't be exposed in the UI (i.e., not returned by the
* `:internal?` - This Setting is for internal use and shouldn't be exposed in the UI (i.e., not returned by the
corresponding endpoints). Default: `false`
* `:getter` - A custom getter fn, which takes no arguments. Overrides the default implementation. (This can in
turn call functions in this namespace like `get-string` or `get-boolean` to invoke the default
......@@ -392,7 +502,7 @@
;;; +----------------------------------------------------------------------------------------------------------------+
(defn set-many!
"Set the value of several `Settings` at once.
"Set the value of several Settings at once.
(set-all {:mandrill-api-key \"xyz123\", :another-setting \"ABC\"})"
[settings]
......
src/metabase/util/date.clj
+ 1
− 0
View file @ 5209d6dc
(ns metabase.util.date
"Utility functions for working with datetimes of different types, and other related tasks."
(:require [clj-time
[coerce :as coerce]
[core :as t]
......
test/metabase/models/setting_test.clj
+ 119
− 2
View file @ 5209d6dc
(ns metabase.models.setting-test
(:require [expectations :refer :all]
(:require [clojure.core.memoize :as memoize]
[expectations :refer :all]
[honeysql.core :as hsql]
[metabase.models.setting :as setting :refer [defsetting Setting]]
[metabase.test.util :refer :all]
[metabase.util.honeysql-extensions :as hx]
[puppetlabs.i18n.core :refer [tru]]
[toucan.db :as db]))
......@@ -264,8 +267,122 @@
;; restore the cache
((resolve 'metabase.models.setting/restore-cache-if-needed!))
;; now set a value for the `toucan-name` setting the wrong way
(db/insert! setting/Setting {:key "toucan-name", :value "Rasta"})
(db/insert! setting/Setting {:key "toucan-name", :value "Reggae"})
;; ok, now try to set the Setting the correct way
(toucan-name "Banana Beak")
;; ok, make sure the setting was set
(toucan-name)))
;;; --------------------------------------------- Cache Synchronization ----------------------------------------------
(def ^:private settings-last-updated-key @(resolve 'metabase.models.setting/settings-last-updated-key))
(defn- clear-settings-last-updated-value-in-db! []
(db/simple-delete! Setting {:key settings-last-updated-key}))
(defn- settings-last-updated-value-in-db []
(db/select-one-field :value Setting :key settings-last-updated-key))
(defn- clear-cache! []
(reset! @(resolve 'metabase.models.setting/cache) nil))
(defn- settings-last-updated-value-in-cache []
(get @@(resolve 'metabase.models.setting/cache) settings-last-updated-key))
(defn- update-settings-last-updated-value-in-db!
"Simulate a different instance updating the value of `settings-last-updated` in the DB by updating its value without
updating our locally cached value.."
[]
(db/update-where! Setting {:key settings-last-updated-key} :value (hx/cast :text (hsql/raw "current_timestamp"))))
(defn- simulate-another-instance-updating-setting! [setting-name new-value]
(db/update-where! Setting {:key (name setting-name)} :value new-value)
(update-settings-last-updated-value-in-db!))
(defn- flush-memoized-results-for-should-restore-cache!
"Remove any memoized results for `should-restore-cache?`, so we can test `restore-cache-if-needed!` works the way we'd
expect."
[]
(memoize/memo-clear! @(resolve 'metabase.models.setting/should-restore-cache?)))
;; When I update a Setting, does it set/update `settings-last-updated`?
(expect
(do
(clear-settings-last-updated-value-in-db!)
(toucan-name "Bird Can")
(string? (settings-last-updated-value-in-db))))
;; ...and is the value updated in the cache as well?
(expect
(do
(clear-cache!)
(toucan-name "Bird Can")
(string? (settings-last-updated-value-in-cache))))
;; ...and if I update it again, will the value be updated?
(expect
(do
(clear-settings-last-updated-value-in-db!)
(toucan-name "Bird Can")
(let [first-value (settings-last-updated-value-in-db)]
(toucan-name "Bird Can")
(let [second-value (settings-last-updated-value-in-db)]
;; first & second values should be different, and first value should be "less than" the second value
(and (not= first-value second-value)
(neg? (compare first-value second-value)))))))
;; If there is no cache, it should be considered out of date!`
(expect
(do
(clear-cache!)
(#'setting/cache-out-of-date?)))
;; But if I set a setting, it should cause the cache to be populated, and be up-to-date
(expect
false
(do
(clear-cache!)
(toucan-name "Reggae Toucan")
(#'setting/cache-out-of-date?)))
;; If another instance updates a Setting, `cache-out-of-date?` should return `true` based on DB comparisons...
;; be true!
(expect
(do
(clear-cache!)
(toucan-name "Reggae Toucan")
(simulate-another-instance-updating-setting! :toucan-name "Bird Can")
(#'setting/cache-out-of-date?)))
;; of course, `restore-cache-if-needed!` should use TTL memoization, and the cache should not get updated right away
;; even if another instance updates a value...
(expect
"Sam"
(do
(flush-memoized-results-for-should-restore-cache!)
(clear-cache!)
(toucan-name "Sam") ; should restore cache, and put in {"toucan-name" "Sam"}
;; since we cleared the memoized value of `should-restore-cache?` call it again to make sure it gets set to
;; `false` as it would IRL if we were calling it again from the same instance
(#'setting/should-restore-cache?)
;; now have another instance change the value
(simulate-another-instance-updating-setting! :toucan-name "Bird Can")
;; our cache should not be updated yet because it's on a TTL
(toucan-name)))
;; ...and when it comes time to check our cache for updating (when calling `restore-cache-if-needed!`, it should get
;; the updated value. (we're not actually going to wait a minute for the memoized values of `should-restore-cache?` to
;; be invalidated, so we will manually flush the memoization cache to simulate it happening)
(expect
"Bird Can"
(do
(clear-cache!)
(toucan-name "Reggae Toucan")
(simulate-another-instance-updating-setting! :toucan-name "Bird Can")
(flush-memoized-results-for-should-restore-cache!)
;; calling `toucan-name` will call `restore-cache-if-needed!`, which will in turn call `should-restore-cache?`.
;; Since memoized value is no longer present, this should call `cache-out-of-date?`, which checks the DB; it will
;; detect a cache out-of-date situation and flush the cache as appropriate, giving us the updated value when we
;; call! :wow:
(toucan-name)))
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment