Add rate limit to Kinesis requests #9863
Conversation
Jackie-Jiang
added
Configuration
Jackie-Jiang
left a comment
Jackie-Jiang
left a comment
There was a problem hiding this comment.
LGTM. Please add the new configuration to the PR description and the documentation
| private final int _numMaxRecordsToFetch; | ||
| private final ExecutorService _executorService; | ||
| private final ShardIteratorType _shardIteratorType; | ||
| private int _rpsLimit; |
There was a problem hiding this comment.
(minor) This can be final. Also, should we treat 0 as unlimited?
There was a problem hiding this comment.
I don't think we should allow unlimited. It opens up misconfiguration. Instead we can just ignore with a warning and reset to the default value.
| private final int _numMaxRecordsToFetch; | ||
| private final ExecutorService _executorService; | ||
| private final ShardIteratorType _shardIteratorType; | ||
| private int _rpsLimit; |
There was a problem hiding this comment.
I don't think we should allow unlimited. It opens up misconfiguration. Instead we can just ignore with a warning and reset to the default value.
Codecov Report
@@ Coverage Diff @@
## master #9863 +/- ##
=============================================
+ Coverage 28.06% 61.36% +33.30%
- Complexity 53 4827 +4774
=============================================
Files 1949 1966 +17
Lines 104632 105848 +1216
Branches 15847 16055 +208
=============================================
+ Hits 29362 64953 +35591
+ Misses 72395 36070 -36325
- Partials 2875 4825 +1950
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| break; | ||
| } | ||
|
|
||
| if (requestSentTime == currentWindow && getRecordsResponse.records().isEmpty()) { |
There was a problem hiding this comment.
@navina I feel like I should remove this isEmpty check from here. Reason being it doesn't cover the cases when we might be getting only 1-2 records on each .getRecords request and thus exceed the throughput limit.
4 participants
This is being done to avoid ThroughputExceeded exceptions in Kinesis due to 5RPS limit on .getRecords requests.
More on this - https://aws.amazon.com/premiumsupport/knowledge-center/kinesis-readprovisionedthroughputexceeded/
Documentation
You can use the following config to enforce per shard rate limit on Kinesis