Arvados

go.sh: you need to force GOPATH to be absolute, for example like this:

rootdir=$(dirname $(readlink -f $0))

otherwise, go blows up like this if you run go.sh from a symlinked path:

go: GOPATH entry is relative; must be absolute path: ".".
  Run 'go help gopath' for usage.

go.sh: needs documentation for arguments required. Is go.sh build keep what you are intending it to be run as?

go.sh: does not seem to replicate build.sh's functionality - it does not put the keep binary in bin/. Or maybe I'm not calling it the right way, cf. the previous point.

services/keep/src/arvados.org/keepproxy/keepproxy.go: has some commented out code that should be removed

services/keep/src/arvados.org/keepproxy/keepproxy.go: don't hardcode https on line 165, that should be configurable (but default to https) or ideally auto-discovered

services/keep/src/arvados.org/keepproxy/keepproxy_test.go: same, don't hardcode https on line 74

services/keep/src/arvados.org/keepproxy/keepproxy_test.go: these two commands need to check for failure and print the command output if run_test_server.py fails:

exec.Command("python", "run_test_server.py", "start").Run()
  exec.Command("python", "run_test_server.py", "start_keep").Run()

FYI - I'm planning to restrict the number of concurrent connections to the Keep Proxy at the nginx level.

Any thoughts on expected memory use patterns of keep proxy? I assume the worst case scenario (in put mode) would be many clients all writing full 64MiB blocks, slowly. That scenario is what the limit on concurrent connections will protect against.

Please write a log line for every GET/HEAD/PUT request to STDOUT. Something simple will do for now. In Keep we use timestamp method /hash at the moment, and that's plenty fine.

Other than that, I'd say this looks pretty good! Tests pass.

1. Added comment to go.sh. Go.sh is just a wrapper around the go toolchain so regular go commands apply.
2. "go install arvados.org/keepproxy" should build keepproxy and copy it to bin/
3. The commented out code was to force the user to disable GET, which in Tom's original writeup was not supposed to be implemented (but I did it anyway). I'm not clear on the status of keep permissions, in theory the proxy supports it but I haven't tested it.
4. Discussed https when calling API server on IRC
5. Checks error return on .Run() now in the tests
6. Regarding memory usage:
   • GET requests should stream through with minimal buffering. It effectively goes directly from the http.Response reader from the Keep server directly into the http.ResponseWriter that sends to the client, with only a small stub that incrementally tallys the md5 checksum. The buffer used by the Go http library is something like 32 KiB
   • PUT requests pre-allocate a buffer based on the Content-Length in the PUT header, and that buffer is held until the block has been sufficiently replicated or it gives up after contacting all Keep servers. That means we probably do want a relatively conservative cap on the number of concurrent PUT requests allowed, e.g. 16 * 64 MiB will require 1 GiB of RAM.
7. Added a bunch of logging, and uncovered a rather serious and stupid bug in not correctly failing on authorization errors :-)

This looks good to merge to me; just one change please: also log requests that are 404 or fail in other ways. It can just be the same log line as for a successful request for now, unless you want to upgrade it a little bit and add the HTTP response code in the log line, that would be nice.