CAMEL-24086: camel-aws2-sqs - give each FIFO batch entry a distinct MessageDeduplicationId#24728
Conversation
…essageDeduplicationId When sending a batch to a FIFO queue, every entry is built from the same Exchange, so configureFifoAttributes derived the MessageDeduplicationId from that one Exchange via the configured strategy. The default ExchangeIdMessageDeduplicationIdStrategy returns exchange.getExchangeId(), identical for all entries, so SQS accepted the batch but silently dropped all but the first message within the deduplication window. The batch overload now appends the entry position to the strategy-provided id, giving each message a distinct, deterministic deduplication id (so a redelivered batch still deduplicates correctly). A null id (content-based deduplication via NullMessageDeduplicationIdStrategy) is left unset, and the group-id / dedup-id strategies are now null-guarded consistently with the single-message overload. Adds SqsProducerFifoBatchDeduplicationTest asserting the batch entries receive distinct deduplication ids. The single-message send path is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrea Cosentino <ancosen@gmail.com>
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
gnodet
left a comment
There was a problem hiding this comment.
Clean, well-scoped fix for FIFO batch deduplication. A few observations:
What the PR does well:
- Correctly identifies that
MessageDeduplicationIdmust be unique per batch entry — SQS silently drops entries with duplicate dedup IDs in FIFO queues - Appending
-<index>is deterministic and preserves the strategy-derived prefix, so the same batch re-sent produces the same per-entry IDs (idempotency preserved) - Adds
ObjectHelper.isNotEmpty()null guards on the batch path, aligning with the single-messageconfigureFifoAttributesoverload (line 227–238) which already had them - Handles the
nulldedup ID case (content-based deduplication) by leaving it unset — correct behavior - Covers both entry paths (
IterableandString-split) - Good test with distinct ID assertion via
HashSetsize check
Checklist:
- Tests: new
SqsProducerFifoBatchDeduplicationTestcovers the fix - Commit convention: JIRA-linked (
CAMEL-24086) - Backward compatibility: maintained — only batch FIFO behavior changes (and was broken before)
- No public API changes
- Mock class already supports
getSendMessageBatchRequests()— no test infra changes needed
Reviewed with oss-review on behalf of gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
davsclaus
left a comment
There was a problem hiding this comment.
Clean, well-scoped fix for the FIFO batch deduplication-id collision. The three changes are correct and minimal:
- Index-suffixed dedup ID (
<strategy-id>-<index>) gives each batch entry a distinct, deterministic id — redelivered batches still deduplicate correctly - Null id (content-based dedup via
NullMessageDeduplicationIdStrategy) is correctly left unset - Null guards on the batch overload now match the single-message overload, fixing a potential NPE
Test covers the key scenario (4-element batch → 4 distinct dedup ids). Single-message send path is unchanged.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
|
🧪 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).
|
…batch entry a distinct MessageDeduplicationId (#24785) CAMEL-24086: camel-aws2-sqs - give each FIFO batch entry a distinct MessageDeduplicationId (#24728) When sending a batch to a FIFO queue, every entry is built from the same Exchange, so configureFifoAttributes derived the MessageDeduplicationId from that one Exchange via the configured strategy. The default ExchangeIdMessageDeduplicationIdStrategy returns exchange.getExchangeId(), identical for all entries, so SQS accepted the batch but silently dropped all but the first message within the deduplication window. The batch overload now appends the entry position to the strategy-provided id, giving each message a distinct, deterministic deduplication id (so a redelivered batch still deduplicates correctly). A null id (content-based deduplication via NullMessageDeduplicationIdStrategy) is left unset, and the group-id / dedup-id strategies are now null-guarded consistently with the single-message overload. Adds SqsProducerFifoBatchDeduplicationTest asserting the batch entries receive distinct deduplication ids. The single-message send path is unchanged. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…batch entry a distinct MessageDeduplicationId (#24786) CAMEL-24086: camel-aws2-sqs - give each FIFO batch entry a distinct MessageDeduplicationId (#24728) When sending a batch to a FIFO queue, every entry is built from the same Exchange, so configureFifoAttributes derived the MessageDeduplicationId from that one Exchange via the configured strategy. The default ExchangeIdMessageDeduplicationIdStrategy returns exchange.getExchangeId(), identical for all entries, so SQS accepted the batch but silently dropped all but the first message within the deduplication window. The batch overload now appends the entry position to the strategy-provided id, giving each message a distinct, deterministic deduplication id (so a redelivered batch still deduplicates correctly). A null id (content-based deduplication via NullMessageDeduplicationIdStrategy) is left unset, and the group-id / dedup-id strategies are now null-guarded consistently with the single-message overload. Adds SqsProducerFifoBatchDeduplicationTest asserting the batch entries receive distinct deduplication ids. The single-message send path is unchanged. Signed-off-by: Andrea Cosentino <ancosen@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Description
Fixes CAMEL-24086.
When sending a batch (a
Listbody, or aStringsplit bybatchSeparator) to a FIFO SQS queue, every entry is built from the sameExchange, soconfigureFifoAttributes(SendMessageBatchRequestEntry.Builder, Exchange)derived theMessageDeduplicationIdfrom that oneExchange. The defaultExchangeIdMessageDeduplicationIdStrategyreturnsexchange.getExchangeId()— identical for all entries — so SQS accepted the batch but silently dropped all but the first message within the 5-minute deduplication window.Fix
<id>-<index>), giving each message a distinct, deterministic deduplication id — a redelivered batch still deduplicates correctly.nullid (content-based deduplication viaNullMessageDeduplicationIdStrategy) is left unset.Tests
Adds
SqsProducerFifoBatchDeduplicationTest— sends a 4-element batch to a.fifoqueue and asserts the four entries receive four distinct deduplication ids (and each sets a group id). Against the old code all four shared one id, so the test fails without the fix. The existingSqsProducerBatchTeststill passes.Scope note
This PR addresses only the FIFO deduplication-id collision. The separate "batch not chunked to the 10-entry API limit" observation is not included here.
Backport
Same code on
camel-4.18.xandcamel-4.14.x; will be backported after merge (fixVersions 4.22.0 / 4.18.4 / 4.14.9).Claude Code on behalf of oscerd