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

Multithreaded SocketServer #106

Closed
wants to merge 10 commits into from
Closed

Multithreaded SocketServer #106

wants to merge 10 commits into from

Conversation

emostov
Copy link
Contributor

@emostov emostov commented Aug 5, 2022

Closes #33

This PR converts the SocketServer to supporting async requests by implementing a thread pool and processing each new socket connection on a new thread.

For context, our communication model creates a new connection for each request/response, thus for each request/response we create a new job on a thread and the job finishes once the request is returned.

Why make the server async

  • Do not block on 1 request. For example we may want to query the enclave health while a proxy request to the secure app is still processing.
  • Take load off the socket by minimizing connection queue size - the OS can handle a finite of requests to connect. The more requests the application layer (think socket server) can accept, the more we can reduce rejected connection requests

Given these goals, pure performance is not the main concern but instead we mainly want to focus on making ours server non-blocking while aligning with our security goals of robust and easy to audit code.

Why a thread pool?

When I set out I originally intended to use async Rust code, which does not necessarily imply multithreading but just that code can be executed in a non-blocking way. In order to run async code in Rust you need an executor (example), which is not provided by the standard library. Note that executors can be both multithreaded and non blocking on a single thread.

There are several popular executors, but the most popular today is probably: https://github.com/tokio-rs/tokio, which implements an async runtime. From what I understand the internals of tokio is quite complex and appears to be a significant review burden.

There are some other options for executors, the simples of which appears to be the futures crate thread pool executor. futures is an extremely popular crate as it provides many of the shared primitive types used across async runtimes in the Rust ecosystem. However, my thinking here is that we can avoid futures all together by implementing our own thread pool.

Relevant reading

qos-core/src/cli.rs Outdated Show resolved Hide resolved
qos-core/src/io/threadpool.rs Outdated Show resolved Hide resolved
@emostov
Copy link
Contributor Author

emostov commented Aug 17, 2022

Some raw notes from an offline discussion with @r-n-o:

  • We want it to respond with a reject in the socket client if the queue is full
    • Ideally we can do this at the socket level
  • The thread pool needs to not take things off the socket if its overloaded
    • this would cause silent failure

@emostov emostov marked this pull request as draft August 18, 2022 15:41
@emostov emostov deleted the branch master December 22, 2022 00:35
@emostov emostov closed this Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make enclave server async
1 participant