CAMEL-24196: camel-aws2-sqs - Fix QueueDoesNotExist swallowed, thread pool leak, and batch chunking#24873
Conversation
… pool leak, and batch chunking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Claude Code on behalf of gnodet
CAMEL-24196: camel-aws2-sqs — three well-scoped fixes — LGTM ✅
1. QueueDoesNotExistException silently swallowed ✅
Good catch. When maxMessagesPerPoll > 10, the parallel polling sends N requests but the existing errorCount == numberOfRequestsPerPoll check fails when only a subset of threads register the error (e.g., errorCount == 1 but N > 1). The new isQueueMissing() && hasErrors() check short-circuits before that, correctly surfacing the queue-missing error regardless of how many parallel requests were sent.
2. Thread pool leak on stop/start ✅
Proper lifecycle fix. doStop() mirrors doShutdown() with two correct differences:
doStop()callssuper.doStop()first (top-down),doShutdown()callssuper.doShutdown()last (bottom-up) — standard Camel lifecycle patterndoStop()uses gracefulshutdown(),doShutdown()usesshutdownNow()— correct escalation semantics
Since doStop() nulls all references, doShutdown() safely skips (idempotent).
3. sendBatchMessage 10-entry chunking ✅
Fixes the hard SQS limit violation (TooManyEntriesInBatchRequestException on >10 entries). Clean extraction into sendBatchEntries(). The totalFailed/totalSuccessful aggregation across chunks is correct.
Note: message.setBody(lastResult) only captures the final chunk's SendMessageBatchResponse. For ≤10 entries (the only case that worked before), behavior is identical. For >10 entries (previously broken), callers who need per-entry results would need to rely on the aggregate headers rather than the body. This is a reasonable tradeoff for a fix that makes >10 entries work at all — just worth documenting if users ask.
Also nice cleanup: Iterable → Iterable<?> removes the raw type warning.
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 10 tested, 29 compile-only — current: 10 all testedMaveniverse Scalpel detected 39 affected modules (current approach: 10).
|
2 participants
Summary
Claude Code on behalf of davsclaus
Fixes three bugs in camel-aws2-sqs:
QueueDoesNotExistException silently swallowed (
Sqs2Consumer): WhenmaxMessagesPerPoll > 10, parallel polling sends N requests. Only the first thread to catchQueueDoesNotExistExceptionfires an error (errorCount=1), but the error propagation check requireserrorCount == N, which never matches. Fix: added explicit check forisQueueMissing() && hasErrors()before the all-errors check.Thread pool leak on stop/start (
Sqs2Consumer):doStart()unconditionally creates a newPollingTaskwith a new thread pool, but cleanup only happened indoShutdown(), notdoStop(). Each route stop/start cycle leaked the previous thread pool. Fix: addeddoStop()override that cleans up the PollingTask, TimeoutExtender, and scheduled executor.sendBatchMessage missing 10-entry chunking (
Sqs2Producer): All entries were collected into a singlesendMessageBatch()call. SQS hard-limits batches to 10 entries; 11+ throwsTooManyEntriesInBatchRequestException. Fix: extractedsendBatchEntries()helper that chunks entries into groups of 10.Test plan
mvn install -DskipTests)doStop()cleans up all resources thatdoStart()creates🤖 Generated with Claude Code