]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
OPTIM: stconn: Don't pretend mux have more data to deliver on EOI/EOS/ERROR
authorChristopher Faulet <cfaulet@haproxy.com>
Fri, 27 Sep 2024 13:05:11 +0000 (15:05 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 30 Sep 2024 14:55:48 +0000 (16:55 +0200)
Doing some benchs on the 3.0, we encountered a small loss on requests/sec on
small objects compared to the 2.8 . After bisecting the issue, it appeared
that this was introduced when the mux-to-mux zero-copy data forwarding was
implemented in 2.9-dev8. Extra subscribes on receives at the end of the
message were responsible of the loss.

A basic configuration, sending H2 requests to a H1 server returning
responses without payload is enough to observe the issue. With the following
command, we can observe a huge increase of epoll_ctl calls on 2.9/3.x:

  h2load -c 100 -m 10 -n 100000 http://...

On 2.8 we have around 3200 calls to epoll_ctl against more than 20k on 3.1.

The fix seems obvious. After a receive, there is no reason to state a mux
have more data to deliver if EOI/EOS/ERROR flag was set on the
stream-endpoint descriptor. With this change, extra calls to epoll_ctl
disappear. However it is a sensitive part so it is important to keep an eye
on it and to not backport it.

Thanks to Willy and Emeric to have spot the issue.

src/stconn.c

index eacbb5c6be51a427fa1aa309fcf4a1c76de0e24e..3fa9dc827c18945936dea473f49384955aa40eaf 100644 (file)
@@ -1520,19 +1520,33 @@ int sc_conn_recv(struct stconn *sc)
                sc_conn_eos(sc);
                ret = 1;
        }
-
        if (sc_ep_test(sc, SE_FL_ERROR)) {
                sc->flags |= SC_FL_ERROR;
                ret = 1;
        }
+
+       if (sc->flags & (SC_FL_EOS|SC_FL_ERROR)) {
+               /* No more data are expected at this stage */
+               se_have_no_more_data(sc->sedesc);
+       }
        else if (!cur_read &&
                 !(sc->flags & (SC_FL_WONT_READ|SC_FL_NEED_BUFF|SC_FL_NEED_ROOM)) &&
                 !(sc->flags & (SC_FL_EOS|SC_FL_ABRT_DONE))) {
-               /* Subscribe to receive events if we're blocking on I/O */
+               /* Subscribe to receive events if we're blocking on I/O. Nothing
+                * was received and it was not because of a blocking
+                * condition.
+                */
                conn->mux->subscribe(sc, SUB_RETRY_RECV, &sc->wait_event);
                se_have_no_more_data(sc->sedesc);
        }
+       else if (sc->flags & SC_FL_EOI) {
+               /* No more data are expected at this stage */
+               se_have_no_more_data(sc->sedesc);
+       }
        else {
+               /* The mux may have more data to deliver. Be sure to be able to
+                * ask it ASAP
+                */
                se_have_more_data(sc->sedesc);
                ret = 1;
        }