rpc: ADR for client event subscription mechanism

[thanethomson]

In partial fulfillment of #313 (implementation will close it).

I've gone through several mental iterations of what the various touchpoints for the event pub/sub interface could look like and I think I've pared it down to the simplest form I can come up with.

More scrutiny would be greatly appreciated! 😁

Rendered

---

• Referenced an issue explaining the need for the change
• Updated all relevant documentation in docs
• Updated all code comments where relevant
• Wrote tests
• Updated CHANGES.md

[liamsi]

rendered: https://github.com/informalsystems/tendermint-rs/blob/adr/subscription/docs/architecture/adr-008-event-subscription.md

[liamsi]

Simple and ergonomic interface 👍 I was first wondering if fn subscribe shouldn't directly return the stream (and unsubscribe would then take a Query too) but it seems having a Subscription type is actually more readable. Looking forward to the implementation :-)

Thanks for taking this over 👍

[romac]

I too very much like the interface 👍

I think it could be useful to sketch out the Subscription type to see how easily we can mock out the Client interface to construct a Subscription which emits a static list of events.

[xla]

Great write-up! The direction is sound, left some open questions from my side inline. I agree that we need to expand the design of the Subscription before we move forward.

[xla]

This is a bit vague, could we expand on this?

[xla]

This needs further detailing as to what the desired issue we like to address with buffered channels. Furthermore the behaviour needs specifying as to how the implementation should behave when it encounters a filled up/blocked channel.

[thanethomson]

The current proposal (and implementation) now makes exclusive use of unbounded channels. This doesn't do away with the need for discussion around handling blocked/full channels, but it buys us a bit of time.

[xla]

It ought to be the responsibility of the user/caller to spawn this "run loop". Otherwise we take away control about lifetime. Instead of having the client silently spawn things in the background it should be made explicit through its interface.

[thanethomson]

I've implemented a handle/driver architecture for most of the parts of the system that need to operate in two async contexts. The "handle" part allows for total control of the "driver" part.

Unfortunately, carelessly dropping the handle part right now with the current architecture still leaves the driver dangling (which will hopefully be more easily addressed when async destructors are available in Rust).

[ebuchman]

What about the ability to receive all subscriptions on a single stream? eg. if I want to subscribe to a bunch of distinct event types and receive them in order?

And would it make sense / simplify things for the base subscription system to be like that, ie. a single stream with all subscriptions, and have the multi streams (I guess the Router) be an optional layer on top?

[thanethomson]

My understanding of the initial reasoning around having Subscription implement the Stream trait is that it opens up possibilities around the use of more general futures functionality like select_all, which would allow your code to combine multiple subscription streams into a single one and iterate through them as one stream.

[ebuchman]

Interesting, and that would preserve order from the underlying socket? Does that make more sense than an approach where all events on the websocket itself are a single Stream and clients have option of partitioning that up into possibly multiple streams per subscription?

[thanethomson]

The docs for select_all claim that elements from the underlying streams are returned while preserving the order in which they are produced by the underlying streams. I can add some tests into #516 that verify this, because I haven't actually tested it yet.

In theory it should work like this. The WebSocket driver I've implemented in #516 pushes events out to subscriptions (into their internal tokio::mpsc buffers) immediately as they're received before trying to read another event from the WebSocket connection, so this approach should preserve ordering when using select_all.

[ebuchman]

Would be helpful if we also had context on what currently exists in the code base and what the issues are.

[ebuchman]

What's the motivation for this?

[thanethomson]

Since the client multiplexes multiple subscriptions on a single WebSocket connection, there are no request/reply mechanics. You send messages and you continuously receive them in any order (i.e. subscription request responses could be mixed in with incoming events related to a different subscription). For example:

1. The client sends a subscription request via the WebSocket connection. This creates a subscription in a Pending state, but it's not yet active (since the RPC can still respond with an error). We don't publish events to Pending subscriptions.
2. The client may, while waiting for a response, receive events for a different subscription (made earlier than (1)).
3. When a response comes back whose ID matches the request from (1), if it's successful, the subscription is marked as Active, and if it failed we cancel and remove it.

Similar dynamics happen when unsubscribing, so unsubscribing is also currently a two-stage process.

[ebuchman]

So when/how can we ever not use the two-stage subscription? ie when can we use the immediate one?

[ebuchman]

Let's merge this and follow up as necessary

Addressed