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

Add lease acquisition recovery interval configurability #406

Merged
merged 6 commits into from
Apr 29, 2021

Conversation

duinness
Copy link
Contributor

@duinness duinness commented Mar 2, 2021

Adding the ability to modify the acquisition recovery interval because when you have a large number of streams on one account trying to recover every 5 seconds can get you stuck in a rate-limit loop.

My test environment has ~100 kinesis streams being managed by 10 instances of lifion-kinesis.
After patching my lifion-kinesis with this change and setting the leaseAquisitionRecoveryInterval to 30 seconds, errors in the environment related to this went from ~5000/min to ~20/min.

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #406 (a6812e8) into develop (5454e62) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##           develop      #406   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1346      1346           
  Branches       185       184    -1     
=========================================
  Hits          1346      1346           
Impacted Files Coverage Δ
lib/index.js 100.00% <ø> (ø)
lib/lease-manager.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5454e62...cb8367f. Read the comment docs.

@duinness duinness changed the title Tie 2146 - add lease acquisition recovery interval configurability Add lease acquisition recovery interval configurability Mar 2, 2021
@duinness
Copy link
Contributor Author

duinness commented Mar 8, 2021

Here is the issue to which this PR should be linked
#409

Doesn't look like I have permission to link them... or else I'm doing something wrong, which is a strong possibility 🙂

Copy link
Contributor

@jennyEckstein jennyEckstein left a comment

Choose a reason for hiding this comment

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

@eaviles PR looks good to me. Just need to patch the doc

@jennyEckstein jennyEckstein merged commit 3d17f73 into lifion:develop Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants