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 exception capture for CreateLogStream to avoid flush get stuck #192

Merged
merged 1 commit into from
Mar 10, 2024

Conversation

lyso
Copy link
Contributor

@lyso lyso commented Oct 24, 2023

Add exception capture for CreateLogStream, to avoid flush stuck when CreateLogStream throttle.

This PR is to address some situation mentioned in issue #132 . When CreateLogStream error out in batch mode (use_queues==True), e.g. throttle on this action, the flush() and close() could be stuck at q.join(). Because the exception raised during CreateLogStream stopped the log delivering thread, then the queue will never get cleared, then the main thread get stuck.

After adding the exception capture, the log delivering thread would be able to proceed to retry up on CreateLogStream throttle.

This PR only covers one of the causes of stuck I am experiencing. It does not introduce breaking changes, so can be published as patch version.

There might be more reasons causing stuck in flush(). It would be better to add a timeout in flush(). However, it will change the interface: adding a timeout threshold as a new argument in the class init and/or flush(). I will raise a separate PR for it.

@kislyuk kislyuk merged commit 04edbfe into kislyuk:develop Mar 10, 2024
@kislyuk
Copy link
Owner

kislyuk commented Mar 10, 2024

Thank you! I will adjust this code to catch a more specific exception, but otherwise, it looks good.

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.

2 participants