Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queue: Allow setting the maximumBatchingWindow #160

Closed
jakejscott opened this issue Feb 1, 2022 · 3 comments
Closed

Queue: Allow setting the maximumBatchingWindow #160

jakejscott opened this issue Feb 1, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@jakejscott
Copy link
Contributor

jakejscott commented Feb 1, 2022

Start from the Use-case

Currently the maximumBatchingWindow is hardcoded to 60 seconds (when using standard queue). We would like to be able to configure this. There's even a TODO in the code for adding this as a setting :)

this.configuration.worker.events = [
            // Subscribe the worker to the SQS queue
            {
                sqs: {
                    arn: this.queue.queueArn,
                    batchSize: batchSize,
                    // TODO add setting
                    maximumBatchingWindow: this.queue.fifo ? undefined : 60,
                    functionResponseType: "ReportBatchItemFailures",
                },
            },
        ];

Example Config

constructs:
    my-queue:
        type: queue
        batchSize: 10
        maxBatchingWindow: 5 # <-- add this
        worker:
            handler: src/report-generator.handler

Implementation Idea

NOTE: The maxBatchingWindow setting is in seconds

@jakejscott jakejscott added the enhancement New feature or request label Feb 1, 2022
@jakejscott
Copy link
Contributor Author

jakejscott commented Feb 1, 2022

Also in the docs it says:

We recommend setting your queue visibility timeout to six times your function timeout, plus the value of MaximumBatchingWindowInSeconds

Currently I think the visibilityTimeout calculation is not taking into account the 60s default for maxBatchWindow when batchSize > 1

@jakejscott
Copy link
Contributor Author

Also I'm wondering if the default for maxBatchWindow should be 0 rather than 60 as it might catch people out?

@mnapoli
Copy link
Member

mnapoli commented Mar 18, 2022

Closed by #161

@mnapoli mnapoli closed this as completed Mar 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants