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

Make all commands ASYNC (dispatch to worker thread pool) #43

Open
valeriansaliou opened this issue Mar 15, 2019 · 7 comments
Open

Make all commands ASYNC (dispatch to worker thread pool) #43

valeriansaliou opened this issue Mar 15, 2019 · 7 comments
Labels
enhancement Enhancement to an existing feature help wanted Extra attention is needed
Milestone

Comments

@valeriansaliou
Copy link
Owner

Currently, commands are processed synchronously in the channel thread. This limits parallelization on setups that open very few Sonic Channel instances, but where Sonic runs on a lot of CPUs. Currently 1 channel = 1 thread; but we'd like to make the channel protocol fully asynchronous and thus Sonic will be able to dispatch commands for work to a thread pool.

We just need to figure out how we can rework the protocol so that a RES for a REQ can be caught later on (eg. with a marker ID as we already do for search queries with PENDING?).

@valeriansaliou valeriansaliou added help wanted Extra attention is needed enhancement Enhancement to an existing feature labels Mar 17, 2019
@valeriansaliou valeriansaliou added this to the v2.0.0 milestone Mar 23, 2019
@ox
Copy link

ox commented Mar 26, 2019

Hi there! I've been casually building a client in Go and have sort of hit this wall. I'm stuck trying to figure out how to model my client to be able to receive arbitrary responses from the server and match them to the requests a user has made. I think the solution is to make "mailboxes" for the different responses I get from the server and match them to pending requests made by the user, but I'm not immediately sure that would work. For instance, 5 PUSH requests are quickly sent, the 3rd fails but the network routes the packet on a longer path, how can the client reliably say which request failed?

I think a user-provided marker ID method would make matching requests to responses much easier and more obviously correct. Maybe something like:

T1: PUSH messages user:0dcde3a6 conversation:71f3d63b "Hello world" REQ(foo)
T2: ... more traffic
TN: OK REQ(foo)

@valeriansaliou
Copy link
Owner Author

valeriansaliou commented Mar 26, 2019

Hi! What you could do is instantiate a global "register" HashMap where you take the ID Sonic returns you in the "PENDING" part, then store it in the HashMap (with your metas allowing for response tracking and routing to the proper Go function the user passed). Once you receive an "EVENT", you can simply extract the ID from the EVENT response, and check if it exists in your register HashMap. If so, you have your user-provided callback to execute. Does it make sense this way?

Notice that there's no ERR for the async protocol (ie. if an EVENT never comes due to an error). You may thus implement a timeout strategy on your end.

@ox
Copy link

ox commented Mar 28, 2019

Ahh yea that makes sense. I think my confusion stemmed more from forgetting that TCP is well-ordered, and that the response to a query will be the next bytes of the connection. I was expecting messages to be coming in at almost random. I'll change my client accordingly. Thanks!

@perzanko
Copy link
Contributor

@valeriansaliou Do 'pending operations' make anything asynchronously now? It seems to me that this ChannelCommandResponse::Pending is returned after the query is executed. Is this functionality not yet implemented?
ref: https://github.com/valeriansaliou/sonic/blob/master/src/channel/command.rs#L354

@valeriansaliou
Copy link
Owner Author

valeriansaliou commented May 12, 2019

@perzanko that's right, for now operations are synchronous on a per-TCP connection basis (this means that multiple open Sonic Channel connections don't block each other, they each have their "synchronous" thread). This was done for simplicity reasons for 1.0.0, although the Sonic Channel protocol already supports asynchronous commands by-design (thus it responds with PENDING and then EVENT). This way, we don't have to implement any protocol changes in Sonic libraries once the Sonic core is made fully asynchronous.

The current Sonic implementation responses "as if" it was working in an asynchronous way, though it's not.

@perzanko
Copy link
Contributor

May I take this issue on myself? I'd like to support you somehow! 🤓

@valeriansaliou
Copy link
Owner Author

@perzanko yes! You may 😄 Let me know if you need help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants