forked from localstack/localstack
-
Notifications
You must be signed in to change notification settings - Fork 0
add: Support for ESM v2 partial batch failure handling (Kinesis & DynamoDB) #9
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
Open
lizard-boy
wants to merge
3
commits into
master
Choose a base branch
from
add/esm/function-response-types
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,12 +22,11 @@ | |
| get_datetime_from_timestamp, | ||
| get_internal_client, | ||
| ) | ||
| from localstack.services.lambda_.event_source_mapping.pollers.poller import Poller | ||
| from localstack.services.lambda_.event_source_mapping.pollers.sqs_poller import get_queue_url | ||
| from localstack.services.lambda_.event_source_mapping.senders.sender import ( | ||
| PartialFailureSenderError, | ||
| SenderError, | ||
| from localstack.services.lambda_.event_source_mapping.pollers.poller import ( | ||
| Poller, | ||
| get_batch_item_failures, | ||
| ) | ||
| from localstack.services.lambda_.event_source_mapping.pollers.sqs_poller import get_queue_url | ||
| from localstack.utils.aws.arns import parse_arn | ||
|
|
||
| LOG = logging.getLogger(__name__) | ||
|
|
@@ -192,18 +191,40 @@ def poll_events_from_shard(self, shard_id: str, shard_iterator: str): | |
| except PartialBatchFailureError as ex: | ||
| # TODO: add tests for partial batch failure scenarios | ||
| if ( | ||
| self.stream_parameters["OnPartialBatchItemFailure"] | ||
| self.stream_parameters.get("OnPartialBatchItemFailure") | ||
| == OnPartialBatchItemFailureStreams.AUTOMATIC_BISECT | ||
| ): | ||
| # TODO: implement and test splitting batches in half until batch size 1 | ||
| # https://docs.aws.amazon.com/eventbridge/latest/pipes-reference/API_PipeSourceKinesisStreamParameters.html | ||
| LOG.warning( | ||
| "AUTOMATIC_BISECT upon partial batch item failure is not yet implemented. Retrying the entire batch." | ||
| ) | ||
| error_payload = ex.partial_failure_payload | ||
| # let entire batch fail (ideally raise BatchFailureError) | ||
| except (SenderError, PartialFailureSenderError, BatchFailureError, Exception) as ex: | ||
| if isinstance(ex, (SenderError, PartialFailureSenderError, BatchFailureError)): | ||
| error_payload = ex.error | ||
|
|
||
| # If the batchItemFailures array contains multiple items, Lambda uses the record with the lowest sequence number as the checkpoint. | ||
| # Lambda then retries all records starting from that checkpoint. | ||
|
|
||
| failed_sequence_ids: list[int] | None = get_batch_item_failures( | ||
| ex.partial_failure_payload | ||
| ) | ||
|
|
||
| # If None is returned, consider the entire batch a failure. | ||
| if failed_sequence_ids is None: | ||
| continue | ||
|
|
||
| # This shouldn't be possible since a PartialBatchFailureError was raised | ||
| if len(failed_sequence_ids) == 0: | ||
| LOG.warning( | ||
| "Invalid state encountered: PartialBatchFailureError raised but no batch item failures found." | ||
| ) | ||
| return | ||
|
|
||
| lowest_sequence_id: str = min(failed_sequence_ids, key=int) | ||
|
|
||
| # Discard all successful events and re-process from sequence number of failed event | ||
| _, events = self.bisect_events(lowest_sequence_id, events) | ||
| except (BatchFailureError, Exception) as ex: | ||
| if isinstance(ex, BatchFailureError): | ||
| error_payload = ex.error | ||
|
|
||
| # FIXME partner_resource_arn is not defined in ESM | ||
|
|
@@ -213,11 +234,9 @@ def poll_events_from_shard(self, shard_id: str, shard_iterator: str): | |
| self.partner_resource_arn or self.source_arn, | ||
| events, | ||
| ) | ||
| attempts += 1 | ||
| finally: | ||
| # Retry polling until the record expires at the source | ||
| if self.stream_parameters.get("MaximumRetryAttempts", -1) == -1: | ||
| # TODO: handle iterator expired scenario | ||
| return | ||
| attempts += 1 | ||
|
|
||
| # Send failed events to potential DLQ | ||
| abort_condition = abort_condition or "RetryAttemptsExhausted" | ||
|
|
@@ -324,3 +343,12 @@ def max_retries_exceeded(self, attempts: int) -> bool: | |
| if maximum_retry_attempts == -1: | ||
| return False | ||
| return attempts > maximum_retry_attempts | ||
|
|
||
| def bisect_events( | ||
| self, sequence_number: str, events: list[dict] | ||
| ) -> tuple[list[dict], list[dict]]: | ||
| for i, event in enumerate(events): | ||
| if self.get_sequence_number(event) == sequence_number: | ||
| return events[:i], events[i:] | ||
|
|
||
| return events, [] | ||
|
Comment on lines
+347
to
+354
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider optimizing bisect_events for large event lists, potentially using binary search |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,9 +19,20 @@ class LambdaSender(Sender): | |
| # Flag to enable the payload dict using the "Records" key used for Lambda event source mapping | ||
| payload_dict: bool | ||
|
|
||
| def __init__(self, target_arn, target_parameters=None, target_client=None, payload_dict=False): | ||
| # Flag to enable partial successes/failures when processing batched events through a Lambda event source mapping | ||
| report_batch_item_failures: bool | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider adding type hints for boolean flags |
||
|
|
||
| def __init__( | ||
| self, | ||
| target_arn, | ||
| target_parameters=None, | ||
| target_client=None, | ||
| payload_dict=False, | ||
| report_batch_item_failures=False, | ||
| ): | ||
| super().__init__(target_arn, target_parameters, target_client) | ||
| self.payload_dict = payload_dict | ||
| self.report_batch_item_failures = report_batch_item_failures | ||
|
|
||
| def send_events(self, events: list[dict]) -> dict: | ||
| if self.payload_dict: | ||
|
|
@@ -72,18 +83,17 @@ def send_events(self, events: list[dict]) -> dict: | |
| error=error, | ||
| ) | ||
|
|
||
| # TODO: test all success, partial, and failure conditions: | ||
| # https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-pipes-dynamodb.html#pipes-ddb-batch-failures | ||
| # The payload can contain the key "batchItemFailures" with a list of partial batch failures: | ||
| # https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-pipes-batching-concurrency.html | ||
| if has_batch_item_failures(payload): | ||
| if self.report_batch_item_failures and has_batch_item_failures(payload): | ||
| error = { | ||
| "message": "Target invocation failed partially.", | ||
| "httpStatusCode": invoke_result["StatusCode"], | ||
| "awsService": "lambda", | ||
| "requestId": invoke_result["ResponseMetadata"]["RequestId"], | ||
| "exceptionType": "BadRequest", | ||
| "resourceArn": self.target_arn, | ||
| "executedVersion": invoke_result.get("ExecutedVersion", "$LATEST"), | ||
| } | ||
| raise PartialFailureSenderError(error=error, partial_failure_payload=payload) | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Use a constant or enum for the list of services that support FunctionResponseTypes