Skip to content

Commit

Permalink
Bust RunTimeline cache (#23482)
Browse files Browse the repository at this point in the history 
## Summary & Motivation

1. Add a version parameter to `HourlyDataCache`
2. If the version in the indexeddb cache doesn't match the version in
code then don't use the indexeddb cache data at all
3. Add version parameter to useRunTImeline

## How I Tested These Changes

Loaded the run timeline using app-proxy and saw that the cache was
cleared. Subsequent loads used the cache
  • Loading branch information
@salazarm
salazarm authored Aug 7, 2024
1 parent 5567671 commit f134bc6
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 59 deletions.
48 changes: 37 additions & 11 deletions js_modules/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/HourlyDataCache.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,38 @@ export const defaultOptions = {
expiry: new Date('3030-01-01'), // never expire,
};

type CacheType<T> = {
version: string | number;
cache: InstanceType<typeof HourlyDataCache<T>>['cache'];
};

export class HourlyDataCache<T> {
private cache: Map<number, Array<TimeWindow<T>>> = new Map();
private subscriptions: Array<{hour: number; callback: Subscription<T>}> = [];
private indexedDBCache?: ReturnType<typeof cache<string, typeof this.cache>>;
private indexedDBCache?: ReturnType<typeof cache<string, CacheType<T>>>;
private indexedDBKey: string;
private version: string | number;

/**
* @param id A unique ID for the hourly data cache in this deployment
* @param [keyPrefix=''] A unique key identifying the timeline view [incorporating filters, etc.]
*/
constructor(id?: string | false, keyPrefix = '', keyMaxCount = 1) {
constructor({
id,
keyPrefix = '',
keyMaxCount = 1,
version,
}: {
id?: string | false;
keyPrefix?: string;
keyMaxCount?: number;
version: string | number;
}) {
this.version = version;
this.indexedDBKey = keyPrefix ? `${keyPrefix}-hourlyData` : 'hourlyData';

// Delete old database from before the prefix, remove this at some point
indexedDB.deleteDatabase('HourlyDataCache:useRunsForTimeline');

if (id) {
this.indexedDBCache = cache<string, typeof this.cache>({
this.indexedDBCache = cache<string, CacheType<T>>({
dbName: `HourlyDataCache:${id}`,
maxCount: keyMaxCount,
});
Expand All @@ -54,8 +68,8 @@ export class HourlyDataCache<T> {
return;
}
const cachedData = await this.indexedDBCache.get(this.indexedDBKey);
if (cachedData) {
this.cache = new Map(cachedData.value);
if (cachedData && cachedData.value.version === this.version) {
this.cache = new Map(cachedData.value.cache);
}
res();
});
Expand All @@ -69,23 +83,35 @@ export class HourlyDataCache<T> {
if (!this.indexedDBCache) {
return;
}
this.indexedDBCache.set(this.indexedDBKey, this.cache, defaultOptions);
this.indexedDBCache.set(
this.indexedDBKey,
{version: this.version, cache: this.cache},
defaultOptions,
);
return;
}
clearTimeout(this.saveTimeout);
this.saveTimeout = setTimeout(() => {
if (!this.indexedDBCache) {
return;
}
this.indexedDBCache.set(this.indexedDBKey, this.cache, defaultOptions);
this.indexedDBCache.set(
this.indexedDBKey,
{version: this.version, cache: this.cache},
defaultOptions,
);
}, 10000);
if (!this.registeredUnload) {
this.registeredUnload = true;
window.addEventListener('beforeunload', () => {
if (!this.indexedDBCache) {
return;
}
this.indexedDBCache.set(this.indexedDBKey, this.cache, defaultOptions);
this.indexedDBCache.set(
this.indexedDBKey,
{version: this.version, cache: this.cache},
defaultOptions,
);
});
}
}
Expand Down
42 changes: 25 additions & 17 deletions ...s/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/__tests__/HourlyDataCache.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ jest.mock('idb-lru-cache', () => {
};
});

const VERSION = 1;

describe('HourlyDataCache', () => {
let cache: HourlyDataCache<number>;

beforeEach(() => {
cache = new HourlyDataCache<number>('test');
cache = new HourlyDataCache<number>({id: 'test', version: VERSION});
});

describe('addData', () => {
Expand Down Expand Up @@ -123,7 +125,7 @@ describe('HourlyDataCache Subscriptions', () => {
let cache: HourlyDataCache<number>;

beforeEach(() => {
cache = new HourlyDataCache<number>();
cache = new HourlyDataCache<number>({version: VERSION});
});

it('should notify subscriber immediately with existing data', () => {
Expand Down Expand Up @@ -200,10 +202,13 @@ describe('HourlyDataCache with IndexedDB', () => {
const sec = Date.now() / 1000;
const nowHour = Math.floor(sec / ONE_HOUR_S);
mockedCache.get.mockResolvedValue({
value: new Map([[nowHour, [{start: nowHour, end: nowHour + ONE_HOUR_S, data: [1, 2, 3]}]]]),
value: {
version: VERSION,
cache: new Map([[nowHour, [{start: nowHour, end: nowHour + ONE_HOUR_S, data: [1, 2, 3]}]]]),
},
});

const cache = new HourlyDataCache<number>('test');
const cache = new HourlyDataCache<number>({id: 'test', version: VERSION});

await cache.loadCacheFromIndexedDB();

Expand All @@ -213,13 +218,13 @@ describe('HourlyDataCache with IndexedDB', () => {
});

it('should save cache to IndexedDB when data is added', async () => {
const cache = new HourlyDataCache<number>('test');
const cache = new HourlyDataCache<number>({id: 'test', version: VERSION});

cache.addData(0, ONE_HOUR_S, [1, 2, 3]);

const mockCallArgs = mockedCache.set.mock.calls[0];
const map = mockCallArgs[1];
expect(map).toEqual(
expect(map.cache).toEqual(
new Map<number, {data: number[]; end: number; start: number}[]>([
[0, [{data: [1, 2, 3], end: 3600, start: 0}]],
]),
Expand All @@ -232,19 +237,22 @@ describe('HourlyDataCache with IndexedDB', () => {

mockedCache.has.mockResolvedValue(true);
mockedCache.get.mockResolvedValue({
value: new Map([
[
Math.floor(eightDaysAgo / ONE_HOUR_S),
[{start: eightDaysAgo, end: eightDaysAgo + ONE_HOUR_S, data: [1, 2, 3]}],
],
[
Math.floor(sixDaysAgo / ONE_HOUR_S),
[{start: sixDaysAgo, end: eightDaysAgo + ONE_HOUR_S, data: [1, 2, 3]}],
],
]),
value: {
version: VERSION,
cache: new Map([
[
Math.floor(eightDaysAgo / ONE_HOUR_S),
[{start: eightDaysAgo, end: eightDaysAgo + ONE_HOUR_S, data: [1, 2, 3]}],
],
[
Math.floor(sixDaysAgo / ONE_HOUR_S),
[{start: sixDaysAgo, end: eightDaysAgo + ONE_HOUR_S, data: [1, 2, 3]}],
],
]),
},
});

const cache = new HourlyDataCache<number>('test');
const cache = new HourlyDataCache<number>({id: 'test', version: VERSION});

await cache.loadCacheFromIndexedDB();

Expand Down
50 changes: 27 additions & 23 deletions js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/useRunsForTimeline.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
COMPLETED_RUN_TIMELINE_QUERY,
FUTURE_TICKS_QUERY,
ONGOING_RUN_TIMELINE_QUERY,
QUERY_VERSION,
useRunsForTimeline,
} from '../useRunsForTimeline';

Expand Down Expand Up @@ -573,32 +574,35 @@ describe('useRunsForTimeline', () => {

const startHour = Math.floor(start / ONE_HOUR_S);
mockedCache.get.mockResolvedValue({
value: new Map([
[
startHour,
value: {
version: QUERY_VERSION,
cache: new Map([
[
{
start: cachedRange[0],
end: cachedRange[1],
data: [
buildRun({
id: 'cached-run',
pipelineName: 'pipeline0',
repositoryOrigin: buildRepositoryOrigin({
id: '1-1',
repositoryName: 'repo1',
repositoryLocationName: 'repo1',
startHour,
[
{
start: cachedRange[0],
end: cachedRange[1],
data: [
buildRun({
id: 'cached-run',
pipelineName: 'pipeline0',
repositoryOrigin: buildRepositoryOrigin({
id: '1-1',
repositoryName: 'repo1',
repositoryLocationName: 'repo1',
}),
startTime: initialRange[0],
endTime: initialRange[1],
updateTime: initialRange[1],
status: RunStatus.SUCCESS,
}),
startTime: initialRange[0],
endTime: initialRange[1],
updateTime: initialRange[1],
status: RunStatus.SUCCESS,
}),
],
},
],
},
],
],
],
]),
]),
},
});

const {result} = renderHook(
Expand Down
20 changes: 12 additions & 8 deletions js_modules/dagster-ui/packages/ui-core/src/runs/useRunsForTimeline.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {workspacePipelinePath} from '../workspace/workspacePath';

const BATCH_LIMIT = 500;

export const QUERY_VERSION = 1;

export const useRunsForTimeline = ({
rangeMs,
filter,
Expand Down Expand Up @@ -61,15 +63,17 @@ export const useRunsForTimeline = ({
const {localCacheIdPrefix} = useContext(AppContext);
const completedRunsCache = useMemo(() => {
if (filter) {
return new HourlyDataCache<RunTimelineFragment>(
localCacheIdPrefix ? `${localCacheIdPrefix}-useRunsForTimeline-filtered` : false,
JSON.stringify(filter),
3,
);
return new HourlyDataCache<RunTimelineFragment>({
id: localCacheIdPrefix ? `${localCacheIdPrefix}-useRunsForTimeline-filtered` : false,
keyPrefix: JSON.stringify(filter),
keyMaxCount: 3,
version: QUERY_VERSION,
});
}
return new HourlyDataCache<RunTimelineFragment>(
localCacheIdPrefix ? `${localCacheIdPrefix}-useRunsForTimeline` : false,
);
return new HourlyDataCache<RunTimelineFragment>({
id: localCacheIdPrefix ? `${localCacheIdPrefix}-useRunsForTimeline` : false,
version: QUERY_VERSION,
});
}, [filter, localCacheIdPrefix]);
const [completedRuns, setCompletedRuns] = useState<RunTimelineFragment[]>([]);

Expand Down

1 comment on commit f134bc6

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-knunuz654-elementl.vercel.app

Built with commit f134bc6.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.