Component architecture

[palango]

The reason I'm discussion this here is that @hackaugusto said the pattern presented is used in Raiden as well and causes irritation there. Also @andrevmatos mentioned the same problem in a call about the transport layer.

Problem in PFS

Currently the design of the BlockchainListener (BCL) is that it polls for new events in an internal loop and then pushes these events to every registered callback function.
In the PFS the BCL is owned by the PathfindingService class. This results in cycles in the object graph at runtime because the PFS registers callbacks in the BCL.

A different approach would be to remove the callback based approach and use a pulling-based approach in which a function get_events() -> List[Event] would be called that returns a list of events since the last invocation.

Solutions?

In the PFS this doesn't feel like a problem yet but I can see the general problem. Does anyone know about useful patterns for this problem?

[pcppcp]

I don't see any problem with this approach, as long as Blockchain is indenpendent entity (runs as greenlet, only runs callbacks and its state is not changed by PFS). Sure, you could have some sort of message passing interface where you abstract callbacks into i.e. a gevent queue and wait for incoming events in the PFS loop, but IMO it is better to keep things simple.

[hackaugusto]

The primary problem I am talking about is the push/push model used for IO, this is just not how non-blocking IO works (it is, however, how asynchronous IO works, but I would translate an async model to a non-block instead of the other way around). What we end up having is a non-blocking system, that works primarily with pooling, that is converted to a callback based system, to then be converted to a single-thread react based system. This is just too convoluted and unnecessary, and this is the not simple version of code for a non-blocking first api.

Here is an example of how selectors does it:

while True:
    events = sel.select()
    for key, mask in events:
        callback = key.data
        callback(key.fileobj, mask)

Here is a simple example of curio does it (trio does pretty much the same):

async def echo_client(client, addr):
    print('Connection from', addr)
    while True:
        data = await client.recv(100000)
        if not data:
            break
        await client.sendall(data)
    print('Connection closed')

The data is not pushed by the IO layer to the application, the application itself pulls the data. That is how non-blocking IO works, and IMO this is the best model to write the application (an asynchronous interface can be made compatible to a non-blocking interface, the data pushed to the application is pushed to a queue, the queue is then polled, there is a trade-off here, where the app is either pull based or push based).

With the pull/push model, there is no need install callbacks, because the transport layer is inactive. What happens here is that the transport layer provides a poll method, that is used by the app when cpu is available.

The cycle in the object graph is a consequence of the push/push model, because the app needs a reference to the transport to push messages to be sent, and the transport needs a reference to the app to push received messages.

On the cycle itself, I don't have a strong argument against it, after all, at the end of the day, one model can be converted to the other, so they have equal power. However, I do think that pull/push is simpler than push/push.

[LefterisJP]

For what it's worth I also think a pull/push approach here would be cleaner.

@jozef you mentioned you want to keep things simple.

Don't you think that having something like what @palango proposed get_events() -> List[Event] to poll events since last invocation is simpler?

[andrevmatos]

I've read a little on this subject on the weekend, as well as trio and curio documentations. IMHO, this pull/push pattern is cleaner only if you have a easily centralizable event handler/loop, that can p[uo]ll for events and push/call the right method for it. When you have multiple possible data/event/action entrypoints, like Raiden and friends (blockchain, rest endpoints, peers' messages), you'd end up with multiple while True loops in different parts of the code and/or a centralized loop which needs to know the business logic of almost every component it touches.
Although cyclic dependencies in the runtime object graph may initially be undesirable, I think it's only bad if you have it explicit. I'd rather have a strongly enforced separation between interface responsibilities (components/classes that knows only about theirselves and their direct children interfaces), and communicates upstream through events/callbacks (even being push/push at runtime). Whoever registered this callback needs to know the object/event interface/arguments, not the emmiter itself, it only emits the event/call the callback and forget..

  parent object
 event ⇅  method call
  child object

Considering we code for human beings to read, not machines, I'd push for a stronger decoupling in the interfaces over avoiding cyclic (implicit) references in runtime object graph.