-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(query-orchestrator): compute interval refresh keys locally instead of querying #11614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
65e759d
c93362d
5e9d484
8956c9a
d715c4b
2131ec4
4f5f6d2
151b09b
a9672ca
13d8373
01043d1
0a5b5c7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1417,6 +1417,36 @@ this variable. | |
| | ------------------------- | ---------------------- | --------------------- | | ||
| | A valid number in seconds | `600` | `600` | | ||
|
|
||
| ## `CUBEJS_REFRESH_KEY_LOCAL_TIME` | ||
|
|
||
| To check an `every`-based [`refresh_key`](/reference/data-modeling/cube#refresh_key), Cube | ||
| asks the database what time it is. If `true`, it uses its own clock instead and those | ||
| queries stop entirely, including the ones scheduled refresh issues to warm them. | ||
|
|
||
| | Possible Values | Default in Development | Default in Production | | ||
| | --------------- | ---------------------- | --------------------- | | ||
| | `true`, `false` | `false` | `false` | | ||
|
|
||
| Cube already caches each result for a fraction of the interval, so this is not one query | ||
| saved per request — it is one per refresh key per renewal window, multiplied by every cube, | ||
| tenant, and timezone in the deployment. | ||
|
|
||
| Refresh keys written with `sql` or marked | ||
| [`incremental`](/reference/data-modeling/pre-aggregations#incremental) keep querying the | ||
| database, since their values depend on your data and not just on the time. So do all | ||
| refresh keys if you have set | ||
| [`queryCacheOptions.refreshKeyRenewalThreshold`][ref-config-query-cache-options], which | ||
| works by caching the query result. | ||
|
|
||
| <Warning> | ||
|
|
||
| Cube now trusts each machine's clock, so they all have to agree. Set this variable to the | ||
| same value on **every** API instance and refresh worker and keep them synchronized with | ||
| NTP. Instances on opposite sides of an interval boundary compute different refresh keys, | ||
| which can build the same pre-aggregation twice. | ||
|
claude[bot] marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Narrowed from the previous docs thread (resolved —
|
||
|
|
||
| </Warning> | ||
|
|
||
| ## `CUBEJS_REFRESH_WORKER` | ||
|
|
||
| If `true`, this instance of Cube will **only** refresh pre-aggregations. | ||
|
|
@@ -2307,6 +2337,7 @@ The port for a Cube deployment to listen to API connections on. | |
| https://motherduck.com/docs/authenticating-to-motherduck/#authentication-using-a-service-token | ||
| [ref-config-db]: /admin/connect-to-data/data-sources | ||
| [ref-config-driver-factory]: /reference/configuration/config#driver_factory | ||
| [ref-config-query-cache-options]: /reference/configuration/config#orchestrator_options | ||
| [ref-config-multiple-ds-cloud]: /admin/connect-to-data/multiple-data-sources | ||
| [ref-config-multiple-ds-decorating-env]: /admin/connect-to-data/multiple-data-sources#under-the-hood | ||
| [ref-preagg-data-source]: /docs/pre-aggregations/refreshing-pre-aggregations#pre-aggregation-data-source | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,12 @@ import { ContinueWaitError } from './ContinueWaitError'; | |
| import { LocalCacheDriver } from './LocalCacheDriver'; | ||
| import { DriverFactory, DriverFactoryByDataSource } from './DriverFactory'; | ||
| import { LoadPreAggregationResult, PreAggregationDescription } from './PreAggregations'; | ||
| import { getCacheHash, extractRequestUUID } from './utils'; | ||
| import { | ||
| getCacheHash, | ||
| extractRequestUUID, | ||
| evaluateLocalRefreshKey, | ||
| isValidLocalRefreshKey, | ||
| } from './utils'; | ||
| import { CacheAndQueryDriverType, MetadataOperationType } from './QueryOrchestrator'; | ||
|
|
||
| export type CacheQueryResultOptions = { | ||
|
|
@@ -52,12 +57,24 @@ export type CacheQueryResultOptions = { | |
| export type RefreshKeyCacheOptions = | ||
| Pick<CacheQueryResultOptions, 'priority' | 'requestId' | 'waitForRenew' | 'dataSource'>; | ||
|
|
||
| /** | ||
| * Everything needed to evaluate an `every` based refreshKey without touching a | ||
| * database: `FLOOR((utcOffset + unixTimestamp - dayOffset) / interval)`. | ||
| */ | ||
| export type LocalRefreshKeyDescriptor = { | ||
| interval: number; | ||
| utcOffset: number; | ||
| dayOffset: number; | ||
| cron?: boolean; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: |
||
| }; | ||
|
|
||
| type QueryOptions = { | ||
| external?: boolean; | ||
| renewalThreshold?: number; | ||
| updateWindowSeconds?: number; | ||
| renewalThresholdOutsideUpdateWindow?: number; | ||
| incremental?: boolean; | ||
| localRefreshKey?: LocalRefreshKeyDescriptor; | ||
| }; | ||
|
|
||
| export type QueryWithParams = [ | ||
|
|
@@ -182,6 +199,10 @@ export class QueryCache { | |
|
|
||
| protected static readonly IN_MEMORY_CACHE_DISABLE_PERIOD = 5 * 60 * 1000; | ||
|
|
||
| protected readonly localRefreshKeyEnabled: boolean; | ||
|
|
||
| protected localRefreshKeyLogged: boolean = false; | ||
|
|
||
| public constructor( | ||
| protected readonly cachePrefix: string, | ||
| protected readonly driverFactory: DriverFactoryByDataSource, | ||
|
|
@@ -208,6 +229,52 @@ export class QueryCache { | |
| this.memoryCache = new LRUCache<string, CacheEntry>({ | ||
| max: options.maxInMemoryCacheEntries || 10000 | ||
| }); | ||
|
|
||
| // Read from the environment rather than an option: the emitting half in the schema | ||
| // compiler is wired straight from the same variable, and an option here could be set | ||
| // on its own, turning the feature into a no-op where no descriptor is ever emitted. | ||
| this.localRefreshKeyEnabled = getEnv('refreshKeyLocalTime'); | ||
| } | ||
|
|
||
| /** | ||
| * Whether interval based refresh keys are answered from this instance clock instead of being | ||
| * run as queries and cached. | ||
| */ | ||
| public isLocalRefreshKeyActive(): boolean { | ||
| return this.localRefreshKeyEnabled && !this.options.refreshKeyRenewalThreshold; | ||
| } | ||
|
|
||
| public localRefreshKeyResult(queryOptions?: QueryOptions): [{ refresh_key: string }] | null { | ||
| if (!this.localRefreshKeyEnabled || !isValidLocalRefreshKey(queryOptions?.localRefreshKey)) { | ||
| return null; | ||
| } | ||
|
|
||
| // `refreshKeyRenewalThreshold` throttles how often the SQL result is re-read, and that is | ||
| // also what bounds how often the key advances: a value cached for a day advances daily, | ||
| // whatever `every` says. A locally evaluated key has no cache entry to age out, so the only | ||
| // way to keep honouring the override is to leave these keys on the SQL path. | ||
| if (!this.isLocalRefreshKeyActive()) { | ||
| this.logLocalRefreshKeyOnce('Local refresh key evaluation skipped', { | ||
| warning: 'refreshKeyRenewalThreshold is set, which throttles refresh key advancement. ' + | ||
| 'Interval based refresh keys keep running as queries. Unset it to evaluate them locally.', | ||
| refreshKeyRenewalThreshold: this.options.refreshKeyRenewalThreshold, | ||
| }); | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| return evaluateLocalRefreshKey(<LocalRefreshKeyDescriptor>queryOptions?.localRefreshKey); | ||
| } | ||
|
|
||
| // Logged on first use rather than from the constructor, where subclass field | ||
| // initialization has not run yet and `this.logger` may not be usable. | ||
|
claude[bot] marked this conversation as resolved.
Outdated
|
||
| private logLocalRefreshKeyOnce(message: string, meta: Record<string, unknown>) { | ||
| if (this.localRefreshKeyLogged) { | ||
| return; | ||
| } | ||
|
|
||
| this.localRefreshKeyLogged = true; | ||
| this.logger(message, meta); | ||
| } | ||
|
|
||
| public getCacheDriver(): CacheDriverInterface { | ||
|
|
@@ -462,6 +529,13 @@ export class QueryCache { | |
| options: RefreshKeyCacheOptions, | ||
| ) { | ||
| const [query, values, queryOptions] = sqlQuery; | ||
|
|
||
| // A locally evaluated key is free: nothing to cache, no queue to wait on. | ||
| const local = this.localRefreshKeyResult(queryOptions); | ||
|
claude[bot] marked this conversation as resolved.
|
||
| if (local) { | ||
| return local; | ||
| } | ||
|
|
||
| const cacheKey = QueryCache.refreshKeyIdentity(sqlQuery, options.dataSource); | ||
|
|
||
| return this.cacheQueryResult(query, values, cacheKey, expiration, { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-anchored — the tightening in
8956c9amoved this sentence, so the old thread went outdated; the concern is unchanged, and the shorter phrasing actually sharpens it."their values depend on your data and not just on the time" is right for
sqlbut not forincremental. An incremental refresh key is time arithmetic — it starts from the sameeveryinterval and is then wrapped inCASE WHEN NOW() < <dateTo + updateWindow>against an allocated partition-range parameter (BaseQuery.js:5088-5101; the code comment there says as much). Its value doesn't depend on your data at all; it depends on which partition is being checked, which the API instance can't reproduce from an interval alone.As written, someone with an
incrementalrollup will conclude their refresh key reads the fact table on every check. Giving each its own reason keeps the plain language and stays true, e.g. "…keep querying the database — asqlkey's value comes from your data, and anincrementalkey's depends on which partition is being checked."Fix this →