150 parallel pull

[lfn3]

First half of #150

It works as expected with the 'docker-ut' and 'base' images.
I'd like to test it against something that will stress it a little more, but I can't think of an easy way of doing this at present.

[cespare]

You should probably remove the relevant // FIXME as part of the change.

[cespare]

What's the purpose of limiting the number of workers to 4? Spawning a whole bunch of goroutines is fine. Maybe you're trying to limit the number of concurrent HTTP requests?

[lfn3]

Yeah, it's to limit the number of concurrent requests.

[cespare]

Can't you range over errChan instead of doing this loop? (And then you can get rid of expectedResponseCount.)

[lfn3]

I don't think so. I'd have to close the channel from one of the goroutines, and I can't tell if another is still in progress.

[cespare]

Ah sorry, I didn't read this closely enough. My mistake.

[cespare]

OK, I'm done :)

[lfn3]

Lol. Thanks. Appreciate the help.

[dominikh]

Maybe a simple []string and strings.Join would be nicer here? You wouldn't allocate as many new strings, you wouldn't have the overhead of reflection and the code would be more straightforward to understand, by not having the "is it initialized yet?" check on every iteration.

errors := []string{"The following errors..."}
for ... {
  // append if error
}

if len(errors) > 1 {
  return errors.New(strings.Join(errors, "\n"))
}

[lfn3]

Good call. Updated

[shykes]

Hey @byatlas, thanks for the contribution. It seems to work well, and definitely speeds up downloads :)

I found one problem: the progress bars write over each other. Not sure what's the simplest way to solve that: maybe add an optional "position" parameter to ProgressReader, which would add N newlines before writing? This is how pv(1) does it for multiple streams (see pv -c -N).

[shykes]

Ping :)

[lfn3]

Sorry for the radio silence, I just got a new job so that's been keeping me busy.

Anyway, I took a swing at this, wrapping a io.Writer both using newlines and console escape codes. Due the the whole async thing, this can cause the cursor to jump around like crazy, and overwrite random lines.
Not particularly useful.

Maybe we just ignore percentage counters and go 'downloading layer %d of %d'?

[shykes]

So, there is now a new HTTP api, and is streams progress using json. So now we can use the json stream to interleave updates about multiple downloads. This will need a small change to the api (specifically, using "progress_FOO" and "progress_BAR" as json keys instead of just "progress").

@byatlas, if you're still motivated to do this, let me know and I will keep this open for you to update it. If you don't have the time, or will get around to it later, let's close it for now and re-open it when it makes sense.

Thanks for the contribution and sorry we couldn't take it in as is.

[lfn3]

I'll close it for now. I might come up with something in a bit, but we'll see.

And no worries, maybe next time.