Skip to content
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

[Dependend] Reset chunks from cache if oudated #8449

Merged
61 changes: 59 additions & 2 deletions cvat-core/src/frames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { FieldUpdateTrigger } from './common';
// frame storage by job id
const frameDataCache: Record<string, {
meta: FramesMetaData;
metaFetchedTimestamp: number;
chunkSize: number;
mode: 'annotation' | 'interpolation';
startFrame: number;
Expand Down Expand Up @@ -57,6 +58,7 @@ export class FramesMetaData {
public stopFrame: number;
public frameStep: number;
public chunkCount: number;
public chunksUpdatedDate: string;

#updateTrigger: FieldUpdateTrigger;

Expand All @@ -71,6 +73,7 @@ export class FramesMetaData {
size: undefined,
start_frame: undefined,
stop_frame: undefined,
chunks_updated_date: undefined,
};

this.#updateTrigger = new FieldUpdateTrigger();
Expand Down Expand Up @@ -149,6 +152,9 @@ export class FramesMetaData {
frameStep: {
get: () => frameStep,
},
chunksUpdatedDate: {
get: () => data.chunks_updated_date,
},
}),
);

Expand Down Expand Up @@ -592,13 +598,45 @@ function getFrameMeta(jobID, frame): SerializedFramesMetaData['frames'][0] {
return frameMeta;
}

async function refreshJobCacheIfOutdated(jobID: number): Promise<void> {
const cached = frameDataCache[jobID];
if (!cached) {
throw new Error('Frame data cache is abscent');
}

const META_DATA_RELOAD_PERIOD = 1 * 60 * 60 * 1000; // 1 hour
const isOutdated = (Date.now() - cached.metaFetchedTimestamp) > META_DATA_RELOAD_PERIOD;

if (isOutdated) {
// get metadata again if outdated
const meta = await getFramesMeta('job', jobID, true);
if (new Date(meta.chunksUpdatedDate) > new Date(cached.meta.chunksUpdatedDate)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Validate comparison of chunksUpdatedDate - Potential improvement needed

The comparison of meta.chunksUpdatedDate and cached.meta.chunksUpdatedDate is implemented correctly using new Date(). However, there's no explicit validation to ensure these are valid date strings. While the type is declared as string, which is appropriate, consider adding a safeguard against potential Invalid Date issues:

  • In the if statement at line 512, add a check to ensure both dates are valid before comparison:
if (!isNaN(new Date(meta.chunksUpdatedDate).getTime()) && 
    !isNaN(new Date(cached.meta.chunksUpdatedDate).getTime()) &&
    new Date(meta.chunksUpdatedDate) > new Date(cached.meta.chunksUpdatedDate)) {
    // existing code
}

This addition will prevent potential runtime errors if either chunksUpdatedDate is invalid, improving the robustness of the code.

Analysis chain

Validate comparison of chunksUpdatedDate

Ensure that meta.chunksUpdatedDate and cached.meta.chunksUpdatedDate are valid date strings to avoid potential Invalid Date issues during comparison.

Run the following script to check if chunksUpdatedDate is a valid date string:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if chunksUpdatedDate is a valid date in the meta data

# Test: Extract chunksUpdatedDate and attempt to parse it as a date
# Expect: Both dates should be valid

jq '.chunksUpdatedDate' cvat-core/src/frames.ts | xargs -I {} date -d {}

Length of output: 133


Script:

#!/bin/bash
# Search for usage of chunksUpdatedDate in the TypeScript file
ast-grep --lang typescript --pattern 'new Date($chunksUpdatedDate)' cvat-core/src/frames.ts

# Search for any type assertions or validations related to chunksUpdatedDate
rg "chunksUpdatedDate" cvat-core/src/frames.ts -A 5 -B 5

Length of output: 1515

// chunks were re-defined. Existing data not relevant anymore
// currently we only re-write meta, remove all cached frames from provider and clear cached context images
// other parameters (e.g. chunkSize) are not supposed to be changed
cached.meta = meta;
cached.provider.cleanup(Number.MAX_SAFE_INTEGER);
for (const frame of Object.keys(cached.contextCache)) {
for (const image of Object.values(cached.contextCache[+frame].data)) {
// close images to immediate memory release
image.close();
}
}
cached.contextCache = {};
}

cached.metaFetchedTimestamp = Date.now();
}
}

export function getContextImage(jobID: number, frame: number): Promise<Record<string, ImageBitmap>> {
return new Promise<Record<string, ImageBitmap>>((resolve, reject) => {
if (!(jobID in frameDataCache)) {
reject(new Error(
'Frame data was not initialized for this job. Try first requesting any frame.',
));
}

bsekachev marked this conversation as resolved.
Show resolved Hide resolved
const frameData = frameDataCache[jobID];
const requestId = frame;
const { startFrame } = frameData;
Expand Down Expand Up @@ -695,7 +733,9 @@ export async function getFrame(
dimension: DimensionType,
getChunk: (chunkIndex: number, quality: ChunkQuality) => Promise<ArrayBuffer>,
): Promise<FrameData> {
if (!(jobID in frameDataCache)) {
const dataCacheExists = jobID in frameDataCache;

if (!dataCacheExists) {
const blockType = chunkType === 'video' ? BlockType.MP4VIDEO : BlockType.ARCHIVE;
const meta = await getFramesMeta('job', jobID);

Expand All @@ -718,6 +758,7 @@ export async function getFrame(

frameDataCache[jobID] = {
meta,
metaFetchedTimestamp: Date.now(),
chunkSize,
mode,
startFrame,
Expand All @@ -743,6 +784,22 @@ export async function getFrame(
};
}

// basically the following functions may be affected if job cache is outdated
// - getFrame
// - getContextImage
// - getCachedChunks
// And from this idea we should call refreshJobCacheIfOutdated from each one
// Hovewer, following from the order, these methods are usually called
// it may lead to even more confusing behaviour
//
// Usually user first receives frame, then user receives ranges and finally user receives context images
// In this case (extremely rare, but nevertheless possible) user may get context images related to another frame
// - if cache gets outdated after getFrame() call
// - and before getContextImage() call
// - and both calls refer to the same frame that is refreshed honeypot frame and this frame has context images
// Thus, it is better to only call `refreshJobCacheIfOutdated` from getFrame()
await refreshJobCacheIfOutdated(jobID);

const frameMeta = getFrameMeta(jobID, frame);
frameDataCache[jobID].provider.setRenderSize(frameMeta.width, frameMeta.height);
frameDataCache[jobID].decodeForward = isPlaying;
Expand All @@ -759,7 +816,7 @@ export async function getFrame(
});
}

export async function getDeletedFrames(instanceType: 'job' | 'task', id): Promise<Record<number, boolean>> {
export async function getDeletedFrames(instanceType: 'job' | 'task', id: number): Promise<Record<number, boolean>> {
if (instanceType === 'job') {
const { meta } = frameDataCache[id];
return meta.deletedFrames;
Expand Down
1 change: 1 addition & 0 deletions cvat-core/src/server-response-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ export interface SerializedFramesMetaData {
deleted_frames: number[];
included_frames: number[];
frame_filter: string;
chunks_updated_date: string;
frames: {
width: number;
height: number;
Expand Down
Loading