Skip to content
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.

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

Fix: Firehose: Multiple firehose delivery streams not receiving messages from kinesis event stream #10155

Conversation

maxhoheiser
Copy link
Member

Motivation

Multiple instances of kcl listening to the same Kinesis event stream - only the "first" will process events, due to all using the same dynamoDB lease table. This was raised in #9476

We are spinning up a mock Kinesis server (https://github.com/etspaceman/kinesis-mock) and use the java plication aws kinesis client library to connect to this mock server. For services queriing events from the mock kinesis server, we provide a connector function listen_to_kinesis in kinesis_connector

This method starts the kcl for each listener. Kcl starts one or more instances with a worker bound to one or more shards. Each worker processes messages from the shard(s) and sends received messages to a programmatically created Python script that sends the messages to a socket. A listener function running in a separate thread listens to incoming messages on this socket and handles further processing defined by the initiator e.g. Firehose

To track shard lease and assignment shards to worker processes as well as to track checkpoints (latest read message from the shard) kcl uses a dynamoDB “lease table”

Changes

Adding a unique id to the lease table name for each instance of kcl that is instantiated.
Adding an AWS validated snapshot test for multiple Firehose delivery streams subscribing to the same Kinesis event stream

This PR fixes #9476

@maxhoheiser maxhoheiser added aws:firehose Amazon Data Firehose aws:kinesis Amazon Kinesis semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases labels Feb 1, 2024
@maxhoheiser maxhoheiser self-assigned this Feb 1, 2024
Copy link

github-actions bot commented Feb 1, 2024

LocalStack Community integration with Pro

    2 files      2 suites   1h 21m 52s ⏱️
2 623 tests 2 374 ✅ 249 💤 0 ❌
2 625 runs  2 374 ✅ 251 💤 0 ❌

Results for commit c186595.

♻️ This comment has been updated with latest results.

@maxhoheiser maxhoheiser marked this pull request as ready for review February 1, 2024 13:24
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

Would be nice if we could avoid that kinesis fixture change for now, to avoid more extensive testing on this :)

localstack/services/firehose/provider.py Show resolved Hide resolved
localstack/testing/pytest/fixtures.py Outdated Show resolved Hide resolved
tests/aws/services/firehose/conftest.py Outdated Show resolved Hide resolved
tests/aws/services/firehose/test_firehose.py Show resolved Hide resolved
@maxhoheiser maxhoheiser force-pushed the fix/firehose/9476-multiple-firehose-delivery-streams-not-receiving-messages-from-kinesis-stream branch 3 times, most recently from 2ddb641 to 147a8ac Compare February 6, 2024 12:09
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

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

Thanks for addressing my comments! Looks good to me!

@coveralls
Copy link

coveralls commented Feb 6, 2024

Coverage Status

coverage: 83.842% (-0.02%) from 83.861%
when pulling c186595 on fix/firehose/9476-multiple-firehose-delivery-streams-not-receiving-messages-from-kinesis-stream
into 871bfb8 on master.

@maxhoheiser maxhoheiser force-pushed the fix/firehose/9476-multiple-firehose-delivery-streams-not-receiving-messages-from-kinesis-stream branch from 147a8ac to c186595 Compare February 7, 2024 09:11
@maxhoheiser maxhoheiser merged commit d49889b into master Feb 7, 2024
27 checks passed
@maxhoheiser maxhoheiser deleted the fix/firehose/9476-multiple-firehose-delivery-streams-not-receiving-messages-from-kinesis-stream branch February 7, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws:firehose Amazon Data Firehose aws:kinesis Amazon Kinesis semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Multiple firehose delivery streams subscribing to kinesis stream doesn't fire both.
3 participants