Implement sync-layer that acts as a proxy for function calls and local/api mutations

[heartsucker]

I should have posted my notes last night after talking to @joshuathayer, but in short we are considering using an actor-model sync layer, possibly based off his prototype.

The idea would be to use the actors as publishers / subscribers to an internal message bus instead of using callbacks. Each actor would handle a specific message type such as SendReplyRequst and SendReplyResponse. The calling function would push a message on to the bus and then listen for relevant responses. This would allow us to isolate each widget's state and allow a widget to control how it looks. It would also isolate the API/DB state away from the rest of the system, with this only being handled in the sync layer.

[ntoll]

Some thoughts / devils advocacy. ;-)

• Has this already been done already in PyQt? (If not, why not? My impression is that the callback style is more Qt-y IYSWIM.)
• How does this improve what we already have? Our application is NOT very complicated, and I can't help but feel like this is a case of YAGNI.
• In the Zen of Python we have "explicit is better than implicit" and this "feels" (again, note the quotes) like we're adding a layer of indirection (add a message somewhere / handle a message from somewhere) rather than just explicitly calling things. I imagine this could make it hard[er] to debug things.
• How does this improve on simple use of QT's signals/handlers?

</devilsadvocacy> :-P

I'm +/-0 on this since I'm not sure what the benefits may be for what should be a very simple application and equally simple implementation.

[heartsucker]

This is my proposed architecture for concurrency, shown using a subset of features of the app.

Summary

In very short, using actors with a bus.

In less short, we have one major message bus where all the messages are dumped. Messages are directives that might call for an interaction with an external resource (API, DB, FS, Qubes) or a mutation of the UI. Messages are typed (Python objects subclassing Message) and may contain UUIDs so that if multiple "downstream" actors can respond to the same message and upstream actors can "merge" these responses.

   ______________________________________
  /                                      |
+------+ +------+                        |
| Star | | Flag |>_______________________|
+------+ +------+                        |
  ^        ^                             |
  |   _____|                             |
  |  /                                   |
+-------+      +-------+                 |
| CL. 1 |      | Reply |>________________|
+-------+      +-------+                 |
   ^              ^                      |
   |              |                      |
+------------+ +---------+ +---------+   |
| Conv. List | | Conv. 1 | | Conv. 2 |   |
+------------+ +---------+ +---------+   |
    ^             ^          ^          /
    |  ___________|__________|         /
    | /                               /
  +----+                             /
  | UI |                            /
  +----+                           /
    ^                             /
    |                            v
  +-------------------------------------+
  |                Bus                  |
  +-------------------------------------+
       | ^      | ^
       v |      v | 
     +----+   +-----+
     | DB |   | API |
     +----+   +-----+

In the diagram above, we can see that the UI elements only have to maintain a reference to the Bus and shoot their messages off there. Their parents will attach to the bus (to to a parent that attaches to the bus) and request message of certain types. The bus will relay matching messages to these actors (roughly pub/sub).

Advantages

This simplifies the logic so that actors only have to be able to fire to the bus or their direct descendants. There is no deeply intertwined referencing.

This also has the advantage of not splitting __init__ / setup and allowing less mocking during tests. We already mock a lot, and for these tests we won't have to mock so many external features, but only the Bus and then check that it received a message.

Actors is a simpler pattern than nested callbacks.

This allows us to ensure strict ordering of replies (one actor per ReplyBoxWidget, or downstream one actor for all messages).

Disadvantages

We have to make sure actor references are cleared on widget destruction to ensure that we don't leave dangling actors everywhere.

We have to sort out something with like QtTimers to ensure snappy processing of messages.

There is no clean stack for debugging (but also messages are self contained, so there's less external dependencies).

Rationale

The arguments against this have been that the SDC is a small, simple app, but even pre-alpha we are already seeing a bunch of concurrency related issues, and I want to avoid the "oh jeez we actually need a mutex/lock here because...". Once we hit that point, we won't back things out and rearchitect, but we'll just wedge it in.

Problems So Far

• Multiple requests for same document
• Strict order of messages in replies (per-workstation)
  • (seriously, IMO this is a 100% absolutely must have for beta)
• Simultaneous sync and {reply,click,delete}
• Tracking open current DispVMs
  • current suggestion is to use the DB, but this count isn't persistent

However, the biggest problem I see with the current architecture is the degree to which things are mocked for simple tests. I think this architecture will significantly decrease the amount of mocking we have to do.

Example Workflo

• App boots
  • Creates BusActor
  • Creates DbActor
    • Attach to BusActor
  • Creates ApiActor
    • Attach to BusActor
  • Creates UiActor
    • Attach to BusActor
    • Creates ConversationListActor
• User clicks on a converstaion
  • CoversationActor is created with a source / source UUID
• User sends a reply
• ConversationActor.publish(SendReplyMessage("foo"))
• Concurrently we get these on the SendReplyMessage
  • ApiActor.recv
    • hits the API
    • publishes success/failure message
  • DbActor.recv
    • Updates the DB
  • UiActor.recv
    • Updates the UI
• On API success/failure, a message hits the bus and tells the DB to "unsend" and the UI to show an error

[heartsucker]

Also as a note, @joshuathayer did some work setting up actors to use Qt's builtins so that it would a Qt-native actor system.

[ntoll]

As you'll see from #128, I'm totally sold on this. ;-)

@heartsucker the example workflow makes sense to me. I took some time to take a quick look for PyQt / native Qt actor / message bus type implementations and turned up nothing useful, so it looks like we're doing this ourselves and @joshuathayer's initial spike would be a good place to start.

For the rest of today (Thursday) and tomorrow I'm not on SD things, but I'll keep tabs on the discussion around this ticket.

[heartsucker]

It seems that using actors in Qt is definitely "a thing", as noted here: https://github.com/actor-framework/actor-framework/tree/eb1cc7b1e911cffcd7ea196cc8d30553db239000/examples/qtsupport

There are python bindings for it here: https://github.com/actor-framework/python

However, there may be issues with the maintenance of the lib as noted here: actor-framework/actor-framework#775

[redshiftzero]

there were definitely some very good ideas in here, but we ended up addressing many of the pain points that this is solving via:

• implementing queues for network actions (e.g. this enables us to ensure that replies aren't sent out of order).
• making better use of signals/slots to update UI elements across the application so that e.g. classes containing network logic (now an ApiJob) don't keep references to the gui.

So I'm going to close this.