Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: fetching dependencies can be very aggressive when going via an HTTP proxy #33669

Open
errordeveloper opened this issue Aug 15, 2019 · 7 comments
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@errordeveloper
Copy link

errordeveloper commented Aug 15, 2019

What version of Go are you using (go version)?

$ go version
go version go1.12.8 windows/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

windows/amd64

What did you do?

  1. Set http_proxy environment variable
  2. git clone https://github.com/weaveworks/eksctl && cd eksctl
  3. go build ./cmd/eksctl

What did you expect to see?

All dependencies get downloaded and binary gets built. If errors occur, there are a few retries with a back-off.

What did you see instead?

Some dependencies get downloaded, some fail to download with 407 error in git fetch. Binary is not built. I had to rerun go build multiple times before it succeeded.

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 15, 2019
@andybons andybons modified the milestones: Unreleased, Unplanned Aug 15, 2019
@andybons
Copy link
Member

@bcmills @jayconrod

@errordeveloper
Copy link
Author

It's kind of to do with having to download the internet, and once you hit a proxy with all the request for git object, it chokes up. One option would be to have retries, as I suggested above, another could be to provide a flag that allows user to set a pause interval between requests.

@errordeveloper errordeveloper changed the title cmd/go: should retry on HTTP errors while fetching dependencies via a proxy cmd/go: fetching dependencies can be very aggressive when going via an HTTP proxy Aug 15, 2019
@errordeveloper
Copy link
Author

I figured the title of the issue was too prescriptive, I renamed it to reflect what the issue is really about.

@heschi
Copy link
Contributor

heschi commented Aug 15, 2019

Does using a GOPROXY, e.g. GOPROXY=https://proxy.golang.org, improve things? It should be less demanding of an HTTP proxy than Git. That doesn't answer the question of whether the go command should retry though.

@bcmills
Copy link
Contributor

bcmills commented Aug 15, 2019

Retries are #28194.

That said, I'm not sure that there is anything we should do on the go command side if the problem is that the git commands overwhelm the proxy. For HTTP requests initiated on the go command side we rely on whatever connection pooling and limiting the net/http package provides by default, but I assume that individual git commands probably don't share proxy connections.

Ideally, your HTTP proxy should be robust enough to just not read from the incoming socket when it doesn't have the capacity to spare, rather than returning spurious 407s for some of the requests.

@bcmills
Copy link
Contributor

bcmills commented Aug 15, 2019

That said, I've been thinking about moving the parallelism limits from the various par.Work calls down to the leaf packages where we actually need to limit things. If we do that, perhaps we can use a more restrictive semaphore for git invocations than we use for package fetches in general.

@gopherbot
Copy link

Change https://golang.org/cl/220080 mentions this issue: design/36460: add design for lazy module loading

gopherbot pushed a commit to golang/proposal that referenced this issue Mar 5, 2020
Updates golang/go#36460
Updates golang/go#27900
Updates golang/go#26955
Updates golang/go#30831
Updates golang/go#32058
Updates golang/go#32380
Updates golang/go#32419
Updates golang/go#33370
Updates golang/go#33669
Updates golang/go#36369

Change-Id: I1d4644e3e8b4e688c2fc5a569312495e5072b7d7
Reviewed-on: https://go-review.googlesource.com/c/proposal/+/220080
Reviewed-by: Russ Cox <rsc@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants