investigate smarter buffer usage

[Geal]

In gitlab by @Geal on Nov 3, 2016, 16:54

untested idea: the buffers are mainly used to parse headers, while the rest of the data can be transferred by splice() (on Linux). So keeping two buffers (one for front->back, one for back->front) could be a bit wasteful.
The proxy could request one buffer from the pool to parse request headers, transfer the headers to the backend, then:

• if it is a headers-only request, reuse the buffer directly to parse back->front
• if it is a request with a full body, return the buffer to the pool, use splice to copy the body to the backend
• if it is a chunking request, return the buffer to the pool, copy and parse each chunk header in a fixed stack buffer (is there a limit to chunk size so we can infer a limit for the chunk header's size?), then use splice to copy the chunk's content

a few unknowns in this:

• how many changes to the current code it would require?
• how to keep compatibility with other platforms?
• can we really use the same buffer for back and front?

[Geal]

In gitlab by @Geal on Nov 4, 2016, 12:50

First experiment in 3270bf7: assert that the back->front buffer is empty and unused when handling the request, and the front->back buffer is empty and unused when handling the response

if it works, it means we can reuse the same buffer for both directions, and halve the memory usage (using two buffers was a safe but wasteful solution).

This experiment will probably not work in the websocket case, where data can go back and forth at any time

[Geal]

In gitlab by @Geal on Jan 11, 2017, 10:34

That first experiment worked, and with the protocol upgrades, I can specify that websocket piping needs two buffers, while HTTP only needs one.

[Geal]

In gitlab by @Geal on Jan 11, 2017, 10:34

next thing to experiment: as indicated in https://eklitzke.org/goroutines-nonblocking-io-and-memory-usage give back the buffer to the pool after a request ended, get a buffer to the pool at the beginning of the next keepalive request.
The idea is that most keepalive connections are mostly idle, so we can use the memory for other active clients in the meantime.

Question: what happens under high load? How costly is it to release a buffer or get one from the pool? Should the buffer be released after a timeout?

[Geal]

so #10 is at odds with the experiment in 3270bf7 because the client might send the body right after sending the request anyway. Of course, that behaviour is wrong, since the client is supposed to wait for the server to answer before sending the body. So we can just ignore a misbehaving client who does that?

[Geal]

other thing to note: in some cases, the server could send an answer before receiving the whole request (example: to refuse a big file upload)

[Geal]

so I think we could remove the double buffer usage now, since the assert have been in use for a long time and have not really triggered (except when caused by unrelated bugs).

• for Handle "100 Continue" answers #10, I'd be inclined to ignore this behaviour, since it means the client does not really respect the 100 Continue feature
• for the case when the server might do an early answer: let's have the HTTP protocol register interest in readable for the backend right from the start. If we get a readable event for the backend while the client has not finished sending its request, let's switch to transmitting the response, then switch back to transmitting the request? This might need a special "early answer" state

This change might be a better fit for a future version, instead of the current release. That will leave us an opportunity for nice optimizations

[Geal]

a bit more info about that issue, since the asserts have run long enough now.

The asserts mainly trigger in 2 cases:

• a client tries to do HTTP pipelining
• invalid HTTP requests

So the main reason we could have two buffers (front->back and back->front) at once to handle a session would be supporting HTTP pipelining.

As a side note, here's the current hack I have to support pipelining, it's not too bad:

--- a/lib/src/network/protocol/http.rs
+++ b/lib/src/network/protocol/http.rs
@@ -14,6 +14,7 @@ use network::buffer_queue::BufferQueue;
 use network::socket::{SocketHandler,SocketResult};
 use network::protocol::ProtocolResult;
 use util::UnwrapLog;
+use nom::HexDisplay;
 
 #[derive(Clone)]
 pub struct StickySession {
@@ -123,7 +124,12 @@ impl<Front:SocketHandler> Http<Front> {
     self.state.as_mut().map(|ref mut state| state.added_res_header = res_header);
     self.front_buf_position = 0;
     self.back_buf_position = 0;
-    self.front_buf.reset();
+    if !self.front_buf.empty() {
+      info!("PIPELINING");
+      self.readiness.front_readiness.insert(Ready::readable());
+    } else {
+      self.front_buf.reset();
+    }
     self.back_buf.reset();
     //self.readiness = Readiness::new();
     self.request_id = request_id;
@@ -590,7 +596,10 @@ impl<Front:SocketHandler> Http<Front> {
   // Forward content to client
   pub fn writable(&mut self, metrics: &mut SessionMetrics) -> ClientResult {
 
-    assert!(self.front_buf.empty(), "investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response");
+    //assert!(self.front_buf.empty(), "investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response\nstate:{:?}\nreadiness: {:?}\nprotocol:{:?}", self.state, self.readiness, self.protocol);
+    if !self.front_buf.empty() {
+      info!("writable: there was more data in the front buffer, keeping for later");
+    }
 
     //handle default answers
     let output_size = self.back_buf.output_data_size();
@@ -863,7 +872,11 @@ impl<Front:SocketHandler> Http<Front> {
       return (ProtocolResult::Continue, ClientResult::Continue);
     }
 
-    assert!(self.front_buf.empty(), "investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response");
+    //assert!(self.front_buf.empty(), "investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response\nstate:{:?}\nreadiness: {:?}\nprotocol:{:?}\nfront buffer data:\n{}",
+    //  self.state, self.readiness, self.protocol, self.front_buf.buffer.data().to_hex(16));
+    if !self.front_buf.empty() {
+      info!("back_readable: there was more data in the front buffer, keeping for later");
+    }
 
     if self.back_buf.buffer.available_space() == 0 {
       self.readiness.back_interest.remove(Ready::readable());

I think I found a good solution, I'll write more about it later today.

[Geal]

so, here's the idea: make the buffers optional for the HTTP implementation, and allocate them as needed. Here's how it would play out:

• when creating the HTTP state machine, only allocate the front buffer (or pass a previous one from rustls handshake)
• we read and parse the data
• once we're connected to the backend, we're waiting for the writable event (because we must pipe the request) and the readable event (in case the backend sends back an early response, before all the request has been sent)
• when the entire request has been read, give back the front buffer to the pool. Unless it still has some data around, in which case we will keep the buffer, to send the next request once the response has been given (this is not exactly how pipelining should work, but the backend server is supposed to be close, so the added latency will not be great)
• when some data gets available from the backend, get a back buffer from the pool
• read and parse data from the backend, send to the frontend
• once the response has been sent to the client, and if we're in keep-alive, return the back buffer to the pool and reset the state. If the front buffer still had some data, start parsing the new request

This solution would have multiple benefits:

• add some limited support for pipelining
• support early responses. Imagine sending a huge file to the server, but the server refuses it. Right now sozu would have to wait until the whole request has been sent to see the response
• much better buffer usage. Right now, we allocate the front and back buffer at the beginning, so one of those is always unused, and during the time between the end of request and the first byte from the response, both buffers are unused (some backend servers can be very slow to answer). With this solution, we would have:
  • 1 buffer used during request parsing
  • 0 buffer used between end of request and beginning of response (or 1 buffer if pipelining HTTP requests)
  • 1 buffer used during response parsing (or 1 buffer if pipelining HTTP requests)
  • 0 buffer used between requests

I'm not planning on implementing this right away, but first I'd like to add a metric to track the buffer pool usage, then we'll see more easily if this has an effect.

[Geal]

HTTP pipelining is implemented in 996c9e7

[Geal]

this is done since 3b88968