Requirements and design of thin-edge Rust extensions #1549
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
itsroshan
reviewed
Nov 8, 2022
itsroshan
reviewed
Nov 8, 2022
itsroshan
reviewed
Nov 8, 2022
itsroshan
reviewed
Nov 8, 2022
itsroshan
reviewed
Nov 8, 2022
albinsuresh
reviewed
Nov 15, 2022
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
b705940
to
27940b9
Compare
didier-wenzek
changed the title
5 tasks
albinsuresh
reviewed
Nov 28, 2022
design/thin-edge-actors-design.md
Outdated
| /// Turn on/off logging of the messages consumed from this mailbox. | ||
| /// | ||
| /// The messages are logged when returned by the `next()` method. | ||
| pub fn log_inputs(&mut self, on: bool) { |
There was a problem hiding this comment.
The developer is expected to turn this logging ON/OFF in code? Or this API will be connected to the external logging system like the tracing logger to control it more dynamically?
There was a problem hiding this comment.
This is a sketch of the API. Logging will be turn on/off by the user.
What I'm doing with this document is to find the right owner for each feature. Giving the responsibility of message logging to the mailbox makes this feature available to all actors without having to change the actor code if we improve logging.
design/thin-edge-actors-design.md
Outdated
Comment on lines
652
to
684
| and still less the unrelated message types consumed by the target as `C: Into<B>`. | ||
| Indeed, that `B` message type encompasses all the message kinds supported by the target, | ||
| and these, as the `C` type, can be completely unrelated to the source business domain that is described by the `A` type. |
There was a problem hiding this comment.
It was hard to follow those couple of sentences.
There was a problem hiding this comment.
When I feel urgent the need to add a diagram this is a sign that this is not so clear even to me ;-).
I will try to come with something clearer.
|
|
||
| /// An `Address<M>` is a `Recipient<N>` provided `N` implements `Into<M>` | ||
| impl<M: Message> Address<M> { | ||
| pub fn as_recipient<N: Message + Into<M>>(&self) -> Recipient<N> { |
There was a problem hiding this comment.
It would have been better if such type conversions are done using an externally pluggable adaptors rather than expecting an impl of Address<M>. Else, it feels like every Addresses needs to be aware of what all it can be converted into, which would lead to tight coupling between components.
There was a problem hiding this comment.
I agree that there is still room for improvement regarding these adapters. Even more, this is a key design point we need to polish.
However, note the loose type constraint: <N: Message + Into<M>>. Note also that this method returns a Recipient i.e. a dyn object abstracting the concrete address and ready to be used by an other actor unaware of the M message type.
impl C8YHandler {
pub fn as_measurement_consumer(&self) -> Recipient<Measurement> {
self.address.as_recipient()
}
pub fn as_event_consumer(&self) -> Recipient<Event> {
self.address.as_recipient()
}
pub fn as_alarm_consumer(&self) -> Recipient<Alarm> {
self.address.as_recipient()
}
}
design/thin-edge-actors-design.md
Outdated
|
|
||
| An actor can have no input messages and only acts as a source of output messages. | ||
| Similarly, an actor can have not output messages and be a sink of input messages. | ||
| Notable examples are respectively a source of measurements and a message logger. |
There was a problem hiding this comment.
Having a single centralised logging Actor could be a bit complex to maintain when the target of different actors could be separate log files on the disk. Sounds like an unnecessary complexity for now. Leaving the logging to individual actors themselves would be simpler, I feel.
There was a problem hiding this comment.
Having a single centralised logging Actor could be a bit complex to maintain when the target of different actors could be separate log files on the disk. Sounds like an unnecessary complexity for now. Leaving the logging to individual actors themselves would be simpler, I feel.
I agree. That said, at this design stage, it's important to understand what would be necessary to implement such a logger.
design/thin-edge-actors-design.md
Outdated
Comment on lines
874
to
904
| * With unbounded channels, there is a risk of actors being overflowed | ||
| and failing to catch up till memory exhaustion. | ||
| * Bounded channels enable back-pressure from slow actors onto fast ones, |
There was a problem hiding this comment.
The decision of unbounded or bounded channels might depend on the function.
It is important that we also support blocking style communication for specific actors (or messages?) so we can make the actors more composable. For some operations such as firmware, software, configuration, these generally should be processed serially to make the handling of those side-effect heavy artefacts more predictable (and generally the order does matter, so we want this to be repeatable).
Also by supporting serial processing of some messages means that operations can also be "queued" on the cloud side, and the user can be reassured that the operations will be processed in the order that they were created.
There was a problem hiding this comment.
That's a good point: the decision must be delegated to the code that connects the actors.
6 tasks
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
88644ff
to
86a3498
Compare
didier-wenzek
commented
Jan 16, 2023
design/thin-edge-actors-design.md
Outdated
Comment on lines
208
to
220
| That said, [actix](https://docs.rs/actix/latest/actix/) is a notable actor implementation for Rust. | ||
| So, this is an option to consider, | ||
| even if I would prefer to avoid the inerrant overhead of using such a large crate. | ||
|
|
||
| To guide a decision toward an actor system for thin-edge, this document | ||
| * explores a design on top of tasks and channels, | ||
| * and compares this design with actix, | ||
| highlighting what would be provided out-of the box, easier or more complex. |
There was a problem hiding this comment.
Nothing has been done to compare this proposal with actix.
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
cbb2d5d
to
a122b32
Compare
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
a122b32
to
633a2fe
Compare
didier-wenzek
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Robot Results
Passed Tests
|
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
633a2fe
to
687a38b
Compare
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
8fed5f0
to
d411d6e
Compare
didier-wenzek
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
4927019
to
8b6be6f
Compare
|
Just pushed a fixup to apply a patch from @reubenmiller to fix markdown formatting issues. |
didier-wenzek
had a problem deploying
to
Test Pull Request
GitHub Actions
Failure
reubenmiller
approved these changes
Jun 9, 2023
albinsuresh
reviewed
Jun 9, 2023
| * react to messages by updating their state, by sending messages to other actors, and possibly by spawning new actors. | ||
|
|
||
| All the interactions between actors are materialized by messages. | ||
| * Synchronous method invocations are replaced by asynchronous message exchanges. |
There was a problem hiding this comment.
Completely replacing synchronous invocations isn't fully realistic, right? Which we learned from our own experience with C8yHttpProxyActor. Although, we can argue that the message exchanges are still asynchronous and we're using a synchronous wrapper built over it.
There was a problem hiding this comment.
What I mean is that there no direct method invocations between entities managing a state.
- Without actors the code of the callee is executed in the thread of the caller. The caller has to wait for an exclusive access on the callee and then to wait the response from the callee.
- With actors, the callee process the request in its own thread. The caller can send requests without exclusive access and can but is not force to await the response of the callee.
| * An actor's peer simulator can be as simple as a pre-registered stream of messages, | ||
| or as sophisticated as an error injector. | ||
| * In a pure actor model setting, all interactions with the system are done via actors, | ||
| and can be simulated, including the clock and the file system. |
There was a problem hiding this comment.
How about Composability as well? As this model lets us bundle feature sets in different shapes and sizes?
There was a problem hiding this comment.
This is what is listed under Flexibility. I agree that both applies.
|
I fixed all the grammar errors highlighted by @albinsuresh and applied most of his suggestions. |
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
See: thin-edge#1622 Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
- Why actors? - Why yet another framework? - Key design ideas - Requirements - Key design issues - Appendix: A taste of using actors in Rust Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
- Document the main design decision - Detail key design aspects - Remove things that have been implemented differently - Remove the example from the appendix Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
didier-wenzek
force-pushed
the
specs/plugin-design.md
branch
from
c5f82e8
to
63fa22b
Compare
didier-wenzek
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
albinsuresh
approved these changes
Jun 9, 2023
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.
Proposed changes
Rational and discussion on how to implement and use the actor model in the context of thin-edge.
Here is the rendered document
Types of changes
Paste Link to the issue
#1407
Checklist
cargo fmtas mentioned in CODING_GUIDELINEScargo clippyas mentioned in CODING_GUIDELINESFurther comments