MQTT streaming: Retire ask in favour of promise #1304
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
The ask pattern is overkill for situations where there is a strong coupling between one actor and another, as is the case between an MQTT session and its many command and event flows. For instance, if an MQTT session actor is lost for any reason, then its flows will also be failed. In this scenario, where a flow notifies the session of, say, an event having been received, it is effectively “calling” the session actor as per a function call, but using a message. The request is effectively synchronous in nature (although there is no blocking of course). The session actor must promise to yield a result, no matter what. Not yielding a result is a bug. As such, we can use promises instead of the ask pattern for tightly coupled callers calling on actors. This then yields the advantage that request/reply timeouts do not have to be considered. Indeed, a strong motivation for the retirement of ask here is that it is difficult to determine a default timeout value. The performance of the MQTT session on a single core processor with memory constraints is quite different to that of a quad core with enormous amounts of memory. Timeouts perhaps make more sense when considering networks, where request/reply is required instead of a synchronous call. Thanks to Jason Longshore for motivating this change and his thoughts on when to use the synchronous call pattern in place of ask.
longshorej
approved these changes
Nov 5, 2018
| @@ -253,7 +253,8 @@ import scala.util.{Failure, Success} | |||
| Subscriber(subscribeData, remote, data.subscriberPacketRouter, data.settings), | |||
| subscriberName | |||
| ) | |||
| case _: Some[_] => // Ignored for existing subscriptions | |||
| case _: Some[_] => | |||
| remote.failure(new IllegalStateException("Duplicate subscribe: " + subscribe)) | |||
ennru
changed the title
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.
The ask pattern is overkill for situations where there is a strong coupling between one actor and another, as is the case between an MQTT session and its many command and event flows. For instance, if an MQTT session actor is lost for any reason, then its associated flows will also be failed. In this scenario, where a flow notifies the session of, say, an event having been received, it is effectively “calling” the session actor as per a function call, but using a message. The request is effectively synchronous in nature (although there is no blocking of course). The session actor must promise to yield a result, no matter what. Not yielding a result is a bug.
As such, we can use promises instead of the ask pattern for tightly coupled callers calling on actors. This then yields the advantage that request/reply timeouts do not have to be considered. Indeed, a strong motivation for the retirement of ask here is that it is difficult to determine a default timeout value. The performance of the MQTT session on a single core processor with memory constraints is quite different to that of a quad core with enormous amounts of memory. Timeouts perhaps make more sense when considering networks, where request/reply is required instead of a synchronous one.
Thanks to @longshorej for motivating this change and his thoughts on when to use the synchronous call pattern in place of ask.