{"action":"created","issue":{"url":"https://api.github.com/repos/sozu-proxy/sozu/issues/83","repository_url":"https://api.github.com/repos/sozu-proxy/sozu","labels_url":"https://api.github.com/repos/sozu-proxy/sozu/issues/83/labels{/name}","comments_url":"https://api.github.com/repos/sozu-proxy/sozu/issues/83/comments","events_url":"https://api.github.com/repos/sozu-proxy/sozu/issues/83/events","html_url":"https://github.com/sozu-proxy/sozu/issues/83","id":201583122,"node_id":"MDU6SXNzdWUyMDE1ODMxMjI=","number":83,"title":"investigate smarter buffer usage","user":{"login":"Geal","id":119296,"node_id":"MDQ6VXNlcjExOTI5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/119296?v=4","gravatar_id":"","url":"https://api.github.com/users/Geal","html_url":"https://github.com/Geal","followers_url":"https://api.github.com/users/Geal/followers","following_url":"https://api.github.com/users/Geal/following{/other_user}","gists_url":"https://api.github.com/users/Geal/gists{/gist_id}","starred_url":"https://api.github.com/users/Geal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Geal/subscriptions","organizations_url":"https://api.github.com/users/Geal/orgs","repos_url":"https://api.github.com/users/Geal/repos","events_url":"https://api.github.com/users/Geal/events{/privacy}","received_events_url":"https://api.github.com/users/Geal/received_events","type":"User","site_admin":false},"labels":[{"id":520883943,"node_id":"MDU6TGFiZWw1MjA4ODM5NDM=","url":"https://api.github.com/repos/sozu-proxy/sozu/labels/Implementation","name":"Implementation","color":"34495e","default":false}],"state":"open","locked":false,"assignee":{"login":"Geal","id":119296,"node_id":"MDQ6VXNlcjExOTI5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/119296?v=4","gravatar_id":"","url":"https://api.github.com/users/Geal","html_url":"https://github.com/Geal","followers_url":"https://api.github.com/users/Geal/followers","following_url":"https://api.github.com/users/Geal/following{/other_user}","gists_url":"https://api.github.com/users/Geal/gists{/gist_id}","starred_url":"https://api.github.com/users/Geal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Geal/subscriptions","organizations_url":"https://api.github.com/users/Geal/orgs","repos_url":"https://api.github.com/users/Geal/repos","events_url":"https://api.github.com/users/Geal/events{/privacy}","received_events_url":"https://api.github.com/users/Geal/received_events","type":"User","site_admin":false},"assignees":[{"login":"Geal","id":119296,"node_id":"MDQ6VXNlcjExOTI5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/119296?v=4","gravatar_id":"","url":"https://api.github.com/users/Geal","html_url":"https://github.com/Geal","followers_url":"https://api.github.com/users/Geal/followers","following_url":"https://api.github.com/users/Geal/following{/other_user}","gists_url":"https://api.github.com/users/Geal/gists{/gist_id}","starred_url":"https://api.github.com/users/Geal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Geal/subscriptions","organizations_url":"https://api.github.com/users/Geal/orgs","repos_url":"https://api.github.com/users/Geal/repos","events_url":"https://api.github.com/users/Geal/events{/privacy}","received_events_url":"https://api.github.com/users/Geal/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/sozu-proxy/sozu/milestones/1","html_url":"https://github.com/sozu-proxy/sozu/milestone/1","labels_url":"https://api.github.com/repos/sozu-proxy/sozu/milestones/1/labels","id":2260929,"node_id":"MDk6TWlsZXN0b25lMjI2MDkyOQ==","number":1,"title":"nice to have","description":"","creator":{"login":"Geal","id":119296,"node_id":"MDQ6VXNlcjExOTI5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/119296?v=4","gravatar_id":"","url":"https://api.github.com/users/Geal","html_url":"https://github.com/Geal","followers_url":"https://api.github.com/users/Geal/followers","following_url":"https://api.github.com/users/Geal/following{/other_user}","gists_url":"https://api.github.com/users/Geal/gists{/gist_id}","starred_url":"https://api.github.com/users/Geal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Geal/subscriptions","organizations_url":"https://api.github.com/users/Geal/orgs","repos_url":"https://api.github.com/users/Geal/repos","events_url":"https://api.github.com/users/Geal/events{/privacy}","received_events_url":"https://api.github.com/users/Geal/received_events","type":"User","site_admin":false},"open_issues":37,"closed_issues":13,"state":"open","created_at":"2017-01-18T10:44:32Z","updated_at":"2018-04-20T08:35:35Z","due_on":null,"closed_at":null},"comments":6,"created_at":"2017-01-18T14:06:51Z","updated_at":"2018-06-14T12:55:10Z","closed_at":null,"author_association":"MEMBER","body":"In gitlab by @Geal on Nov 3, 2016, 16:54\n\nuntested 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.\nThe proxy could request one buffer from the pool to parse request headers, transfer the headers to the backend, then:\n- if it is a headers-only request, reuse the buffer directly to parse back->front\n- if it is a request with a full body, return the buffer to the pool, use splice to copy the body to the backend\n- 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\n\na few unknowns in this:\n- how many changes to the current code it would require?\n- how to keep compatibility with other platforms?\n- can we really use the same buffer for back and front?"},"comment":{"url":"https://api.github.com/repos/sozu-proxy/sozu/issues/comments/397284909","html_url":"https://github.com/sozu-proxy/sozu/issues/83#issuecomment-397284909","issue_url":"https://api.github.com/repos/sozu-proxy/sozu/issues/83","id":397284909,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NzI4NDkwOQ==","user":{"login":"Geal","id":119296,"node_id":"MDQ6VXNlcjExOTI5Ng==","avatar_url":"https://avatars0.githubusercontent.com/u/119296?v=4","gravatar_id":"","url":"https://api.github.com/users/Geal","html_url":"https://github.com/Geal","followers_url":"https://api.github.com/users/Geal/followers","following_url":"https://api.github.com/users/Geal/following{/other_user}","gists_url":"https://api.github.com/users/Geal/gists{/gist_id}","starred_url":"https://api.github.com/users/Geal/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Geal/subscriptions","organizations_url":"https://api.github.com/users/Geal/orgs","repos_url":"https://api.github.com/users/Geal/repos","events_url":"https://api.github.com/users/Geal/events{/privacy}","received_events_url":"https://api.github.com/users/Geal/received_events","type":"User","site_admin":false},"created_at":"2018-06-14T12:55:10Z","updated_at":"2018-06-14T12:55:10Z","author_association":"MEMBER","body":"a bit more info about that issue, since the asserts have run long enough now.\r\n\r\nThe asserts mainly trigger in 2 cases:\r\n* a client tries to do [HTTP pipelining](https://en.wikipedia.org/wiki/HTTP_pipelining)\r\n* invalid HTTP requests\r\n\r\nSo the main reason we could have two buffers (front->back and back->front) at once to handle a session would be supporting HTTP pipelining.\r\n\r\nAs a side note, here's the current hack I have to support pipelining, it's not too bad:\r\n\r\n```\r\n--- a/lib/src/network/protocol/http.rs\r\n+++ b/lib/src/network/protocol/http.rs\r\n@@ -14,6 +14,7 @@ use network::buffer_queue::BufferQueue;\r\n use network::socket::{SocketHandler,SocketResult};\r\n use network::protocol::ProtocolResult;\r\n use util::UnwrapLog;\r\n+use nom::HexDisplay;\r\n \r\n #[derive(Clone)]\r\n pub struct StickySession {\r\n@@ -123,7 +124,12 @@ impl<Front:SocketHandler> Http<Front> {\r\n     self.state.as_mut().map(|ref mut state| state.added_res_header = res_header);\r\n     self.front_buf_position = 0;\r\n     self.back_buf_position = 0;\r\n-    self.front_buf.reset();\r\n+    if !self.front_buf.empty() {\r\n+      info!(\"PIPELINING\");\r\n+      self.readiness.front_readiness.insert(Ready::readable());\r\n+    } else {\r\n+      self.front_buf.reset();\r\n+    }\r\n     self.back_buf.reset();\r\n     //self.readiness = Readiness::new();\r\n     self.request_id = request_id;\r\n@@ -590,7 +596,10 @@ impl<Front:SocketHandler> Http<Front> {\r\n   // Forward content to client\r\n   pub fn writable(&mut self, metrics: &mut SessionMetrics) -> ClientResult {\r\n \r\n-    assert!(self.front_buf.empty(), \"investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response\");\r\n+    //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);\r\n+    if !self.front_buf.empty() {\r\n+      info!(\"writable: there was more data in the front buffer, keeping for later\");\r\n+    }\r\n \r\n     //handle default answers\r\n     let output_size = self.back_buf.output_data_size();\r\n@@ -863,7 +872,11 @@ impl<Front:SocketHandler> Http<Front> {\r\n       return (ProtocolResult::Continue, ClientResult::Continue);\r\n     }\r\n \r\n-    assert!(self.front_buf.empty(), \"investigating single buffer usage: the front->back buffer should not be used while parsing and forwarding the response\");\r\n+    //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{}\",\r\n+    //  self.state, self.readiness, self.protocol, self.front_buf.buffer.data().to_hex(16));\r\n+    if !self.front_buf.empty() {\r\n+      info!(\"back_readable: there was more data in the front buffer, keeping for later\");\r\n+    }\r\n \r\n     if self.back_buf.buffer.available_space() == 0 {\r\n       self.readiness.back_interest.remove(Ready::readable());\r\n```\r\n\r\nI think I found a good solution, I'll write more about it later today."}}
{"actor":{"display_login":"Geal"}}