Skip to content

Commit f6c11e5

Browse files
committed
PM-5231: Allow validation uploads to queue by phase
What was broken Uploading a second Marathon Match validation submission for another phase while a prior validation task was still active returned HTTP 500 from the Work app flow. Root cause The shared ECS launcher always attempted to stop older active tasks for the same challenge and member before launching a new task. PM-5231 validation uploads reuse the manager/copilot member id for each phase, so a phase-by-phase validation upload tried to stop the previous phase's active validation task before queueing the next one. What was changed Added a launch option to skip superseded member task cancellation. The validation-upload endpoint now uses that option so Example, Provisional, and System validation runs can be queued independently while existing rerun/submission scoring keeps the original cancellation behavior. Any added/updated tests Added ECS service coverage for disabled superseded-task stopping and updated validation-upload service coverage to assert the non-cancelling launch mode.
1 parent c131fd9 commit f6c11e5

4 files changed

Lines changed: 76 additions & 6 deletions

File tree

src/api/marathon-match-config/marathon-match-config.service.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,7 @@ describe('MarathonMatchConfigService', () => {
812812
undefined,
813813
{
814814
memberId: '40051399',
815+
stopSupersededMemberTasks: false,
815816
},
816817
);
817818
});

src/api/marathon-match-config/marathon-match-config.service.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,6 +1368,7 @@ export class MarathonMatchConfigService {
13681368
undefined,
13691369
{
13701370
memberId,
1371+
stopSupersededMemberTasks: false,
13711372
},
13721373
);
13731374

src/shared/modules/global/ecs.service.spec.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,69 @@ describe('EcsService', () => {
264264
);
265265
});
266266

267+
it('keeps older active tasks when superseded task stopping is disabled', async () => {
268+
const { service, send } = createService();
269+
send.mockImplementation((command) => {
270+
if (command instanceof ListTasksCommand) {
271+
return Promise.resolve(
272+
command.input.desiredStatus === 'RUNNING'
273+
? {
274+
taskArns: [
275+
'arn:aws:ecs:us-east-1:123456789012:task/cluster/old-task',
276+
],
277+
}
278+
: { taskArns: [] },
279+
);
280+
}
281+
if (command instanceof DescribeTasksCommand) {
282+
return Promise.resolve({
283+
tasks: [
284+
activeTask({
285+
taskArn:
286+
'arn:aws:ecs:us-east-1:123456789012:task/cluster/old-task',
287+
submissionId: 'old-submission',
288+
}),
289+
],
290+
});
291+
}
292+
if (command instanceof RunTaskCommand) {
293+
return Promise.resolve({
294+
tasks: [
295+
{
296+
taskArn:
297+
'arn:aws:ecs:us-east-1:123456789012:task/cluster/new-task',
298+
},
299+
],
300+
});
301+
}
302+
if (command instanceof DescribeTaskDefinitionCommand) {
303+
return Promise.resolve({ taskDefinition: {} });
304+
}
305+
306+
throw new Error(`Unexpected command ${command.constructor.name}`);
307+
});
308+
309+
await service.launchScorerTask(
310+
'challenge-1',
311+
'new-submission',
312+
baseTaskConfig,
313+
basePhaseConfig,
314+
undefined,
315+
{
316+
memberId: 'member-1',
317+
stopSupersededMemberTasks: false,
318+
},
319+
);
320+
321+
const sentCommands = send.mock.calls.map((call) => call[0] as unknown);
322+
expect(
323+
sentCommands.some((command) => command instanceof StopTaskCommand),
324+
).toBe(false);
325+
expect(
326+
sentCommands.some((command) => command instanceof RunTaskCommand),
327+
).toBe(true);
328+
});
329+
267330
it('blocks new launches when the global scorer task cap is reached', async () => {
268331
process.env.ECS_SCORER_MAX_CONCURRENT_TASKS = '1';
269332
const { service, m2mService, send } = createService();

src/shared/modules/global/ecs.service.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export interface MarathonMatchScorerTaskLaunchResult {
3939

4040
export interface MarathonMatchScorerTaskLaunchOptions {
4141
memberId?: string;
42+
stopSupersededMemberTasks?: boolean;
4243
}
4344

4445
interface ActiveScorerTask {
@@ -89,7 +90,7 @@ export class EcsService {
8990
* @param mmConfig Task definition name and version from the marathonMatchConfig record.
9091
* @param scoringPhase Active phase settings used by the runner for flags and seed range.
9192
* @param reviewId Optional review-api review id that should be marked completed after callback processing.
92-
* @param launchOptions Optional submission owner metadata used for per-member in-flight task cancellation.
93+
* @param launchOptions Optional submission owner metadata and cancellation controls for in-flight member tasks.
9394
* @returns ECS task launch details with task/log mapping metadata.
9495
* Required env vars: ECS_CLUSTER, ECS_CONTAINER_NAME, ECS_SUBNETS, ECS_SECURITY_GROUPS,
9596
* AWS_REGION, MARATHON_MATCH_API_URL, REVIEW_TYPE_ID, and Auth0 M2M settings
@@ -117,6 +118,8 @@ export class EcsService {
117118
const taskDefinitionVersion = mmConfig.taskDefinitionVersion?.trim();
118119
const testPhase = this.mapConfigTypeToTestPhase(scoringPhase.configType);
119120
const memberId = launchOptions.memberId?.trim();
121+
const shouldStopSupersededMemberTasks =
122+
launchOptions.stopSupersededMemberTasks !== false;
120123

121124
if (!taskDefinitionName) {
122125
throw new Error('Missing required task definition name in mmConfig.');
@@ -136,11 +139,13 @@ export class EcsService {
136139
containerName,
137140
);
138141
const launchableActiveTasks = activeTasks
139-
? await this.stopSupersededMemberScorerTasks(cluster, activeTasks, {
140-
challengeId,
141-
submissionId,
142-
memberId,
143-
})
142+
? shouldStopSupersededMemberTasks
143+
? await this.stopSupersededMemberScorerTasks(cluster, activeTasks, {
144+
challengeId,
145+
submissionId,
146+
memberId,
147+
})
148+
: activeTasks
144149
: null;
145150
let maxConcurrentScorerTasks: number | null = null;
146151

0 commit comments

Comments
 (0)