Improvements to AWS topic subscription and SQS access policy generation#808
Merged
yang-xiaodong merged 1 commit intodotnetcore:masterfrom Mar 29, 2021
Conversation
yang-xiaodong
approved these changes
Mar 29, 2021
Member
yang-xiaodong
left a comment
There was a problem hiding this comment.
Looks good, Sorry for the delayed review
Member
|
Hello @AndriiLab , This PR included in version Do you have interest to join @dotnetcore/cap-committers of CAP as a maintainer? |
Collaborator
Author
|
Hello @yang-xiaodong, |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 are few problems with
DotNetCore.CAP.AmazonSQSprovider. Our setup includes about 150 topics and about 15 queues. CAP is configured withConsumerThreadCount = 4. This results in about 700 subscriptions.Problem 1
When executing CAP with this configuration the subscription to SNS fails with the error
AmazonSimpleNotificationServiceException: Rate exceededand then required queues not created. This relates to thesnsClient.CreateTopicAsynccall and occurs because AWS cannot process so many concurrent requests of SNS topic creation at a time.Proposed solution to Problem 1
Separate topic creation and subscription process (i.e. extract topic creation out of the consumer threads). I have added
FetchTopicsmethod toIConsumerClientwhich returns a list of topics by default, but for AmazonSQS provider it handles the logic for SNS topic creation and/or its ARN fetching. This method is called before consumer threads creation.Problem 2
When subscribing more than 20 SNS topics to the queue the access policy generated incorrectly and only the first 20 topics are able to post messages into the queue.
Proposed solution to Problem 2
This relates to the internal limit for SQS access policy to include only 20 statements. Thus to enable more than 20 SNS topics access to a single SQS we need to compact policy from:
{ "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Principal": { "AWS": "*" }, "Action": "sqs:SendMessage", "Resource": "arn:aws:sqs:us-east-1:MyQueue", "Condition": { "ArnLike": { "aws:SourceArn": "arn:aws:sns:us-east-1:FirstTopic" } } }, { "Effect": "Allow", "Principal": { "AWS": "*" }, "Action": "sqs:SendMessage", "Resource": "arn:aws:sqs:us-east-1:MyQueue", "Condition": { "ArnLike": { "aws:SourceArn": "arn:aws:sns:us-east-1:SecondTopic" } } }] }to the compacted version with a single statement:
{ "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Principal": { "AWS": "*" }, "Action": "sqs:SendMessage", "Resource": "arn:aws:sqs:us-east-1:MyQueue", "Condition": { "ArnLike": { "aws:SourceArn": [ "arn:aws:sns:us-east-1:FirstTopic", "arn:aws:sns:us-east-1:SecondTopic" ] } } }] }After examination of AWS SDK, I have ended up with
AmazonPolicyExtensionsextensions, which help to achieve this.Both improvements affect only
DotNetCore.CAP.AmazonSQSprovider, but require changes inDotNetCore.CAPsince the execution of theFetchTopicsmethod needs to be added toConsumerRegister