Buffer git-receive-pack responses
This allows us to respond with HTTP 500 instead of 200 when a 'git push' is rejected by the server.
Follow-up to https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/38
Merge request reports
Activity
mentioned in merge request !38 (closed)
150 // the error response from gitlab-workhorse will always be HTTP 500 151 // (Internal Server Error). 152 bodyWriter := &bytes.Buffer{} // buffer the entire response in memory 153 defer flushBuffer(w, bodyWriter) 154 } 155 142 156 // Start writing the response 143 157 w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-result", action)) 144 158 w.Header().Add("Cache-Control", "no-cache") 145 w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return 146 159 147 160 // This io.Copy may take a long time, both for Git push and pull. 148 if _, err := io.Copy(w, stdout); err != nil { 149 helper.LogError(fmt.Errorf("handlePostRPC copy output of %v: %v", cmd.Args, err)) 161 if _, err := io.Copy(bodyWriter, stdout); err != nil { 162 helper.Fail500(w, fmt.Errorf("handlePostRPC copy output of %v: %v", cmd.Args, err)) This might be a problem. During a (succesful) long push there is a risk of timeouts already, because it can take a long time before
git receive-packprints any output to stdout. That creates a long 'silence' in the HTTP response which can cause intermediate proxies like HAProxy and NGINX to terminate the request.If we buffer the entire response we may be increasing the delay even more.
Another thing we could do is buffer e.g. the first 8192 bytes of output, regardless of whether it is 'git push' or 'git pull'. Then if the buffer contains less than 8192 bytes, assume the Git command is done already and call Cmd.Wait() so we can still set the HTTP status code to 500 (or some other more appropriate value). If the buffer contains 8192 bytes, assume success (write HTTP 200) and start sending the buffer and the rest of the data.
mentioned in merge request !42 (merged)
Closing in favor of !42 (merged) where we buffer the first 8192 bytes; it is more complicated but also more predictable in terms of memory use.
mentioned in commit 285f47a7
