Croway opened a new pull request, #29271: URL: https://github.com/apache/camel/pull/29271
## What was wrong [CAMEL-26649](https://issues.apache.org/jira/browse/CAMEL-26649): by default the `aws2-sqs` consumer asked SQS for one message per poll. - `maxMessagesPerPoll` had no default (0), and the consumer turns 0 or a negative value into `maxNumberOfMessages(1)` (`Math.max(1, …)`, since CAMEL-21031). Before CAMEL-21031 the parameter was not sent at all, and SQS's own default is 1, so the effective default has always been 1. - With the defaults (`delay=500`, `greedy=false`), one consumer thread therefore routed about 2 messages per second, with one `ReceiveMessage` and one `DeleteMessage` per message. - The option was documented as "Is default unlimited, but use 0 or negative number to disable it as unlimited". That is the generic batch-consumer wording, and it is wrong for SQS. ## What changed - `Sqs2Endpoint`: `maxMessagesPerPoll` now defaults to 10, the most messages SQS returns to one receive request. - The option description now says what the consumer does: - a value above 10 is split into receive requests of at most 10 messages (CAMEL-21031); - 0 or a negative value receives one message per poll; - the messages of a poll wait their turn while their visibility timeout runs. - The consumer code is unchanged: values above 10, 0 and negative values behave as before. - Regenerated: the component JSON, the catalog JSON, the catalog copy of the doc, and the endpoint DSL. The component DSL has no change, as the option is an endpoint option. ## Tests - New `SqsConsumerMaxMessagesPerPollTest`. It uses `AmazonSQSClientMock` and a real route, with one poll per test: - default: one `ReceiveMessage` with `MaxNumberOfMessages=10`, and a batch of 10 exchanges; - `maxMessagesPerPoll` = 1, 0 or -1: one `ReceiveMessage` for 1 message, and a batch of 1. - New `SqsComponentConfigurationTest.createEndpointWithDefaultMaxMessagesPerPoll`. - The earlier reproducer used the module's mock with 30 messages queued: - on main, 5 messages took 5 receives of 1 message and 2.1 s; - with this change, the 30 messages take 3 receives of 10 and about 1 s. - On main, both new default tests fail: `CamelBatchSize ... Expected: <10> but was: <1>`, and `expected: <10> but was: <0>`. With the fix, they pass. - `camel-aws2-sqs` unit tests: 1592 tests, 0 failures. This includes the SQS DST worlds, 1500 seeds. - `camel-aws2-sqs` ITs on the Floci container: 17 tests, 0 failures. Several of these consumers use the default, including the FIFO batch IT. - No existing test asserted the old default. `Sqs2ConsumerTest` always sets `maxMessagesPerPoll`, and the other tests that use the default queue a single message. ## Docs - Component doc, Batch Consumer section. It now describes: - the default of 10 and how values above 10 are split; - long polling with `waitTimeSeconds`: without it, SQS often returns fewer messages than asked for; - the visibility timeout of the messages that wait in a batch: use `extendMessageVisibility` with `visibilityTimeout`, or a longer timeout; - FIFO queues: within a poll, the next messages of a group are still routed and deleted after one of them fails. - Upgrade guide 4.24, new entry "camel-aws2-sqs - the consumer receives up to 10 messages per poll". It covers: - the new default; - the risk for slow routes without `extendMessageVisibility`, where messages become visible again and are delivered twice; - the FIFO note; - `maxMessagesPerPoll=1` to keep the previous behaviour. ## Not in this PR (follow-ups) - **Batched deletes** (part 2 of the JIRA: an opt-in `DeleteMessageBatch` on the sync `SqsClient`). I left it out to keep this change small and easy to review. Deferring the delete past the end of the exchange needs several design choices, and it deserves its own review: - the visibility extender must keep a completed message until its delete is sent; - it must be decided what happens to exchanges that complete after the poll has ended (asynchronous routing), or while the route is suspended; - failed entries must be reported without the exchange, which may already be released to the pool; - the DST fake needs `DeleteMessageBatch` to check the timing. - **`aws-sqs-source` Kamelet** (camel-kamelets): it declares `maxMessagesPerPoll` with `default: 1` and `maximum: 10`, and always passes it. Kamelet users therefore keep one message per poll. Its `maximum: 10` is also out of date since CAMEL-21031. ## Backport Not a backport candidate for camel-4.18.x or camel-4.22.x, because it changes a default. On those versions, users can set `maxMessagesPerPoll=10` themselves. _Claude Code on behalf of Federico Mariani_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
