Skip to content

Commit

Permalink
Merge pull request #2618 from HHS/TTAHUB-3682/stalled-job
Browse files Browse the repository at this point in the history 
Update maintenance to support passing job timeout
  • Loading branch information
@GarrettEHill
GarrettEHill authored Jan 31, 2025
2 parents 87a901d + 30a7abd commit 72bdad3
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 111 deletions.
22 changes: 11 additions & 11 deletions src/lib/maintenance/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,23 +118,21 @@ const processMaintenanceQueue = () => {

/**
* Adds a maintenance job to the queue if a processor is defined for the given type.
*
* @param {string} category - The type of maintenance job to add to the queue.
* @param {*} [data=null] - Optional data to include with the maintenance job.
*/
const enqueueMaintenanceJob = async (
const enqueueMaintenanceJob = async ({
category,
data = null,
requiredLaunchScript = null,
requiresLock = false,
holdLock = false,
) => {
jobSettings = {},
}) => {
const action = async () => {
// Check if there is a processor defined for the given type
if (category in maintenanceQueueProcessors) {
try {
// Add the job to the maintenance queue
maintenanceQueue.add(category, { ...data, ...referenceData() });
maintenanceQueue.add(category, { ...data, ...referenceData() }, jobSettings);
} catch (err) {
// Log any errors that occur when adding the job to the queue
auditLogger.error(err);
Expand Down Expand Up @@ -537,13 +535,15 @@ addCronJob(
// The function to execute takes in the category, type, timezone, and schedule parameters
(category, type, timezone, schedule) => new CronJob(
schedule, // The schedule parameter specifies when the job should run
() => enqueueMaintenanceJob(
MAINTENANCE_CATEGORY.MAINTENANCE, // constant representing the category of maintenance
{
type: MAINTENANCE_TYPE.CLEAR_MAINTENANCE_LOGS, // shorthand property notation for type: type
() => enqueueMaintenanceJob({
// constant representing the category of maintenance
category: MAINTENANCE_CATEGORY.MAINTENANCE,
data: {
// shorthand property notation for type: type
type: MAINTENANCE_TYPE.CLEAR_MAINTENANCE_LOGS,
dateOffSet: 90, // otherwise, merge the provided data object
},
),
}),
null,
true,
timezone, // The timezone parameter specifies the timezone in which the job should run
Expand Down
6 changes: 3 additions & 3 deletions src/lib/maintenance/common.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,13 +155,13 @@ describe('Maintenance Queue', () => {
const processor = jest.fn();
addQueueProcessor(category, processor);
maintenanceQueue.add = jest.fn();
enqueueMaintenanceJob(category, data);
expect(maintenanceQueue.add).toHaveBeenCalledWith(category, data);
enqueueMaintenanceJob({ category, data });
expect(maintenanceQueue.add).toHaveBeenCalledWith(category, data, {});
});

it('should log an error if no processor is defined for the given type', () => {
const category = 'non-existent-category';
enqueueMaintenanceJob(category);
enqueueMaintenanceJob({ category });
expect(auditLogger.error).toHaveBeenCalledWith(new Error(`Maintenance Queue Error: no processor defined for ${category}`));
});
});
Expand Down
8 changes: 4 additions & 4 deletions src/lib/maintenance/db.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,16 +291,16 @@ const enqueueDBMaintenanceJob = async (
type,
data,
percent = null, // optional parameter with default value of null
) => enqueueMaintenanceJob(
MAINTENANCE_CATEGORY.DB, // constant representing the category of maintenance
{
) => enqueueMaintenanceJob({
category: MAINTENANCE_CATEGORY.DB, // constant representing the category of maintenance
data: {
type, // shorthand property notation for type: type
...(!data // spread operator used to merge properties of two objects
// if data is not provided, call nextBlock function and merge its result
? await nextBlock(type, percent)
: data), // otherwise, merge the provided data object
},
);
});

// This code adds a queue processor for database maintenance tasks.
// The MAINTENANCE_CATEGORY.DB is used to identify the category of maintenance task.
Expand Down
8 changes: 4 additions & 4 deletions src/lib/maintenance/db.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,13 +424,13 @@ describe('maintenance', () => {
await enqueueDBMaintenanceJob(type, data);

expect(enqueueSpy).toHaveBeenCalledTimes(1);
expect(enqueueSpy).toHaveBeenCalledWith(
MAINTENANCE_CATEGORY.DB,
expect.objectContaining({
expect(enqueueSpy).toHaveBeenCalledWith({
category: MAINTENANCE_CATEGORY.DB,
data: expect.objectContaining({
type,
someKey: 'someValue',
}),
);
});
});
});
});
124 changes: 72 additions & 52 deletions src/lib/maintenance/import.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
importDownload,
importProcess,
importMaintenance,
enqueue,
} from './import';
import { MAINTENANCE_TYPE, MAINTENANCE_CATEGORY } from '../../constants';
import {
Expand Down Expand Up @@ -58,26 +57,28 @@ describe('import', () => {
it('should enqueue a maintenance job with the correct category and type', async () => {
const type = MAINTENANCE_TYPE.IMPORT_SCHEDULE;
const id = 123;
await enqueueImportMaintenanceJob(type, id);
expect(enqueueMaintenanceJob).toHaveBeenCalledWith(
MAINTENANCE_CATEGORY.IMPORT,
{ type, id },
undefined,
false,
false,
);
await enqueueImportMaintenanceJob({ type, id });
expect(enqueueMaintenanceJob).toHaveBeenCalledWith({
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: {},
});
});

it('should be able to enqueue a job without an id', () => {
const type = MAINTENANCE_TYPE.IMPORT_SCHEDULE;
enqueueImportMaintenanceJob(type);
expect(enqueueMaintenanceJob).toHaveBeenCalledWith(
MAINTENANCE_CATEGORY.IMPORT,
{ type, id: undefined },
undefined,
false,
false,
);
enqueueImportMaintenanceJob({ type });
expect(enqueueMaintenanceJob).toHaveBeenCalledWith({
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type, id: undefined },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: {},
});
});
});

Expand Down Expand Up @@ -126,11 +127,14 @@ describe('import', () => {
expect(enqueueMaintenanceJob)
.toHaveBeenNthCalledWith(
index + 1,
MAINTENANCE_CATEGORY.IMPORT,
{ type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
undefined,
false,
false,
{
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 6000 },
},
);
}

Expand Down Expand Up @@ -165,11 +169,14 @@ describe('import', () => {
expect(enqueueMaintenanceJob)
.toHaveBeenNthCalledWith(
index + 1,
MAINTENANCE_CATEGORY.IMPORT,
{ type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
undefined,
false,
false,
{
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 6000 },
},
);
}

Expand Down Expand Up @@ -264,20 +271,26 @@ describe('import', () => {
expect(enqueueMaintenanceJob)
.toHaveBeenNthCalledWith(
1,
MAINTENANCE_CATEGORY.IMPORT,
{ type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
undefined,
false,
false,
{
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type: MAINTENANCE_TYPE.IMPORT_DOWNLOAD, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 6000 },
},
);
expect(enqueueMaintenanceJob)
.toHaveBeenNthCalledWith(
2,
MAINTENANCE_CATEGORY.IMPORT,
{ type: MAINTENANCE_TYPE.IMPORT_PROCESS, id },
undefined,
false,
false,
{
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type: MAINTENANCE_TYPE.IMPORT_PROCESS, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 4500 },
},
);
expect(results?.isSuccessful).toBe(true);
});
Expand All @@ -303,11 +316,14 @@ describe('import', () => {
expect(enqueueMaintenanceJob)
.toHaveBeenNthCalledWith(
2,
MAINTENANCE_CATEGORY.IMPORT,
{ type: MAINTENANCE_TYPE.IMPORT_PROCESS, id },
undefined,
false,
false,
{
category: MAINTENANCE_CATEGORY.IMPORT,
data: { type: MAINTENANCE_TYPE.IMPORT_PROCESS, id },
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 4500 },
},
);
});

Expand Down Expand Up @@ -398,14 +414,17 @@ describe('import', () => {
await anonymousFunction();

expect(enqueueMaintenanceJob).toHaveBeenCalledWith(
MAINTENANCE_CATEGORY.IMPORT,
{
type: MAINTENANCE_TYPE.IMPORT_PROCESS,
id,
category: MAINTENANCE_CATEGORY.IMPORT,
data: {
type: MAINTENANCE_TYPE.IMPORT_PROCESS,
id,
},
requiredLaunchScript: undefined,
requiresLock: false,
holdLock: false,
jobSettings: { timeout: 4500 },
},
undefined,
false,
false,
);
});

Expand All @@ -424,13 +443,14 @@ describe('import', () => {
const anonymousFunction = maintenanceCommand.mock.calls[0][0];
await anonymousFunction();

expect(enqueueMaintenanceJob).not.toHaveBeenCalledWith(
MAINTENANCE_CATEGORY.IMPORT,
{
expect(enqueueMaintenanceJob).not.toHaveBeenCalledWith({
category: MAINTENANCE_CATEGORY.IMPORT,
data: {
type: MAINTENANCE_TYPE.IMPORT_PROCESS,
id,
},
);
jobSettings: { timeout: 4500 },
});
});

it('should return an object with isSuccessful false when processing fails', async () => {
Expand Down
Loading

0 comments on commit 72bdad3

Please sign in to comment.