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

Relayer Concurrency Architecture #121

Closed
4 tasks done
ancazamfir opened this issue Jun 30, 2020 · 7 comments
Closed
4 tasks done

Relayer Concurrency Architecture #121

ancazamfir opened this issue Jun 30, 2020 · 7 comments
Assignees
Labels
I: logic Internal: related to the relaying logic
Milestone

Comments

@ancazamfir
Copy link
Collaborator

ancazamfir commented Jun 30, 2020

Summary

Describe and implement the relayer concurrency architecture for first release of the Relayer.

Problem Definition

There are a few potential threads required by the event based relayer, global, per chain and/ or per relaying path. In general congestion or failure in the communication with one chain should not affect the communication with other chains and for unrelated paths.
The issue should be addressed with a minimal proposal, improvements will be added in the next releases.

Proposal

Here is one proposal, final decision and implementation may look differently.
relayer_concurrency

config executor - single thread that runs upon start and triggers initiation messages for objects included in the config file. These are disjoint from the chain IBC events, something like ConnOpenStart or ChannOpenStart that could potentially cause the relayer to build and send ConnOpenInit or ChannOpenInit messages to the chain. It should work even these events are received by the event handler in the same time with the live chain IBC events. In other words, no synchronization with starts of other threads should be required.

chain monitor - one thread per chain that registers with the chain and forwards the notification messages to the event handler. Currently the relayer registers for Tx and Block notifications. It then extracts the IBC events from the Tx and generates a "NewBlock" event also for the block. Note that the notification may include multiple IBC Events. Also they arrive before the NewBlock event.

chain querier - these should be short lived threads that block on ABCI queries and send the response back to event handler. Or maybe this is per chain thread in first release? Event relayer should not block forever on full channel to this thread.

chain light client - one thread per chain that receives relayer requests, downloads and verifies headers, and returns the minimal header set in the response. In addition this thread should ensure latest locally stored state does not expire, i.e. when some timer fires it fetches and verifies the latest header. Timeout should be a fraction of the trusting period. Timer should be reset by the on-demand relayer requests.

chain tx - per chain thread that submits IBC messages and packets to the chain. Some sort of confirmation is expected but we need still to clarify if and how the relayer will act on this, or waits for an IBC event for confirmation, or times out.

relayer thread - one global thread that includes the:

  • event handler that:
    • deconstructs the IBC event vector from the monitor and sends individual events to the message builder
    • receives responses from queriers and light clients, and forwards them to the message builder
    • receives requests from the message builder and forwards them to the appropriate querier, light client or tx threads
  • message builder - runs within the same context of the event handler. There is one per builder object, i.e. instances of connections, channels and packets. It implements an FSM that triggers new queries as required, deals with "conflicting" events caused by race conditions. Note: the CPU load is minimum and will probably be fine even when we add the (succinct) proof verification later. The IO performed by the event handler is also minimal as it involves only inter-thread communication...again if no blocking channels. Otherwise we revise.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir
Copy link
Collaborator Author

Some thoughts after the discussion today about the relayer concurrency and concerns about its complexity ^. Here is one possibility maybe, collecting from bits of conversations and suggestions today:

  • event handler starts a message builder thread for each object associated with an event

  • event handler keeps track of all events that have started builder threads

  • the builder thread is a "blinker" thread, it is oblivious to everything that happens while it runs:

    • it runs the sequential synchronous relaying algorithm,
    • blocks on everything, queries and light client requests. The LC algorithm runs within the builder thread context.
    • if and when it finishes, it sends the output transactions to the event handler. So it should be started with a send channel or some handle configured
  • in the meantime, event handler determines upon handling of new events, if these should result in some of the builder threads and their outputs to be invalidated.

  • the event handler may kill a thread (can we do this easily?) or let the invalid thread finish and just discard its output transactions

  • the event handler will also decide when to re-spawn threads for the same events that triggered the just-killed thread in the first place.

Regarding the scale numbers for the active builders, it should be naturally proportional to the number of channels. Let' say in a block at height h we receive a number of transaction notifications. Few of them may be connections and channel events, but most will be packets events. One builder is created for each of the connection and channel distinct objects. For packets, we group them per channel and we send all packets belonging to a channel in a single transaction, therefore one thread for each channel to deal with the packets. Many of you mentioned relaying per "path" between two chains. I think we get something close. Of course, worse case each packet is for a different channel and we may end up with hundreds or thousands of threads. Also these may live into the next block that's coming with more TX-es. There are things we could do but I think we shouldn't worry about it for the first release.

We should still think the details through but at first look:

  • builders are simpler
  • no need for querier and light client threads (*)
  • we keep the monitors the way they are (well, this was the case before, I think we are fine with what is there)
  • the complexity of the event handler is similar as most of the logic is the same with what we were thinking for the async case. With the previous proposal the event handler would still delete message builders in some cases the only difference was that the message builder was able to make some recovery on its own (like in the famous case of on-chain client advancing beyond some planned updated). Now this would need to be handled by the event handler.

(*) we need some background LC thread to keep latest headers within trusted period. Also the event handler will still need to keep the on-chain client states within trusted period. This is something we haven't discussed today but I think it's a minor issue and we can minimize the work here by just doing updates from the LC thread even if some might be redundant.

@ebuchman
Copy link
Member

ebuchman commented Jul 6, 2020

My understanding of the complexity in the first diagram is that it's hard to see complete relayer flows (eg. receive event->query state->query lightclient->tx etc.) by looking at the EventHandler - we'd need to build a ton of little state machines in the EventHandler to track/execute those flows clearly. In the end, it seems this would be a prime candidate for async/await - we'd be able to code complete relayer sequences directly, so it'd be easy to see the flow, and it would all execute async. This might be what we ultimately want to work towards, but probably not where we start.

Re the new proposal:

event handler starts a message builder thread for each object associated with an event

I wasn't really sure what was meant by "object associated with an event" here. Can you clarify what we would create a thread for and how often?

One idea that might be extremely simple would be to start with 2 long-lived synchronous threads for each pair of chains A and B, one relaying from A to B, the other from B to A. For each chain we're subscribed to, we would route its incoming events into queues for each other chain, and have one thread reading off each queue and doing complete relayer flows. Something like:

Blank Diagram (1)

I think this would eliminate the need for a central Event Handler thread altogether

The Event Router would manage some internal state so it can figure out which events are for which chains. It may need to query the chain to figure out for eg. clients/conns/channs it doesn't know about yet. And each relayer thread from A->X would need to be able to query A's light client.

We'd then have one synchronous thread handling all logic for relaying from chain A to chain B. So we'd never race with ourselves eg. on client updates. Conceivably, for each block on chain A, we could submit one tx to chain B with msgs for all relevant events in that block.

@ancazamfir
Copy link
Collaborator Author

My understanding of the complexity in the first diagram is that it's hard to see complete relayer flows (eg. receive event->query state->query lightclient->tx etc.) by looking at the EventHandler - we'd need to build a ton of little state machines in the EventHandler to track/execute those flows clearly.

We build one state machine with < 10 states. With one function per state. This should cover all connection, channel, packets handling

In the end, it seems this would be a prime candidate for async/await - we'd be able to code complete relayer sequences directly, so it'd be easy to see the flow, and it would all execute async. This might be what we ultimately want to work towards, but probably not where we start.

Not sure how it would look with async/await. We would need to handle incoming events and future cancellation.

I wasn't really sure what was meant by "object associated with an event" here. Can you clarify what we would create a thread for and how often?

Potentially for every IBC event in a block we would create a thread, similar on how we create a "builder message" object in the initial proposal. Packet events belonging to the same channel are grouped to create a single thread/ builder. You don't need to know the destination chain so all logic and queries are done from the thread/ builder.

The Event Router would manage some internal state so it can figure out which events are for which chains. It may need to query the chain to figure out for eg. clients/conns/channs it doesn't know about yet.

This is a problem as it would block the Event Router.

We'd then have one synchronous thread handling all logic for relaying from chain A to chain B.

This may sound appealing in this one sentence. But it will still have to deal with timeouts, errors and retries. In the Go Relayer there are go routines spawned and things are retried a number of times between sleeps.

So we'd never race with ourselves eg. on client updates.

For connection, the A->B thread may need to update the client on A. And of course there would be races with other relayers.

@adizere
Copy link
Member

adizere commented Jul 6, 2020

Trying to clarify my own thoughts, hope this will help others as well.

Maybe the most important part worth understanding is that we are discussing here two orthogonal problems:

  1. find the trace of logical steps (or the relayer flow) that a relayer must do, for every event, depending on that event;
  2. schedule and execute these steps, i.e., assign steps to components, as well as their lifetime and error handling.

Takeaways so far:

  • At a very high-level, Anca's first suggestion proposes a clear separation between the two problems: the message builder would encode everything related to problem (1), while event handler handles problem (2). Intuitively, the message builder emits deterministic steps, such as "query for X" or "do an RPC for Y" (does not execute any step so it non-blocking, purely functional!), while the event handler schedules these steps, by assigning each step to specific components (e.g., light client or chain querier, which execute the step). Once the result of a step comes back, the event handler feeds that result into a message builder, which may emit follow-up steps, and so on.

  • Anca's second suggestion makes a compromise with respect to separation of (1) from (2), in the sense that the message builder would have more responsibility over handling these problems. In other words, a message builder knows the steps (this is point 1) and also executes the step (point 2). A message builder can do a almost-complete relayer flow (almost, except for the initial triggering of the flow), while the event handler trigger and may kill message builders. This a more "compact" design.

  • As I understand it, Ethan's suggestion takes the "compact" idea further. The complete relayer flow is now well-defined in a thread and there is a predefined number of threads that work in parallel. Each thread can tackle entirely problems 1 and 2, as each thread knows the full trace of steps and how to execute them, what to do upon error, etc. This is simpler, as it has fewer independent components, but more monolithic.

@adizere
Copy link
Member

adizere commented Jul 6, 2020

When it comes to moving forward, some simple questions that can guide us would be:

  • which codebase is easier to maintain? is it easier to maintain more, smaller, components that each do one thing (like in the first suggestion), or is it easier to mange fewer, larger, slightly more sophisticated components (like in the last two suggestions)? (I genuinely do not know the answer to this for Rust.)
  • if we're aiming to throw away this implementation eventually, then...
  • the engineering effort is smaller for the latter two suggestions, as there is less concurrency, less "management" to do, is this intuition correct?
  • testing seems easier when components are smaller, is that correct?
  • incremental improvements seem easier when components are more independent & smaller, correct?

@brapse
Copy link
Contributor

brapse commented Jul 7, 2020

I must apologize here for not providing a more precise recommendation that builds on the momentum here. After reading the ADR a few times and following along with the discussion here I still find it very difficult to understand the underlying motivation for the suggested approach.

It is very difficult to validate a concurrency design without identifying the bottlenecks and mapping a proposed design onto them. Specifically If our bottlenecks are IO and signature verification, those bottlenecks that need to be saturated for an “optimal” solution. However I suspect that those bottlenecks are not equally important, it could be that IO dominates performance before signature verification becomes a bottleneck. And so if we were to introduce progressive levels of performance (which I suspect we should) we might want to do IO concurrently first before we worry about signature verification.

Another component that I would look for in validating a concurrency design is a proposal of how it will make complex scenarios testable. I think it makes sense to start with a tested synchronous implementation to move with confidence towards a more performant implementation and ideally the test harness/cases would still apply. It isn't clear to me how any of the above would be tested and so to be frank, it's not really possible for me to consider it a good idea until that bit is clear.

So my regretfully broad recommendation here is identify the bottlenecks and at least form a credible hypothesis over their interactions along with a solid test regimen. And then frame the proposal in how it would solve those problems because that would provide us with the confidence we need to evaluate and make different tradeoffs.

@ancazamfir ancazamfir modified the milestones: v.0.0.3, v0.0.4 Aug 14, 2020
@ebuchman ebuchman modified the milestones: v0.0.4, v0.0.5 Aug 28, 2020
@ancazamfir ancazamfir modified the milestones: v0.0.5, v0.0.7 Oct 16, 2020
@adizere
Copy link
Member

adizere commented Nov 5, 2020

This is superseded by #326.

@adizere adizere closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: logic Internal: related to the relaying logic
Projects
None yet
Development

No branches or pull requests

6 participants