Skip to content

Conversation

@PJEstrada
Copy link
Contributor

@PJEstrada PJEstrada commented Oct 12, 2021

Main Changes

  • Added GracefulKiller class to catch SIGINT AND SIGTERM signals.
  • Refactored the thread startup process to be on a separate class and be done on demand in main.py, this prevents threads from being recreated on every import of process_media.py
  • Added ProcessMediaQueueManager class to manages all the queues of the worker and centralize the queue management actions.
  • Small visual improvement to the input list so that errors can be seen when clickin on the error icon.

Gracefull Kill Algorithm Overview:

We will check the length of the FRAME_QUEUE, VIDEO_QUEUE and PROCESSING_INPUT_LIST and wait for a max of 2 hours until they are empty. If they all get empty, we kill the walrus. We also set the flagSTOP_PROCESSING_DATA=True to avoid processing new messages on the current worker.

If more than 2 hours pass and all the messages are not handled, we set the messages status to failed.

Notes:

This PR should be deployed alongside with this: diffgram/diffgram-helm#5 to allow kubernetes to give a grace period for shutfdown on new deploys

There's still an issue when killing the walrus and a video file is getting written using moviepy. I've created an issue on their repo to ask about ways to gor around this: Zulko/moviepy#1672

@github-actions
Copy link

github-actions bot commented Oct 12, 2021

Unit Test Results

0 files  ±0  0 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 0f77684. ± Comparison against base commit becd9a4.

♻️ This comment has been updated with latest results.

@cypress
Copy link

cypress bot commented Oct 12, 2021



Test summary

60 0 0 0Flakiness 0


Run details

Project DiffgramFrontend
Status Passed
Commit 0f77684
Started Oct 14, 2021 6:01 PM
Ended Oct 14, 2021 6:17 PM
Duration 16:00 💡
OS Linux Debian - 10.9
Browser Chrome 90

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@PJEstrada
Copy link
Contributor Author

Hey @Anthony-Sarkis!

This PR should now be ready to go. Please let me know if you find any issues with it or if there's something you think we need to change before meging

Copy link
Member

@anthony-chaudhary anthony-chaudhary left a comment

Choose a reason for hiding this comment

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

Great work!! Looks really good. 🎉

@anthony-chaudhary anthony-chaudhary merged commit eb5d0e8 into master Oct 14, 2021
@anthony-chaudhary anthony-chaudhary deleted the gracefull-shutdown branch October 14, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reprocessing stale inputs automatically - improve that process Idea: Process Media Supervisor

2 participants