diff --git a/packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationLoadCache.ts b/packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationLoadCache.ts index e2c94d02c6349..d4ef0d6c6346b 100644 --- a/packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationLoadCache.ts +++ b/packages/cubejs-query-orchestrator/src/orchestrator/PreAggregationLoadCache.ts @@ -194,14 +194,18 @@ export class PreAggregationLoadCache { const [query, values, queryOptions]: QueryWithParams = Array.isArray(sqlQuery) ? sqlQuery : [sqlQuery, [], {}]; if (!this.queryResults[this.queryCache.queryRedisKey([query, values])]) { + const renewalThreshold = this.queryCache.options.refreshKeyRenewalThreshold + || queryOptions?.renewalThreshold + || 2 * 60; + const expiration = Math.max(renewalThreshold, 60 * 60); + this.queryResults[this.queryCache.queryRedisKey([query, values])] = await this.queryCache.cacheQueryResult( query, values, [query, values], - 60 * 60, + expiration, { - renewalThreshold: this.queryCache.options.refreshKeyRenewalThreshold - || queryOptions?.renewalThreshold || 2 * 60, + renewalThreshold, renewalKey: [query, values], waitForRenew, priority, diff --git a/packages/cubejs-query-orchestrator/test/unit/PreAggregationLoadCache.test.ts b/packages/cubejs-query-orchestrator/test/unit/PreAggregationLoadCache.test.ts new file mode 100644 index 0000000000000..e8d79c6ae684d --- /dev/null +++ b/packages/cubejs-query-orchestrator/test/unit/PreAggregationLoadCache.test.ts @@ -0,0 +1,89 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { PreAggregationLoadCache } from '../../src/orchestrator/PreAggregationLoadCache'; + +const FIFTEEN_DAYS_SECS = 15 * 24 * 60 * 60; +const ONE_HOUR_SECS = 60 * 60; + +describe('PreAggregationLoadCache', () => { + describe('keyQueryResult', () => { + let cacheQueryResult: jest.Mock; + let queryRedisKey: jest.Mock; + let queryCache: { cacheQueryResult: jest.Mock; queryRedisKey: jest.Mock; options: Record }; + let loadCache: PreAggregationLoadCache; + + const callKeyQueryResult = (renewalThreshold?: number, refreshKeyRenewalThreshold?: number) => { + if (refreshKeyRenewalThreshold !== undefined) { + queryCache.options.refreshKeyRenewalThreshold = refreshKeyRenewalThreshold; + } else { + delete queryCache.options.refreshKeyRenewalThreshold; + } + + const sqlQuery: [string, unknown[], { renewalThreshold?: number }] = [ + 'SELECT MAX(loaded_at)', + [], + renewalThreshold !== undefined ? { renewalThreshold } : {}, + ]; + + return loadCache.keyQueryResult(sqlQuery, false, 10); + }; + + beforeEach(() => { + cacheQueryResult = jest.fn().mockResolvedValue('result'); + queryRedisKey = jest.fn().mockReturnValue('redis-key'); + queryCache = { + cacheQueryResult, + queryRedisKey, + options: {}, + }; + + loadCache = new PreAggregationLoadCache( + {} as any, + queryCache as any, + {} as any, + { dataSource: 'default' }, + ); + }); + + it('uses renewalThreshold for both expiration and renewal when every is long', async () => { + await callKeyQueryResult(FIFTEEN_DAYS_SECS); + + expect(cacheQueryResult).toHaveBeenCalledWith( + 'SELECT MAX(loaded_at)', + [], + ['SELECT MAX(loaded_at)', []], + FIFTEEN_DAYS_SECS, + expect.objectContaining({ + renewalThreshold: FIFTEEN_DAYS_SECS, + }), + ); + }); + + it('keeps minimum 1-hour expiration for short renewalThreshold (backward compatible)', async () => { + await callKeyQueryResult(10); + + expect(cacheQueryResult).toHaveBeenCalledWith( + 'SELECT MAX(loaded_at)', + [], + ['SELECT MAX(loaded_at)', []], + ONE_HOUR_SECS, + expect.objectContaining({ + renewalThreshold: 10, + }), + ); + }); + + it('uses refreshKeyRenewalThreshold for both expiration and renewal when set globally', async () => { + await callKeyQueryResult(FIFTEEN_DAYS_SECS, 3600); + + expect(cacheQueryResult).toHaveBeenCalledWith( + 'SELECT MAX(loaded_at)', + [], + ['SELECT MAX(loaded_at)', []], + 3600, + expect.objectContaining({ + renewalThreshold: 3600, + }), + ); + }); + }); +});