camput performance

[Marc-Antoine Ruel]

Hi all,

Here's my analysis about camput performance, please correct me if I'm wrong or

missing something.

- client.Client.StatBlobs() supports stating multiple files in a single request

  but is currently unbounded which could cause issues with overly long query url.

  I wrote https://camlistore.org/r/249 to limit request length and make the

  request size parallel "while at it". In particular,

  pkg/blobserver/handlers/stat.go(37) "const maxStatBlobs = 1000" so the client

  code was probably never used in batching mode.

- There's concurrency in in cmd/camput/files.go; with 5 concurrent requests for

  upload and 10 for local cache lookup. Hypothesis is that cache lookup duration

  is minimal and performance is largely bound by the latency of the http

  requests due to the sheer number of http requests done serially.

- Cache lookup seems to be totally CPU bound so it should use

  runtime.GOMAXPROCS(-1) instead of hardcoding to 10.

- camput starts a loop in cmd/camput/files.go to upload each file individual

  with its own client.Client instead. It's created with

  client.NewUploadHandleFromString() and then client.Client.Upload() is

  inherently serial in its operation.

- schema.serverHasBlob() is eventually called from schema.writeFileChunks() for

  the rolling hash blobs, interestingly this is also done serially with the

  uploadLastSpan() inner function, compounding request latency.

I would have like to help with it but I'm not sure where I could do a meaningful

contribution taking in account the different layers involved (schema, client,

camput), in theory:

- client.Client.StatBlobs() should be run in a loop, accepting a mix of file

  lookup and chunk lookup.

- That mainly requires changing the caller so that the task items are surfaced

  to client.Client.

- camput should defer upload parallelism to client.Client.

- Remote store chunks lookup should be done in parallel after the file

  is detected to not be present.

- Remote store file lookup should be batched, especially in case of >thousands

  files.

- A priority queue should be used to determine which batch to stat first, the

  largest files first, since they are the slowest to upload if they are a 100%

  remote store miss.

- client.Client parallelism should parallelize both stating and uploading, but

  should give priority to stating requests.

I didn't profile the app yet, I only looked at the code, so the above hypothesis

could be all wrong. I've saw related TODOs in the code so I guess it's just a

matter of priorities that explain the current state.

I wrote https://camlistore.org/r/248 to enable localhost HTTP PROXY to

eventually try out with a 500ms latency inducing proxy and optimize

accordingly.

And sorry Brad, I didn't know misc/review sent email by default. I wanted you to get this email before the reviews.

M-A

[Brad Fitzpatrick]

On Mon, Jan 28, 2013 at 7:09 AM, Marc-Antoine Ruel <mar...@google.com> wrote:

Hi all,

Here's my analysis about camput performance, please correct me if I'm wrong or

missing something.

Excellent analysis, and welcome!

- client.Client.StatBlobs() supports stating multiple files in a single request

  but is currently unbounded which could cause issues with overly long query url.

  I wrote https://camlistore.org/r/249 to limit request length and make the

  request size parallel "while at it". In particular,

  pkg/blobserver/handlers/stat.go(37) "const maxStatBlobs = 1000" so the client

  code was probably never used in batching mode.

Great cleanup project, thanks for taking it on.

 

- There's concurrency in in cmd/camput/files.go; with 5 concurrent requests for

  upload and 10 for local cache lookup. Hypothesis is that cache lookup duration

  is minimal and performance is largely bound by the latency of the http

  requests due to the sheer number of http requests done serially.

I'm sure there's work to be done here, as you've found.  There used to be tons of resource leaks until a month or so ago, so those limits were very critical in keeping things sane.  Performance analysis and tuning is definitely due.

Too many things are serial, for sure.

Related bug: https://code.google.com/p/camlistore/issues/detail?id=84 for bootchart output so we could visualize camput/camget better.

- Cache lookup seems to be totally CPU bound so it should use

  runtime.GOMAXPROCS(-1) instead of hardcoding to 10.

I don't think -1 does what you think it means.  There's no way to make it go unbounded.  You probably mean runtime.NumCPU().

- camput starts a loop in cmd/camput/files.go to upload each file individual

  with its own client.Client instead.

I know it does each file individually for now (see below), but I don't think each one has its own HTTP client... sure?

 

It's created with

  client.NewUploadHandleFromString() and then client.Client.Upload() is

  inherently serial in its operation.

I'm actually in the process of fixing this one right now for the Android client (which uses camput as a child process).

The current TreeUpload mode (single arg being a directory) is much more optimized than the 1-n file args case.

 

 - schema.serverHasBlob() is eventually called from schema.writeFileChunks() for

  the rolling hash blobs, interestingly this is also done serially with the

  uploadLastSpan() inner function, compounding request latency.

Yes, this has been bugging me too.  I believe there are TODOs there about this.

It was so tricky to get all that code working well and bug-free that I hid from that problem for awhile, but I'm confident in our tests and FileReader / FileWriter now, so it's probably time to tackle.

 

I would have like to help with it but I'm not sure where I could do a meaningful

contribution taking in account the different layers involved (schema, client,

camput), in theory:

- client.Client.StatBlobs() should be run in a loop, accepting a mix of file

  lookup and chunk lookup.

- That mainly requires changing the caller so that the task items are surfaced

  to client.Client.

- camput should defer upload parallelism to client.Client.

- Remote store chunks lookup should be done in parallel after the file

  is detected to not be present.

To some degree.  If you have a 2TB local file, you don't want to buffer that all in memory.  In general, the FileWriter writes a file from an io.Reader, not something seekable.  (and if it's seekable: it might be changing underneath you).  But yes.

 

- Remote store file lookup should be batched, especially in case of >thousands

  files.

Yes.

 

- A priority queue should be used to determine which batch to stat first, the

  largest files first, since they are the slowest to upload if they are a 100%

  remote store miss.

Yes.  Even without a priority queue, I'd be happy just to do batching in the case where client.go's maxParallelHTTP is exhausted and a stat has to wait.

It should never be the case that 500 blob stats are waiting on reqGate, and then do 500 1-blob stats.  They should be aggregated at that point.  I'd fix that first.

 

- client.Client parallelism should parallelize both stating and uploading, but

  should give priority to stating requests.

I didn't profile the app yet, I only looked at the code, so the above hypothesis

could be all wrong. I've saw related TODOs in the code so I guess it's just a

matter of priorities that explain the current state.

I wrote https://camlistore.org/r/248 to enable localhost HTTP PROXY to

eventually try out with a 500ms latency inducing proxy and optimize

accordingly.

You don't need a proxy.  The server already supports both verbose per-HTTP request output and also latency + bandwidth control, which I added specifically for optimizing camput and camget.

See the dev-server script at the root and its flags.  Day-to-day we work with dev-server and dev-camput / dev-camget.

 

And sorry Brad, I didn't know misc/review sent email by default. I wanted you to get this email before the reviews.

No worries. Spam away.

[Marc-Antoine Ruel]

2013/1/28 Brad Fitzpatrick <br...@danga.com>

On Mon, Jan 28, 2013 at 7:09 AM, Marc-Antoine Ruel <mar...@google.com> wrote:

- Cache lookup seems to be totally CPU bound so it should use

  runtime.GOMAXPROCS(-1) instead of hardcoding to 10.

I don't think -1 does what you think it means.  There's no way to make it go unbounded.  You probably mean runtime.NumCPU().

The minimum value between runtime.GOMAXPROCS(-1) and runtime.NumCPU() would make sense? E.g. if GOMAXPROCS < NumCPU, it's not worth sending NumCPU requests. I can do a CL for that, it's ~4 lines.

 

- camput starts a loop in cmd/camput/files.go to upload each file individual

  with its own client.Client instead.

I know it does each file individually for now (see below), but I don't think each one has its own HTTP client... sure?

Was I sure? Absolutely not. I was confused by fileMapFromDuplicate().

  

 

 - schema.serverHasBlob() is eventually called from schema.writeFileChunks() for

  the rolling hash blobs, interestingly this is also done serially with the

  uploadLastSpan() inner function, compounding request latency.

Yes, this has been bugging me too.  I believe there are TODOs there about this.

It was so tricky to get all that code working well and bug-free that I hid from that problem for awhile, but I'm confident in our tests and FileReader / FileWriter now, so it's probably time to tackle.

 

I would have like to help with it but I'm not sure where I could do a meaningful

contribution taking in account the different layers involved (schema, client,

camput), in theory:

- client.Client.StatBlobs() should be run in a loop, accepting a mix of file

  lookup and chunk lookup.

- That mainly requires changing the caller so that the task items are surfaced

  to client.Client.

- camput should defer upload parallelism to client.Client.

- Remote store chunks lookup should be done in parallel after the file

  is detected to not be present.

To some degree.  If you have a 2TB local file, you don't want to buffer that all in memory.  In general, the FileWriter writes a file from an io.Reader, not something seekable.  (and if it's seekable: it might be changing underneath you).  But yes.

Interesting, because you still need to read the file two times; once to get the file's sha-1 and a second time to send the chunks. So could it change underneath anyway? That's what my pet project does too and I don't see a way of implementing it in a one-read pass that is efficient.

- A priority queue should be used to determine which batch to stat first, the

  largest files first, since they are the slowest to upload if they are a 100%

  remote store miss.

Yes.  Even without a priority queue, I'd be happy just to do batching in the case where client.go's maxParallelHTTP is exhausted and a stat has to wait.

It should never be the case that 500 blob stats are waiting on reqGate, and then do 500 1-blob stats.  They should be aggregated at that point.  I'd fix that first. 

I'm still unsure about how to design it in a way that doesn't leaks implementation details across the interface layers. I see it would have to expose a "work item" scheduler but that feels wrong. I need to read more code. If you have an idea how to design the stating of files and chunks more efficiently, I'm listening.

 

I wrote https://camlistore.org/r/248 to enable localhost HTTP PROXY to

eventually try out with a 500ms latency inducing proxy and optimize

accordingly.

You don't need a proxy.  The server already supports both verbose per-HTTP request output and also latency + bandwidth control, which I added specifically for optimizing camput and camget.

See the dev-server script at the root and its flags.  Day-to-day we work with dev-server and dev-camput / dev-camget.

Failed to find it; neither of the two following commands is informative:

./dev-server -help

gopath/bin/linux_amd64/camlistored -help

I think supporting a local proxy enables more fiddling with the requests and less custom code in the project, without having to resort to use 2 workstations. For example do an experiment by HTTP 500'ing 1% of the requests. Or add extraneous 500ms to 10% of the requests. Overall, reproducing how AppEngine behaves. :) I don't mind if you don't think it's useful.

I'd be happy to help with documentation if I fail to provide useful code as I don't plan to put more than a few tens of hours on the project.

M-A

[Brad Fitzpatrick]

On Sun, Feb 3, 2013 at 11:13 AM, Marc-Antoine Ruel <mar...@google.com> wrote:

2013/1/28 Brad Fitzpatrick <br...@danga.com>

On Mon, Jan 28, 2013 at 7:09 AM, Marc-Antoine Ruel <mar...@google.com> wrote:

- Cache lookup seems to be totally CPU bound so it should use

  runtime.GOMAXPROCS(-1) instead of hardcoding to 10.

I don't think -1 does what you think it means.  There's no way to make it go unbounded.  You probably mean runtime.NumCPU().

The minimum value between runtime.GOMAXPROCS(-1) and runtime.NumCPU() would make sense? E.g. if GOMAXPROCS < NumCPU, it's not worth sending NumCPU requests. I can do a CL for that, it's ~4 lines.

Sure. That works.

 

 - schema.serverHasBlob() is eventually called from schema.writeFileChunks() for

  the rolling hash blobs, interestingly this is also done serially with the

  uploadLastSpan() inner function, compounding request latency.

Yes, this has been bugging me too.  I believe there are TODOs there about this.

It was so tricky to get all that code working well and bug-free that I hid from that problem for awhile, but I'm confident in our tests and FileReader / FileWriter now, so it's probably time to tackle.

I actually worked on this last night and it's now all parallel.  The client also batches stats now, so bandwidth is much higher, number of HTTP requests is way down, and overall latency is reduced.

I wrote https://camlistore.org/r/248 to enable localhost HTTP PROXY to

eventually try out with a 500ms latency inducing proxy and optimize

accordingly.

You don't need a proxy.  The server already supports both verbose per-HTTP request output and also latency + bandwidth control, which I added specifically for optimizing camput and camget.

See the dev-server script at the root and its flags.  Day-to-day we work with dev-server and dev-camput / dev-camget.

Failed to find it; neither of the two following commands is informative:

./dev-server -help

gopath/bin/linux_amd64/camlistored -help

Just look at dev-server itself.  It's not much code.  The part you're looking for is:

my $opt_KBps = 150; # if non-zero, kBps to throttle connections                                                                                                                                                                 

my $opt_latency_ms = 90; # added latency in millisecond                                                                                                                                                                         

....

unless ($opt_fast) {

    $ENV{DEV_THROTTLE_KBPS} = $opt_KBps;

    $ENV{DEV_THROTTLE_LATENCY_MS} = $opt_latency_ms;

}

 

I think supporting a local proxy enables more fiddling with the requests and less custom code in the project, without having to resort to use 2 workstations. For example do an experiment by HTTP 500'ing 1% of the requests. Or add extraneous 500ms to 10% of the requests. Overall, reproducing how AppEngine behaves. :) I don't mind if you don't think it's useful.

I'm not against it, if that's your tool of choice. I didn't notice on the first review why you were copying ProxyFromEnvironment into the CL... I've replied on the CL now.