Skip to content
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

Further MQTT streaming hardening #1327

Merged
merged 6 commits into from
Nov 16, 2018
Merged

Further MQTT streaming hardening #1327

merged 6 commits into from
Nov 16, 2018

Conversation

huntc
Copy link
Contributor

@huntc huntc commented Nov 13, 2018

Numerous hardening activities around MQTT streaming. Please visit each commit for a full description. However, in summary:

  • Corrected misinterpretation of behaviour setup
  • Avoid a race condition when creating child actors
  • Handle a duplicate publish while consuming
  • Connection packet id distinction which now allows more than one client to send the same packet id as another

I had thought that Behaviors.setup would only get called once for an actor. This is not the case - it is called each time a behaviour is transitioned to.

There was only one place where my misunderstanding mattered, which was where a promise could be tried to be completed multiple times.
We observed an `InvalidActorNameException` when creating child actors. This could have been due to the following sequence when receiving a “Publish Received Locally” message:

1. prl received, producer actor created
2. producer actor terminates and sends a termination message
3. prl received before termination message is received, the parent actor creates another producer
4. the termination message is received and then attempts to create another producer with the same name

The solution is to explicitly track active consumers and producers rather than rely on another data structure such as `context.children`, which will be updated in response to other events.
Duplicate publish events received from a remote were previously stashed until any existing handling was complete. This commit changes that so that they are routed and handled.

The commit also tightens up some exception handling around benign exceptions that the developer should not need to consider.
In particular, actor names need to avoid the sequence number of an actor conflicting with MQTT topic names.
Packet ids were previously not distinguished according to their connection. This caused a problem when there was more than one client issuing the same packet ids!

This commit addresses the problem by permitting the routing of remotely received packet ids by connection id and client id.
@huntc huntc changed the title WIP - DO NOT MERGE - further MQTT streaming hardening Further MQTT streaming hardening Nov 15, 2018
@huntc huntc changed the title Further MQTT streaming hardening WIP - DO NOT MERGE - Further MQTT streaming hardening Nov 16, 2018
Accidentally removed topic filtering from a previous commit. This could cause unexpected behaviour given that every publish would be broadcast to all topics.
@huntc huntc changed the title WIP - DO NOT MERGE - Further MQTT streaming hardening Further MQTT streaming hardening Nov 16, 2018
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

@ennru ennru merged commit f4d6193 into akka:master Nov 16, 2018
@ennru ennru added this to the 1.0-M2 milestone Nov 16, 2018
@ennru
Copy link
Member

ennru commented Nov 16, 2018

Thank you!

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.

None yet

3 participants