-
Notifications
You must be signed in to change notification settings - Fork 174
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 queue flushing signal handler #792
base: develop
Are you sure you want to change the base?
Conversation
…'t drop the message that failed to publish.
try: | ||
message = event_queue.get(timeout=0.2) | ||
except TimedOutError: | ||
if len(batcher.serialize()) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The length here will always be greater than 0
because we add a header. We might want to add a items_count
function to the batcher to count the number of items currently in the queue. It could use the length of batcher._items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
Closes #
💸 TL;DR
Add a queue flushing signal handler, so when the event publisher receives a SIGINT/SIGTERM signal it'll continue flushing the queue, so events aren't dropped on shutdown.
📜 Details
Design Doc
Jira
🧪 Testing Steps / Validation
Tested using the included
Dockerfile
using the following steps:This screenshot below shows the reproduction of a bug where we would exit from a
BatchFull
exception:This screenshot below shows that the
BatchFull
exception is no longer present using the same steps above and sending a SIGINT toevent_publisher
will still cause it to wait for the POSIX queue to be empty before exiting:✅ Checks