8000 fix(esm/kinesis): Always store NextShardIterator from GetRecords by gregfurman · Pull Request #12677 · localstack/localstack · GitHub
[go: up one dir, main page]
Skip to content

fix(esm/kinesis): Always store NextShardIterator from GetRecords #12677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 29, 2025
Merged

fix(esm/kinesis): Always store NextShardIterator from GetRecords #12677

merged 1 commit into from
May 29, 2025

Conversation

gregfurman
Copy link
Contributor
@gregfurman gregfurman commented May 28, 2025 •
edited
Loading

Motivation

Currently, if an ESMs MaximumBatchingWindowInSeconds is 0 AND no records are collected, a shard iterator never moves forward. This PR ensures that we will always update a shard's iterator value given a successful GetRecords call.

Note, that any failures during processing should be handled by the internal retrier loop resulting in retrying the batch until successful or being discarded/sent to a DLQ. We should not be relying on the shard iterator value in order to keep track since these values are ephemeral and prone to expiry (i.e if a lambda processes for longer than 300s).

Changes

  • Ensure we always update a shard iterator following a succesful GetRecords call.

@gregfurman gregfurman self-assigned this May 28, 2025
@gregfurman gregfurman added type: bug Bug report semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) labels May 28, 2025
@gregfurman gregfurman added this to the 4.5 milestone May 28, 2025
@github-actions GitHub Actions
Copy link

Test Results - Preflight, Unit

21 579 tests  ±0   19 927 ✅ ±0   6m 23s ⏱️ +17s
     1 suites ±0    1 652 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 311db75. ± Comparison against base commit b90f172.

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 8s ⏱️ -1s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 311db75. ± Comparison against base commit b90f172.

@github-actions GitHub Actions
Copy link

Test Results - Alternative Providers

597 tests   420 ✅  14m 37s ⏱️
  4 suites  177 💤
  4 files      0 ❌

Results for commit 311db75.

@github-actions GitHub Actions
Copy link

LocalStack Community integration with Pro

    2 files  ±    0      2 suites  ±0   1h 18m 31s ⏱️ - 22m 56s
3 260 tests  - 1 209  2 974 ✅  - 1 107  286 💤  - 102  0 ❌ ±0 
3 262 runs   - 1 209  2 974 ✅  - 1 107  288 💤  - 102  0 ❌ ±0 

Results for commit 311db75. ± Comparison against base commit b90f172.

This pull request removes 1209 tests. 
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

@github-actions GitHub Actions
Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 23m 32s ⏱️
4 824 tests 4 283 ✅ 541 💤 0 ❌
4 830 runs  4 283 ✅ 547 💤 0 ❌

Results for commit 311db75.

@gregfurman gregfurman marked this pull request as ready for review May 29, 2025 07:18
@gregfurman gregfurman requested a review from tiurin May 29, 2025 07:18
@gregfurman gregfurman merged commit 4d2139c into master May 29, 2025
66 checks passed
@gregfurman gregfurman deleted the fix/esm/stream-repeating branch May 29, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiurin tiurin tiurin approved these changes

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev is a code owner

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

Assignees

@gregfurman gregfurman

Labels
aws:lambda:event-source-mapping AWS Lambda Event Source Mapping (ESM) semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases type: bug Bug report
Projects
None yet
Milestone
4.5
Development

Successfully merging this pull request may close these issues.
2 participants
@gregfurman @tiurin
0