Various AsyncProducer improvements #855
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dasch
requested changes
Aug 3, 2020
spec/async_producer_spec.rb
Outdated
|
|
||
| expect(async_producer.thread_mutex).to_not eq(async_producer2.thread_mutex) | ||
| end | ||
| end |
There was a problem hiding this comment.
I don't think we need a test for this – it's not testing functionality, but rather implementation details.
Currently we reuse the same mutex for every instance of `AsyncProducer`. This means that one producer can block another producer from producing if it holds the mutex. This is unnecessary because the details of whether a particular producer's threads are running are irrelevant to every other producer.
There's no need to grab the mutex twice.
This is in preparation for a future change which will add another call to these methods and we don't want to duplicate the code.
We don't need to grab the mutex if both threads are still alive. But if we _do_ need to grab the mutex (i.e., if either/both of the threads are no longer alive), we'll need to re-check that the threads are alive later since someone else may have already restarted them by the time we've grabbed the mutex.
Currently if the worker thread has died unexpectedly and we subsequently call `deliver_messages`, the messages won't actually get delivered until another message is produced. With this change we will restart the worker thread so it can receive the `deliver_messages` message.
Similar to the previous commit, if a worker thread unexpectedly dies with unhandled messages in its queue and then we try to call `shutdown`, those messages won't be handled unless `produce` is called in the future (in which case the worker thread will read the `shutdown` message enqueued here and then exit again). With this change we will restart the worker thread so it can receive the `shutdown` message.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the commit messages for details.