Skip to content

Conversation

@shivanshgaur
Copy link
Contributor

@shivanshgaur shivanshgaur commented Jul 10, 2020

Currently, Redis broker pops concurrency+2 extra messages from the queue and keeps it in memory while processing previous messages. Now if the worker process shuts down abruptly during the processing due to unforeseen circumstances, then concurrency+2 messages are lost forever.
Also, during graceful termination, the already popped messages are first processed and only then the worker shuts down. This leads to higher time for graceful terminations and one has to set the graceful termination timeout period to a higher value.
This PR makes changes to enqueue those messages back into the queue once a termination signal is received. This will bring down the time for graceful termination considerably.

@codecov
Copy link

codecov bot commented Jul 10, 2020

Codecov Report

Merging #577 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #577   +/-   ##
=======================================
  Coverage   39.38%   39.38%           
=======================================
  Files          32       32           
  Lines        3344     3344           
=======================================
  Hits         1317     1317           
  Misses       1885     1885           
  Partials      142      142           

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 802f45d...dce1ec8. Read the comment docs.

close(deliveries)
return
case <-pool:
select {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to prevent any further message pops

@RichardKnop RichardKnop merged commit a5c08ef into RichardKnop:master Jul 10, 2020
@shivanshgaur shivanshgaur deleted the avoid_message_loss branch July 10, 2020 17:41
@shivanshgaur
Copy link
Contributor Author

@RichardKnop Can we create a release with this?

@RichardKnop
Copy link
Owner

Sure will do.

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