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

Don't block thread when making new connection in BamClientPool #319

Closed
LLFourn opened this Issue Oct 8, 2018 · 2 comments

Comments

Projects
4 participants
@LLFourn
Copy link
Contributor

LLFourn commented Oct 8, 2018

Given we are using a non-blocking web framework now we shouldn't block the controller threads. However, the comit_client::ClientFactory::client_for method currently blocks when it has to create a new connection. It should instead return a Future for that connection.

It should also keep track of when it is already connecting to something and return a future which resolves when the existing connection attempt completes.

Hint:
Use ItemCache to return/create a future for a given COMIT node. The future Item is a Arc<ComitClient>

DoD:

  • client_for returns a future

@LLFourn LLFourn added this to To Refine in Swap Beta via automation Oct 8, 2018

@LLFourn LLFourn added this to the Open-Source milestone Oct 8, 2018

@thomaseizinger

This comment has been minimized.

Copy link
Member

thomaseizinger commented Oct 8, 2018

Maybe we can check existing implementations of HTTP client libraries in how they do this.

@LLFourn

This comment has been minimized.

Copy link
Contributor Author

LLFourn commented Oct 15, 2018

@thomaseizinger great idea. I took a look at hyper: https://github.com/hyperium/hyper/blob/master/src/client/pool.rs

It does roughly what we discussed (I think):

  1. Upon new client request, record that you are in the connecting state for that connection start connecting.
  2. Return a oneshot::Receiver to the caller and store a oneshot::Sender in HashMap<IpAddr, Vec<Sender>>.
  3. Any new connection requests for the same host get returned a oneshot::Receiver as well
  4. When the connection is established and the client is created iterate over the oneshot::Senders and send a .clone() of the Arc<Client> to them.

The problem that HTTP clients are solving is a little different I guess because at least with HTTP 1.1 you have to wait until idle to get the connection where as with our thing we can just give everyone clients at the same time without issues.

@bonomat bonomat removed this from the Open-Source milestone Oct 29, 2018

@D4nte D4nte modified the milestone: TenX Xmas Summit Demo Nov 7, 2018

@D4nte D4nte assigned D4nte and unassigned D4nte Nov 14, 2018

@D4nte D4nte added the icebox label Nov 22, 2018

@LLFourn LLFourn referenced this issue Jan 16, 2019

Closed

Implement the noise handshake protocol #668

2 of 2 tasks complete

@LLFourn LLFourn added work-in-progress and removed groomed labels Jan 21, 2019

@LLFourn LLFourn self-assigned this Jan 21, 2019

@D4nte D4nte added this to the Sprint 7 milestone Jan 23, 2019

@D4nte D4nte added review and removed work-in-progress labels Jan 29, 2019

@LLFourn LLFourn closed this in #688 Feb 1, 2019

@wafflebot wafflebot bot removed the review label Feb 1, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.