add async support to tls_fetcher/proxy and qos #524

Turnalek · 2025-05-14T19:25:59Z

Summary & Motivation (Problem vs. Solution)

Implement async qos_net::AsyncProxy
this includes:
- Reaper::execute_async [qos_core]
- Stream [qos_core]
- AsyncPool/Stream [qos_core]
- Listener [qos_core]
- RequestProcessor [qos_core]
- ProxyConnection [qos_net]
- Proxy [qos_net]
- ProxyStream [qos_net]
- HostServer [qos_host]

as well as fixing qos_next's qos_core "vm" feature dependency issue by only requiring it if qos_net is requested with it, adding new integration tests and fixing qos_test_primitives shutdown handling on unix (SIGINT)

New dependencies:

[email protected]
[email protected]
[email protected] -> was added in some places but we already used it before

How I Tested These Changes

Locally and in preprod.

Pre merge check list

Update CHANGELOG.MD

src/qos_core/Cargo.toml

emostov · 2025-09-05T19:07:54Z

src/qos_test_primitives/src/lib.rs

+
+			// allow clean exit
+			std::thread::sleep(Duration::from_millis(50));
+		}


I'm surprised child.kill doesn't do this for us

What does the sleep do

Unfortunate that send signal is nightly https://doc.rust-lang.org/stable/std/os/unix/process/trait.ChildExt.html#tymethod.send_signal

it's just to ensure the drops are done on the running child. If we just SIGKILL the children won't cleanup.

src/qos_nsm/src/nitro/error.rs

emostov · 2025-09-05T19:59:06Z

src/qos_core/src/reaper.rs

-				test_only_init_phase_override,
-			);
-			SocketServer::listen(addr, processor).unwrap();
+			tokio::runtime::Builder::new_current_thread()


We talked about on our call - we will spawn this as a new task. And remove the std thread and new tokio runtime

I think I'll do this on a follow up PR along with Reaper refactor.

src/integration/examples/boot_enclave.rs

emostov · 2025-09-08T16:06:04Z

src/integration/examples/boot_enclave.rs

What is the goal with adding this example? Shouldn't this be almost the same boot e2e test?

It's for local testing, so you can spin up a full local enclave that's properly booted and then feed it whatever. Makes it easier than having to write a specific test, it's also "live" so easier to debug.

emostov · 2025-09-08T18:41:49Z

src/integration/tests/interleaving_socket_stress.rs

+	);
+
+	// do a bunch of timing out requests at the same time, this still needs to fit within the pool size
+	for _ in 0..3 {


what would happen if we did more then the pool size (>5)

src/integration/tests/qos_host.rs

src/integration/tests/remote_tls.rs

src/qos_core/Cargo.toml

emostov · 2025-09-08T19:17:21Z

src/qos_core/src/cli.rs

-				None,
-			);
+			// start reaper in a thread so we can terminate on ctrl+c properly
+			std::thread::spawn(move || {


we can put this into a tokio task, right?

yes, once Reaper is refactored

src/qos_core/src/io/pool.rs

r-n-o

First read through! Overall no major concerns, the only "big" thing I'd like to push for is getting --pool-size out of the pivot args now rather than later. I think it'll be easier to do before this lands and gets deployed in mono. My other comments are all small stuff!

src/init/init.rs

src/init/Cargo.toml

r-n-o · 2025-09-10T00:12:09Z

src/qos_core/src/reaper.rs

+// find the u32 value of --pool-size argument passed to the pivot if present
+fn extract_pool_size_arg(args: &[String]) -> Option<u32> {
+	if let Some((i, _)) =
+		args.iter().enumerate().find(|(_, a)| *a == "--pool-size")
+	{
+		if let Some(pool_size_str) = args.get(i + 1) {
+			match pool_size_str.parse::<u32>() {
+				Ok(pool_size) => Some(pool_size),
+				Err(_) => None,
+			}
+		} else {
+			None
+		}
+	} else {
+		None
+	}
+}


relates to my earlier comment about awkwardness of this being a pivot arg...let's make this a proper manifest field and this can go away! (I think it's worth it: it's easy enough to change now, but if this ships as-is I think the change becomes more daunting)

I'm mostly concerned with keeping this alive longer. If we do this change we have to retest the full stack again and keeping both this and the mono PRs up is becoming really hard due to all the related changes from main.

src/qos_net/src/proxy.rs

src/qos_net/src/proxy_connection.rs

src/qos_net/src/proxy_stream.rs

src/qos_test_primitives/src/lib.rs

src/Cargo.lock

r-n-o · 2025-09-11T17:58:43Z

src/qos_core/src/client.rs

 	}
+
+	/// Expands the underlaying `AsyncPool` to given `pool_size`


(typo) underlying

r-n-o

Some followups left to address but I think this PR is in a great spot already! It'll be easier to review and address the rest in separate, smaller PRs. Let's get you out of rebase hell! 🚀

Turnalek mentioned this pull request May 14, 2025

[WIP]: add async support to tls_fetcher/proxy and qos #517

Closed

1 task

Turnalek force-pushed the ales/async_proxy branch 4 times, most recently from 85d5895 to 3667ab4 Compare May 15, 2025 20:56

Turnalek commented May 15, 2025

View reviewed changes

src/qos_core/Cargo.toml Outdated Show resolved Hide resolved

Turnalek force-pushed the ales/async_proxy branch 10 times, most recently from 7aeca11 to 65672b2 Compare May 27, 2025 19:30

Turnalek force-pushed the ales/async_proxy branch 6 times, most recently from 6810191 to f4d01f0 Compare June 3, 2025 19:05

Turnalek marked this pull request as ready for review June 4, 2025 21:47

Turnalek requested review from a team as code owners June 4, 2025 21:47

Turnalek force-pushed the ales/async_proxy branch 5 times, most recently from f372272 to b55c07f Compare June 11, 2025 16:30

emostov reviewed Sep 5, 2025

View reviewed changes

src/qos_nsm/src/nitro/error.rs Outdated Show resolved Hide resolved

emostov reviewed Sep 5, 2025

View reviewed changes