diff --git a/src/__tests__/__snapshots__/construct-hub.test.ts.snap b/src/__tests__/__snapshots__/construct-hub.test.ts.snap index 2dd5303ba..8667fe9e3 100644 --- a/src/__tests__/__snapshots__/construct-hub.test.ts.snap +++ b/src/__tests__/__snapshots__/construct-hub.test.ts.snap @@ -11682,6 +11682,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", @@ -26968,6 +26969,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", @@ -41818,6 +41820,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", @@ -56812,6 +56815,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", @@ -71815,6 +71819,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", diff --git a/src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap b/src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap index 930aad7ae..bed94ad3e 100644 --- a/src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap +++ b/src/__tests__/devapp/__snapshots__/snapshot.test.ts.snap @@ -12951,6 +12951,7 @@ Runbook: https://github.com/cdklabs/construct-hub/blob/main/docs/operator-runboo "Properties": { "KmsMasterKeyId": "alias/aws/sqs", "MessageRetentionPeriod": 1209600, + "VisibilityTimeout": 1800, }, "Type": "AWS::SQS::Queue", "UpdateReplacePolicy": "Delete", diff --git a/src/__tests__/package-sources/npmjs.test.ts b/src/__tests__/package-sources/npmjs.test.ts index c6d948b01..49f6a0aea 100644 --- a/src/__tests__/package-sources/npmjs.test.ts +++ b/src/__tests__/package-sources/npmjs.test.ts @@ -204,6 +204,34 @@ test('retries laggy packuments through a queue that moves give-ups to a dead-let }); }); +test('every queue the packument processor reads hides messages for longer than the function runs', () => { + const template = synth(new NpmJs()); + + const [processorId, processor] = Object.entries( + template.findResources('AWS::Lambda::Function', { + Properties: { + Description: Match.stringLikeRegexp( + 'Processes packages from the npm registry' + ), + }, + }) + )[0] as [string, any]; + const queues = template.findResources('AWS::SQS::Queue'); + const mappings = Object.values( + template.findResources('AWS::Lambda::EventSourceMapping', { + Properties: { FunctionName: { Ref: processorId } }, + }) + ) as any[]; + + expect(mappings).toHaveLength(2); + for (const mapping of mappings) { + const queueId = mapping.Properties.EventSourceArn['Fn::GetAtt'][0]; + // Unset means the SQS default of 30 seconds. + const visibility = queues[queueId].Properties.VisibilityTimeout ?? 30; + expect(visibility).toBeGreaterThanOrEqual(processor.Properties.Timeout); + } +}); + test('the billing mode of the known versions table is configurable', () => { const template = synth( new NpmJs({ diff --git a/src/package-sources/npmjs/packument-queue-processor.ts b/src/package-sources/npmjs/packument-queue-processor.ts index def1da211..52b21d604 100644 --- a/src/package-sources/npmjs/packument-queue-processor.ts +++ b/src/package-sources/npmjs/packument-queue-processor.ts @@ -14,6 +14,17 @@ import { PackumentProcessor } from './packument-processor'; import { IDenyList } from '../../backend/deny-list/api'; import { ILicenseList } from '../../backend/license-list/api'; +/** The timeout of the packument processor function. */ +const FUNCTION_TIMEOUT = Duration.minutes(5); + +/** + * The visibility timeout of both queues, which the function reads. Lambda + * requires at least the function timeout; AWS recommends 6x. + */ +const QUEUE_VISIBILITY_TIMEOUT = Duration.minutes( + 6 * FUNCTION_TIMEOUT.toMinutes() +); + export interface PackumentQueueProcessorProps { /** The table recording the package versions that were processed. */ readonly knownVersions: ITableV2; @@ -54,13 +65,14 @@ export class PackumentQueueProcessor extends Construct { this.deadLetterQueue = new Queue(this, 'DLQ', { encryption: QueueEncryption.KMS_MANAGED, retentionPeriod: Duration.days(14), + visibilityTimeout: QUEUE_VISIBILITY_TIMEOUT, }); this.queue = new Queue(this, 'Queue', { encryption: QueueEncryption.KMS_MANAGED, retentionPeriod: Duration.days(14), // Only used when the processor cannot schedule the next attempt itself, - // for example on a timeout. AWS recommends 6x the function timeout. - visibilityTimeout: Duration.minutes(30), + // for example on a timeout. + visibilityTimeout: QUEUE_VISIBILITY_TIMEOUT, deadLetterQueue: { queue: this.deadLetterQueue, maxReceiveCount: PACKUMENT_MAX_ATTEMPTS, @@ -77,7 +89,7 @@ export class PackumentQueueProcessor extends Construct { [ENV_PACKUMENT_DEAD_LETTER_QUEUE_ARN]: this.deadLetterQueue.queueArn, }, memorySize: 2_048, - timeout: Duration.minutes(5), + timeout: FUNCTION_TIMEOUT, tracing: Tracing.ACTIVE, }); this.function.addEventSource(