Skip to content

Commit

Permalink
DPO3DPKRT-843/invalid Scenes Crash Batch Download (#620)
Browse files Browse the repository at this point in the history 
(fix) delay batch submissions to avoid tight loop
(fix) invalid scenes crashing batch download process
(fix) error responses not accurate or clear when only some jobs failed
(fix) cleanup logs and comments
  • Loading branch information
@EMaslowskiQ
EMaslowskiQ authored Aug 14, 2024
1 parent 1fc1592 commit e6bac3a
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 108 deletions.
5 changes: 3 additions & 2 deletions client/src/api/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ export default class API {
const body = JSON.stringify({ statusOnly, rePublish, idSystemObject });
let uri: string = API_ROUTES.GEN_DOWNLOADS;
console.log('[PACKRAT:DEBUG] body: ',body);
console.trace('API.generateDownloads');

let options;
if(statusOnly) {
Expand Down Expand Up @@ -82,8 +81,10 @@ export default class API {
return fetch(`${serverEndpoint}/${route}`, defaultOptions)
.then(response => {
// Check if the response returned a successful status code
if (!response.ok)
if (!response.ok) {
console.log('response: ',response);
return { success: false, message: response.statusText };
}
return response.json(); // Assuming the server responds with JSON
})
.catch(error => {
Expand Down
44 changes: 36 additions & 8 deletions client/src/pages/Admin/components/AdminToolsView.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -636,15 +636,43 @@ const AdminToolsBatchGeneration = (): React.ReactElement => {
const response: RequestResponse = await API.generateDownloads(sceneIDs,false,republishScenes);
if(response.success === false) {

// if the job is running then handle differently
if(response.message && response.message.includes('already running')) {
console.log(`[Packrat:WARN] cannot do ${BatchOperations[operation]}. (${response.message})`);
toast.warn(`Not running ${BatchOperations[operation]}. Job already running. Please wait for it to finish.`);
} else {
console.log(`[Packrat:ERROR] cannot run ${BatchOperations[operation]}. (${response.message})`);
toast.error(`Cannot ${BatchOperations[operation]}. Check the report.`);
// make sure we have data and responses
if(!response.data || !Array.isArray(response.data)) {
console.log(`[Packrat:ERROR] cannot run ${BatchOperations[operation]}. invalid response data.`,response);
toast.error(`${BatchOperations[operation]} failed. Got unexpected data from server.`);
return;
}
return;

// get our unique error messages
const uniqueMessages = Array.from(
new Set(
response.data
.filter(response => !response.success && response.message) // Ensure there is a message
.map(response => `${response.id}: ${response.message}`) // Extract the messages
)
);
const toastErrorMsg: string = (uniqueMessages.length>1) ? 'Check the console.' : uniqueMessages[0];

// see if we have nuance to the response (i.e. some failed/some passed)
const allFailed: boolean = response.data.every( response => response.succcess===false );
if(allFailed===true) {
const errorMsg: string = (response.data.length>1)
? `All ${response.data.length} scenes failed during ${BatchOperations[operation]} run.`
: `${BatchOperations[operation]} cannot run. ${uniqueMessages[0]}`;

console.log(`[Packrat:ERROR] ${errorMsg}`,response.data);
toast.error(`${BatchOperations[operation]} failed. (${toastErrorMsg})`);
return;
}

// only some failed so we need to handle this
const failedCount: number = response.data.filter(response => !response.success).length;
console.log(`[Packrat:ERROR] ${response.data.length}/${selectedList.length} scenes failed. (${uniqueMessages.join(' |')})`,response.data);
toast.warn(`${BatchOperations[operation]} had issues. ${failedCount} scenes failed. (${toastErrorMsg})`);

// we bail early so the selection is maintained on failure
// TODO: deselect those that were successful.
return false;
}

// clear selection on succcess
Expand Down
126 changes: 29 additions & 97 deletions server/http/routes/api/generateDownloads.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,16 +42,31 @@ const generateResponse = (success: boolean, message?: string | undefined, id?: n
state
};
};
const buildOpResponse = (message: string, responses: GenDownloadsResponse[]): OpResponse => {
// if empty send error response
if(responses.length===0)
return { success: false, message: 'no responses from operation' };
const buildOpResponse = (responses: GenDownloadsResponse[]): OpResponse => {
if (responses.length === 0) {
return { success: false, message: 'No responses from operation', data: [] };
}

// see how many were successfuly and extract error messages from those not successful
const successCount = responses.filter(response => response.success).length;
const messages = Array.from(
new Set(
responses
.filter(response => !response.success && response.message)
.map(response => response.message as string)
)
);

// build up our final message based on what was found
const message =
successCount === responses.length
? `All ${successCount} scenes submitted successfully`
: `${successCount}/${responses.length} scenes submitted successfully (errors: ${messages.join(', ')})`;

// cycle through all responses and see if we
return {
success: responses.every((r)=>r.success),
success: successCount === responses.length,
message,
data: [...responses],
data: responses,
};
};

Expand All @@ -77,10 +92,10 @@ const createOpForScene = async (idSystemObject: number, idUser: number): Promise

// create our workflow for generating downloads
const result: WorkflowCreateResult = await wfEngine.generateDownloads(scene.idScene, workflowParams);
// LOG.info(`API.generateDownloads post creation. (result: ${H.Helpers.JSONStringify(result)})`,LOG.LS.eDEBUG);
LOG.info(`API.generateDownloads post creation. (result: ${H.Helpers.JSONStringify(result)})`,LOG.LS.eDEBUG);

const isValid: boolean = result.data.isValid ?? false;
const isJobRunning: boolean = (result.data.activeJobs.length>0) ?? false;
const isJobRunning: boolean = (result.data.activeJobs?.length>0) ?? false;
const idWorkflow: number | undefined = (result.data.workflow?.idWorkflow) ?? undefined;
const idWorkflowReport: number | undefined = (result.data.workflowReport?.idWorkflowReport) ?? undefined;

Expand All @@ -90,6 +105,7 @@ const createOpForScene = async (idSystemObject: number, idUser: number): Promise
return generateResponse(false,result.message,idSystemObject,{ isValid, isJobRunning, idWorkflow, idWorkflowReport });
}

console.log('a');
return generateResponse(true,`Generating Downloads for: ${scene.Name}`,idSystemObject,{ isValid, isJobRunning, idWorkflow, idWorkflowReport });
};
const getOpStatusForScene = async (idSystemObject: number): Promise<GenDownloadsResponse> => {
Expand Down Expand Up @@ -241,79 +257,6 @@ const publishScene = async (response: GenDownloadsResponse, intendedState: COMMO
return;
};

// queue management
// enum OpStatus {
// UNDEFINED,
// PENDING,
// RUNNING,
// FINISHED,
// ERROR,
// RETRY
// }
// type OpsQueueItem = {
// status: OpStatus;
// idSystemObject: number;
// message?: string;
// response: Promise<GenDownloadsResponse> | null;
// };
// const processScenes = async (idSystemObjects: number[], idUser: number): Promise<GenDownloadsResponse[]> => {
// const maxActive: number = 3;

// // add our ids to the queue
// const queue: OpsQueueItem[] = idSystemObjects.map((id) => ({
// status: OpStatus.PENDING,
// idSystemObject: id,
// response: null,
// }));

// // hold our active items and any returned promises
// const active: Set<OpsQueueItem> = new Set();
// const responses: GenDownloadsResponse[] = [];

// // function to process the next item checking that we're not exceeding our
// // concurrency and checking status of all items
// const processNext = async () => {
// // if over our max active, just return (nothing to add)
// if (active.size >= maxActive) return;

// // find the next item based on the status. find is sequential so we get a FIFO behavior
// const nextItem: OpsQueueItem | undefined = queue.find(item => item.status === OpStatus.PENDING || item.status === OpStatus.RETRY);
// if (!nextItem) return;

// // update our status and add to list of active items
// nextItem.status = OpStatus.RUNNING;
// active.add(nextItem);

// try {
// // we create the op and wait for it to finish
// // TODO: returns FINISH before checking state of the workflow
// nextItem.response = createOpForScene(nextItem.idSystemObject, idUser);
// const response = await nextItem.response;
// responses.push(response);
// nextItem.status = OpStatus.FINISHED;
// } catch (error) {
// // set our error and push a failed attempt into our responses
// nextItem.status = OpStatus.ERROR;
// nextItem.message = error as string;
// responses.push(generateResponse(false,error as string,nextItem.idSystemObject));
// } finally {
// // remove from our active items and process a new one
// active.delete(nextItem);
// processNext();
// }
// };

// // cycle through all queue items constantly
// while (queue.some(item => item.status !== OpStatus.FINISHED && item.status !== OpStatus.ERROR)) {
// if (active.size < maxActive) {
// processNext();
// }
// await new Promise(resolve => setTimeout(resolve, 100)); // Small delay to prevent tight loop
// }

// return responses;
// };

export async function generateDownloads(req: Request, res: Response): Promise<void> {

// make sure we're authenticated (i.e. see if request has a 'user' object)
Expand Down Expand Up @@ -376,16 +319,13 @@ export async function generateDownloads(req: Request, res: Response): Promise<vo
}

// TEMP: limit IDs to a number that can be handled by Cook/Packrat
let messageSuffix: string | null = null;
const maxIDs: number = 10;
if(idSystemObjects.length>maxIDs) {
LOG.info('API.generateDownloads too many scenes submitted. limiting to 10',LOG.LS.eHTTP);
messageSuffix = `(Capped to ${maxIDs})`;
idSystemObjects.splice(10);
}

// cycle through IDs
let messagePrefix: string = '';
const responses: GenDownloadsResponse[] = [];
for(let i=0; i<idSystemObjects.length; i++) {
const idSystemObject: number = idSystemObjects[i];
Expand All @@ -394,17 +334,7 @@ export async function generateDownloads(req: Request, res: Response): Promise<vo
if(statusOnly===true) {
const result: GenDownloadsResponse = await getOpStatusForScene(idSystemObject);
responses.push(result);
messagePrefix = 'Getting Status for:';
} else {
/** TODO
* - push to queue
* - spin up queue worker (if not already)
* - if active list < threshold add new one to list
* - cycle through active items
* - if finished/error remove from list and store results (just return status obj)
* - if active list is empty then finish and return response
*/

// get our published state for the scene ebfore doing anything because generate downloads
// will create a new Scene version that is unpublished, affecting our ability to re-publish
const sceneSOV: DBAPI.SystemObjectVersion | null = await DBAPI.SystemObjectVersion.fetchLatestFromSystemObject(idSystemObject ?? -1);
Expand All @@ -417,7 +347,6 @@ export async function generateDownloads(req: Request, res: Response): Promise<vo
// create our operation and execute download generation
const result: GenDownloadsResponse = await createOpForScene(idSystemObject,LS.idUser);
responses.push(result);
messagePrefix = 'Generating Downloads for:';

// if we want to republish the scene then we fire off the promise that checks the status
// and when done re-publishes the scene (non-blocking)
Expand All @@ -429,8 +358,11 @@ export async function generateDownloads(req: Request, res: Response): Promise<vo
publishScene(result, currentPublishedState);
}
}

// stagger a second to avoid a tight request loop
H.Helpers.sleep(1000);
}

// create our combined response and return info to client
res.status(200).send(JSON.stringify(buildOpResponse(`${messagePrefix} ${responses.length} scenes ${messageSuffix ?? ''}`,responses)));
res.status(200).send(JSON.stringify(buildOpResponse(responses)));
}
2 changes: 1 addition & 1 deletion server/workflow/impl/Packrat/WorkflowEngine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export class WorkflowEngine implements WF.IWorkflowEngine {
// make sure we can run the recipe (valid scene, not running, etc)
if(scene.PosedAndQCd === false) {
LOG.error(`WorkflowEngine.generateDownloads failed. Scene is invalid. (idScene: ${scene.idScene})`,LOG.LS.eWF);
return { success: false, message: 'not posed or QC\'d', data: { isValid: false } };
return { success: false, message: 'not posed, licensed, or QC', data: { isValid: false } };
}
const isValid: boolean = true;
//#endregion
Expand Down

0 comments on commit e6bac3a

Please sign in to comment.