ibraheemdev / too-many-web-servers

https://ibraheem.ca/posts/too-many-web-servers/
98 stars 6 forks source link

Flow in non_blocking.rs is problematic #4

Open ncihnegn opened 1 year ago

ncihnegn commented 1 year ago

https://github.com/ibraheemdev/too-many-web-servers/blob/9cce25a523b0e251bee0fbb14693f98efc4e8eba/src/bin/non_blocking.rs#L23

Given a new connection, it will be put into connections in state Write and wait for the next connection. Until then, the response won't be returned. Better to process connections first, then wait for the next connection.

ibraheemdev commented 1 year ago

I'm not sure what you mean. Connections start in the Write state, and accept is non blocking.

ncihnegn commented 1 year ago

When I run this on macOS, the first request won't get response until there is another request.

jmevel commented 1 year ago

Hi @ibraheemdev ,

I'm following your blog post as well. Quite interesting but I also can't wrap my head around your logic in the Non-Blocking Server section.

In the way the algorithm is written there would only be a single item in the connections vector as well as in the completed one in my opinion. Here is the thing:

  1. The program will loop on match listener.accept() until it receives a request
  2. The connection is pushed to the connections vector in Read state
  3. We then move out of the match listener.accept(). At that point only one request has been added to the connections vector
  4. We start handling the request (reading, writing and flushing)
  5. We remove the request from the completed vector
  6. We are back to the match listener.accept() to handle the 2nd request

I tried to launch several requests in parallel with the seq 1 5 | xargs -Iname -P5 curl "http://localhost:3000" command and debugged it to see if I was missing something
I also tried to launch a single request, break inside the Ok arm of the match listener.accept() with a breakpoint then opened another terminal and sent a 2nd request. In both cases my debugging showed that the requests were handled one after the other, not at all in parallel.

So correct me if I'm wrong but I think your program is indeed non-blocking but the logic with the connections and completed vectors is actually useless. There will never be two items at the same time in these vectors.

Thanks for your answer 🙏 (a Rust beginner eager to learn)

ibraheemdev commented 1 year ago

We start handling the request (reading, writing and flushing) We remove the request from the completed vector

Handling the request is not a synchronous step. If every operation completed immediately, then yes, you are right. But read/write from a remote TCP connection will likely return WouldBlock before being able to complete, and so we can accept more requests and handle all requests that are still pending, only removing them once they complete.

jmevel commented 1 year ago

We start handling the request (reading, writing and flushing) We remove the request from the completed vector

Handling the request is not a synchronous step. If every operation completed immediately, then yes, you are right. But read/write from a remote TCP connection will likely return WouldBlock before being able to complete, and so we can accept more requests and handle all requests that are still pending, only removing them once they complete.

I didn't have much free time for this recently but I must say it took me a while to understand the whole picture and why this is indeed handling requests in parallel. I had to print messages all over the code and run (not debug) it with many HTTP requests sent in parallel to be 100% sure of how it works 😅

Thanks for your answers. I'll now be able to read the next section of your post !

ziyouwa commented 3 months ago

Hi, @ibraheemdev I'm following your blog to learn about Rust asynchronous programming. In the non-blocking example, when I connect to the server using curl https://localhost:3000, it alternates between success and failure. According to the logs, once the Read state enters WouldBlock, if there is only one connection in the connections, the entire next loop exits, causing unfinished reads to be stuck.

ibraheemdev commented 3 months ago

@ziyouwa the 'next loop can exit, but the outer loop should come back around to the connect again?

ziyouwa commented 3 months ago

@ziyouwa the 'next loop can exit, but the outer loop should come back around to the connect again?

I apologize for the confusion.

        loop {
            match listener.accept() {
        ...
            Err(e) if e.kind() == io::ErrorKind::WouldBlock => {
                    // println!("main loop received WouldBlock");
                    continue;      👈  It was the cause of the problem.
                }
ibraheemdev commented 3 months ago

Ah, that's addressed later in the blog post, right? Is there a problem with the final version in non_blocking.rs?

ziyouwa commented 3 months ago

Ah, that's addressed later in the blog post, right? Is there a problem with the final version in non_blocking.rs?

Your code is fine, it's my problem. Also, this blog post is excellent, I've translated it into Chinese, and the link at: https://blog.windeye.top/rust_async/learningrustasyncwithwebserver/ I'm very sorry for not obtaining your consent in advance. Please notify me to remove it if there are any trouble.