Skip to content

fix: stream-router contract — SequenceNumber batch failures and REMOVE record guard - #86

Merged
dwsmith1983 merged 9 commits into
mainfrom
fix/stream-router-contract
Sep 12, 2026
Merged

fix: stream-router contract — SequenceNumber batch failures and REMOVE record guard#86
dwsmith1983 merged 9 commits into
mainfrom
fix/stream-router-contract

Conversation

@dwsmith1983

@dwsmith1983 dwsmith1983 commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

Two fixes in the stream-router Lambda, plus matching filters on the DynamoDB event source mappings.

  • Batch item failures now report the record sequence number. The handler returned the event ID, which AWS does not recognise, so any partial failure re-drove the entire batch and replayed job-log writes and published events that are not safe to repeat. With the correct identifier Lambda checkpoints at the first failed record. Records after that point are still re-driven (that is how partial-batch reporting works), so making those writes safe to repeat remains a separate change.
  • Deleted rows are no longer processed as writes. The handler never checked the stream record type, so a deleted row reached the write handlers with no item data. Deleting a rerun-request row started a rerun with reason manual, deleting a sensor row (including the baseline delete the rerun path itself issues) could publish a post-run verdict, and 30-day job-log expiries invoked the function for nothing. Deletes are now skipped and logged, with TTL expiries identified. The one exception is a deleted pipeline config, which still invalidates the config cache.
  • Event source mapping filters in deploy/terraform/lambda.tf: inserts and updates always pass; deletes pass only for the control table's config row; the job-log mapping gets the writes filter only. This is an in-place update. Filtered records advance the stream without invoking or billing the function.
  • LocalStack parity in deploy/localstack/deploy.py, verified end to end: the full local deploy and smoke check passed, and both mappings show the expected filters.
  • First test file for the live internal/lambda/stream package: failure identifier, delete guard (verified failing before the fix: writes, a state machine start, and a published event were observed for a deleted row), config cache invalidation, TTL detection, key extraction ordering.

The deprecated copy in internal/lambda/stream_router.go is untouched because its tests assert the old identifier; its doc comment now flags the known problems. It is removed when the root test suite is ported to the live packages.

Operator note

Run terraform apply after merging to pick up the new event source mapping filters. The handler guard protects deployments in the meantime, and deploy order does not matter. An existing LocalStack stack needs a teardown and redeploy to get the filters.

Test plan

  • make audit green at HEAD (lint clean, race suite passes)
  • terraform -chdir=deploy/terraform validate succeeds
  • LocalStack full deploy and smoke check passed; filters present on both stream mappings
  • After terraform apply: confirm both stream mappings show filter criteria and that deleting a rerun-request row no longer starts an execution

Follow-ups

  • Make the job-log write and event publish safe to repeat, so a retried batch cannot duplicate them.
  • The config cache re-reads the table on every call while no pipeline configs exist.
  • Port the root internal/lambda test suite to the live sub-packages and delete the deprecated copies.
  • Store-layer write races: rerun accounting, the unconditional trigger status upsert, and the lock reset without a status check.

AWS matches ReportBatchItemFailures identifiers against the DynamoDB
stream record SequenceNumber. HandleStreamEvent returned the EventID, so
Lambda did not recognise the identifier and re-drove the whole batch --
with bisect_batch_on_function_error and maximum_retry_attempts = 3 that
replayed already-successful records whose WriteJobEvent and PublishEvent
side effects are not idempotent.

Adds the first test file to internal/lambda/stream.
handleRecord ignored EventName, so a deleted row reached the write
handlers with a nil NewImage. Deleting a RERUN_REQUEST# row (those rows
have no TTL and are never cleaned up) started a rerun with reason
"manual"; deleting a SENSOR# row -- including the postrun-baseline#
delete the rerun path itself issues -- could publish POST_RUN_PASSED or
POST_RUN_FAILED; 30-day JOB# TTL expiries invoked the Lambda for nothing.

REMOVE records are now skipped and logged at info with the pk, sk, event
id and whether the delete came from TTL expiry. SK = CONFIG is the one
exception: it still invalidates the config cache.
Defense in depth behind the handleRecord REMOVE guard. Both stream
mappings now carry filter_criteria; the OR-ed filters pass INSERT and
MODIFY on both tables, plus REMOVE of the CONFIG row on the control
table so the config cache is still invalidated when a pipeline config is
deleted. The joblog table holds no CONFIG rows, so it gets the writes
filter only.

Filtered records advance the stream iterator without invoking or billing
the function, which also removes the 30-day JOB# TTL-expiry invocations.
Requires terraform apply to take effect on existing deployments.
Mirrors the filter_criteria blocks added to deploy/terraform/lambda.tf so
local E2E runs exercise the same delivery contract as production: writes
always pass, and REMOVE passes only for the control table's CONFIG row.

Existing mappings are left alone (the function is still idempotent), so a
running stack needs a teardown and redeploy to pick the filters up.
Documents the SequenceNumber batch-failure fix and the REMOVE guard,
including the event source mapping filters and the terraform apply that
operators need to run to pick them up.
@github-actions github-actions Bot added tests Test changes lambda Lambda handlers deploy Deployment and ASL docs Documentation labels Sep 12, 2026
@dwsmith1983 dwsmith1983 self-assigned this Sep 12, 2026
@dwsmith1983
dwsmith1983 merged commit a209393 into main Sep 12, 2026
3 checks passed
@dwsmith1983
dwsmith1983 deleted the fix/stream-router-contract branch September 12, 2026 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy Deployment and ASL docs Documentation lambda Lambda handlers tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant