Skip to content

Commit 11c1578

Browse files
Copilotnpalm
andcommitted
refactor: improve performance and reduce code duplication
Address code review feedback: - Use Set for invalidMessageIds to improve lookup performance from O(n) to O(1) - Extract duplicate retry message publishing logic into helper function - Maintain both invalidMessages array (for return value) and Set (for fast lookups) This improves performance when processing large message batches and makes the code more maintainable by eliminating duplication. Co-authored-by: npalm <11609620+npalm@users.noreply.github.com>
1 parent 90874d7 commit 11c1578

File tree

1 file changed

+20
-12
lines changed
  • lambdas/functions/control-plane/src/scale-runners

1 file changed

+20
-12
lines changed

lambdas/functions/control-plane/src/scale-runners/scale-up.ts

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,17 @@ function generateRunnerServiceConfig(githubRunnerConfig: CreateGitHubRunnerConfi
9090
return config;
9191
}
9292

93+
async function publishRetryMessagesForValid(
94+
messages: ActionRequestMessageSQS[],
95+
invalidMessageIds: Set<string>,
96+
): Promise<void> {
97+
for (const message of messages) {
98+
if (!invalidMessageIds.has(message.messageId)) {
99+
await publishRetryMessage(message as ActionRequestMessageRetry);
100+
}
101+
}
102+
}
103+
93104
async function getGithubRunnerRegistrationToken(githubRunnerConfig: CreateGitHubRunnerConfig, ghClient: Octokit) {
94105
const registrationToken =
95106
githubRunnerConfig.runnerType === 'Org'
@@ -276,6 +287,7 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
276287

277288
const validMessages = new Map<string, MessagesWithClient>();
278289
const invalidMessages: string[] = [];
290+
const invalidMessageIds = new Set<string>();
279291
for (const payload of payloads) {
280292
const { eventType, messageId, repositoryName, repositoryOwner } = payload;
281293
if (ephemeralEnabled && eventType !== 'workflow_job') {
@@ -394,17 +406,15 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
394406
if (ephemeralEnabled) {
395407
// This removes `missingInstanceCount` items from the start of the array
396408
// so that, if we retry more messages later, we pick fresh ones.
397-
invalidMessages.push(...messages.splice(0, missingInstanceCount).map(({ messageId }) => messageId));
409+
const removedMessages = messages.splice(0, missingInstanceCount);
410+
invalidMessages.push(...removedMessages.map(({ messageId }) => messageId));
411+
removedMessages.forEach(({ messageId }) => invalidMessageIds.add(messageId));
398412
}
399413

400414
// No runners will be created, so skip calling the EC2 API.
401415
if (newRunners <= 0) {
402416
// Publish retry messages for all remaining messages that are not marked as invalid
403-
for (const message of validMessagesForRetry) {
404-
if (!invalidMessages.includes(message.messageId)) {
405-
await publishRetryMessage(message as ActionRequestMessageRetry);
406-
}
407-
}
417+
await publishRetryMessagesForValid(validMessagesForRetry, invalidMessageIds);
408418
continue;
409419
}
410420
}
@@ -456,15 +466,13 @@ export async function scaleUp(payloads: ActionRequestMessageSQS[]): Promise<stri
456466
failedInstanceCount,
457467
});
458468

459-
invalidMessages.push(...messages.slice(0, failedInstanceCount).map(({ messageId }) => messageId));
469+
const failedMessages = messages.slice(0, failedInstanceCount);
470+
invalidMessages.push(...failedMessages.map(({ messageId }) => messageId));
471+
failedMessages.forEach(({ messageId }) => invalidMessageIds.add(messageId));
460472
}
461473

462474
// Publish retry messages for all messages that are not marked as invalid
463-
for (const message of validMessagesForRetry) {
464-
if (!invalidMessages.includes(message.messageId)) {
465-
await publishRetryMessage(message as ActionRequestMessageRetry);
466-
}
467-
}
475+
await publishRetryMessagesForValid(validMessagesForRetry, invalidMessageIds);
468476
}
469477

470478
return invalidMessages;

0 commit comments

Comments
 (0)