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

Match for a Throttling/Rate Limiting error #20

Merged
merged 1 commit into from
May 25, 2021

Conversation

jschniper
Copy link
Contributor

I made this fork a while back and meant to submit a PR but this should help with the problem in issue #6.

@pmenhart
Copy link
Collaborator

Did you test this solution in high load situations?
AWS limit is 5 requests per second per log stream. We are supposed to re-try after a delay.

I implemented two alternatives in my fork:

  • Remove all messages from the buffer. Transfer to AWS is delayed until the buffer fills again. Disadvantage: messages are lost.
  • Explicitly delay transfer by 500msec then try again. Retrying without shedding the load brings a risk of compromising system stability.

Neither of these alternatives is bullet-proof. Therefore I am still not comfortable to offer my change as a PR for general use.

@jschniper
Copy link
Contributor Author

My only goal with this was to prevent the hard lock in our application that we were seeing when the Elixir logger switched from asynchronous to synchronous logging. I think there is definitely more work that needs to be done but I just wanted to get this out here in case anyone else ran into the same issue.

@pmenhart
Copy link
Collaborator

You are right, it is definitely better than an unmatched error. We should accept your PR and add a comment/caveat to the code.
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants