nginx/ngx-rust

`async` executor on nginx event loop

Opened this issue · 5 comments

Previously, we had evaluated use of tokio within the modules. The results weren't reassuring:

nginx is optimized to be a multi-process single-threaded application. None of the internal APIs are thread safe, and it's extremely dangerous to call some methods from another thread (anything that deals with IO scheduling, timers or other globals). Rare cases of thread interaction (thread pools in aio threads) are not cross-platform and have bottlenecks. Additionally, the notification method from the thread pools (ngx_notify) is a private detail, not reusable without changing the API.

  • Any data sent to the tokio runtime should be fully copied and detached from the nginx internal structures (ngx_http_request_t). That is expensive.
  • Any events, including incoming body, generated response bytes, abnormal termination of the request, etc, must be passed between threads through a synchronized event queue. Said event queue should integrate with the nginx main loop using platform-dependent methods (eventfd, kevent, etc) or risk degrading performance even more.
  • Thread synchronization may cause delayed or blocking reaction on the nginx events. E.g. task cancellation when the request is aborted and the cleanup handler is running, must be synchronous.
  • Threads mess up CPU affinities, data locality, etc. That adds to the already non-negligible costs of copying the data.
    Ideally, we want to have everything in the main thread of a worker process, unless it's a blocking task that should be offloaded to the thread pool.

Considering all of the above, it appears to be safer and simpler for us to implement our own task executor running on top of the nginx event loop. The Rust ecosystem offers enough components and examples for that.

The MVP goal for this task is to be able to run hyper/client as an upstream implementation; that is a sufficiently complex use case for an async runtime.

This sounds really promising. right now we are shipping nginx with a tokio1 runtime in production for >5 years now and we are constantly finding leaks, segfaults, memory corruption. (one of) the reason we didn't integrate with the nginx event loop was that the async ecosystem wasn't really ready and most of the crates depend on an specific runtime. now situation is a bit better. for example hyper has already decoupled their runtime usage2. But (if i remember correctly) we do still need a few things from epoll to get exposed in nginx itself. anyways, i don't know the exact state of this work (i have only seen a few basic infra done) but i volunteer working on this if you want anyone.

Footnotes

  1. our integration is basically this that i've submitted a while back but with a few patches/optimization on nginx itself: https://github.com/nginx/ngx-rust/blob/main/examples/async.rs

  2. tokio rt support is provided in hyper-util crate. we can bring our own runtime like this: https://hyper.rs/guides/1/init/runtime/

Hi @sahandevs,

I’ve been working on a similar situation, and I noticed that the nginx-acme project actually has code that integrates the NGINX event loop with Hyper. That might be useful as a reference. Hope this helps!

https://github.com/nginx/nginx-acme/blob/main/src/net/http.rs

I believe this issue can be closed because ngx::async_ now provides an executor on the nginx event loop: https://github.com/nginx/ngx-rust/blob/main/src/async_/spawn.rs#L139

I am working on updating the examples to show this in #198 (caution, very broken at the moment)

We are currently trying to use ngx:async_ because the current (tokio) async.rs example seems to "busy-loop" by scheduling itself again over and over when waiting for outstanding io (https://github.com/nginx/ngx-rust/blob/main/examples/async.rs#L107) and mentions bad performance, and we wanted to avoid having to copy stuff in and out all the time, e.g. wanted to be able to pass &mut Request.

However, I see the following problems with current async_:

  • The Scheduler expects to be called from the main thread only, but as soon as I/O is involved, schedule will be called from other threads (necessarily, I believe?). I/O is the main reason we are looking towards async in the first place.
  • main loop is normally in epoll when I/O finishes. The examples mentions a "300ms timer", but I'm not getting that. epoll timeout is commonly -1 unless I keep some kind of noop timer scheduled myself, which I want to avoid. This means that tasks waiting on I/O will be blocked until something unrelated happens, like a new request coming in, which interrupts epoll.

@bavshin-f5 You mentioned ngx_notify, it seems perfect for this: It will interrupt epoll, and allows moving tasks to the main thread. What API changes would be needed? I was actually able to rewrite spawn using nginx_notify here: pschyska@52568f6

It seems to work for me, and solves the two issues I mentioned. I/O wakeups can come in from different threads now, and would interrupt epoll to promptly run the waiters. I've rewritten the example to concurrently issue a reqwest call, tokio sleep (both via async-compat, which we are using currently), ngx_rust sleep and ngx_rust resolver call, and some native futures yield and sleep "time-wasting" (https://github.com/pschyska/ngx-rust/blob/main/examples/async.rs#L190), just for some examples.

What do you think about that approach?

@bavshin-f5 Would you be able to review my PR? You seem like the right person from the previous discussions 🙂 I can't request reviewers on the PR due to lack of permissions.
We are using my fork in production since a couple of weeks and are very interested getting it upstream.
There are also ideas implementing tokio io traits properly instead of relying on async-compat, but I'd like to get feedback on the ngx_notify approach in my PR first.