Skip to content

CLD-538: route failed DNS-update messages to DLQ via partial-batch fa… - #15

Open
nicktill-ibm wants to merge 1 commit into
mainfrom
cld_538_dlq_client
Open

CLD-538: route failed DNS-update messages to DLQ via partial-batch fa…#15
nicktill-ibm wants to merge 1 commit into
mainfrom
cld_538_dlq_client

Conversation

@nicktill-ibm

@nicktill-ibm nicktill-ibm commented Jun 30, 2026

Copy link
Copy Markdown

Failed SQS records were being lost or causing duplicate NS1 POSTs. The handler returned batchItemFailures, but the event source mapping never had ReportBatchItemFailures enabled, so Lambda ignored it: soft failures (a returned messageId) were deleted as success, and uncaught exceptions failed the whole batch and re-POSTed already-synced records on retry forever, with no DLQ.

  • template: enable ReportBatchItemFailures on the SQS event source; add a RedrivePolicy on CloudSyncUpdatesQueue (maxReceiveCount 5) targeting the existing DLQ; set VisibilityTimeout 60s and DLQ retention to 14 days; drop the unused EventInvokeConfig MaximumRetryAttempts (async-only).
  • handler: wrap each record in try/except so one failure reports only its own messageId instead of failing the batch.
  • tests: add unit coverage for the batch contract and record_handler outcomes, plus conftest import/region setup and missing test deps.

@nicktill-ibm

Copy link
Copy Markdown
Author

There are a few issues/concerns with the current approach:

  1. Soft failure → silently skipped. record_handler() correctly returns the failed messageId, but without ReportBatchItemFailures enabled on the event source, Lambda ignores that return value entirely and tells SQS "the whole batch succeeded." So the failed message gets deleted along with everything else, no retry, no DLQ, just gone.
  2. Hard failure → whole batch retried, not just the failed one. With no try/except around each record, an exception from one record propagates out of handler() and fails the entire invocation. SQS's response to that is to redeliver the entire batch (all 5 messages), including the ones that already succeeded and were already POSTed to NS1, causing duplicate POSTs to the gateway for records that didn't need reprocessing.
  3. Hard failure → no terminal state, loops forever. Because DNSUpdateQueue had no RedrivePolicy wired to the DLQ, there was no maxReceiveCount cap. A permanently-broken message (bad payload, etc.) just cycles: retry → fail → whole batch redelivered → retry → fail, forever, with no offramp to land somewhere for investigation.

This PR should fix all three. Th re-drive logic to retry the messages off the DLQ will be added on in another PR.

@nicktill-ibm
nicktill-ibm marked this pull request as ready for review August 4, 2026 13:47
@nicktill-ibm
nicktill-ibm requested a review from cbertinato August 4, 2026 13:47

@cbertinato cbertinato left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants