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: duplicate proxy requests sent for @v/list #51391

Closed
raygecao opened this issue Feb 28, 2022 · 13 comments
Closed

cmd/go: duplicate proxy requests sent for @v/list #51391

raygecao opened this issue Feb 28, 2022 · 13 comments
Assignees
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Milestone

Comments

@raygecao
Copy link

What is the URL of the page with the issue?

https://go.dev/ref/mod#communicating-with-proxies

What is your user agent?

Chrome

Screenshot

image

What did you do?

I serve my mod cache as proxy to validate the process of loading a package. I observe a different order of api calling to find the latest version.

What did you expect to see?

According to the description from module doc, I should see the sequence of calling latest api after what list api returned is empty or none of the returned versions can be used.

What did you see instead?

I see latest api is called before list api to find the latest version
image-20220227193426192

@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2022
@raygecao raygecao changed the title x/website: x/website: The calling order for finding the latest version of the modules is mismatch? Feb 28, 2022
@mengzhuo
Copy link
Contributor

The first request shows GET list and I think it's work as intended.
Please take a look at modfetch/proxy.go

func (p *proxyRepo) latest() (*RevInfo, error) {
data, err := p.getBytes("@v/list")
if err != nil {
return nil, p.versionError("", err)
}

Unlike many projects, the Go project does not use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals only.

For questions please refer to https://github.com/golang/go/wiki/Questions

@raygecao
Copy link
Author

@mengzhuo Thanks for your reply, maybe the ending ? make it just like a question, but I still think it's a confusing description in Go Modules Reference doc.

The descriptions are as follows:

When the go command requests the latest version of a module, it first sends a request for $module/@v/list. If the list is empty or none of the returned versions can be used, it sends a request for $module/@latest

When resolving the latest version of a module, the go command will request $base/$module/@v/list, then, if no suitable versions are found, $base/$module/@latest.

while my experiment shows $base/$module/@latest is called before $base/$module/@v/list.

I think the code you shows before is the second request, The first request is GET latest shown as:

func (p *proxyRepo) Latest() (*RevInfo, error) {
data, err := p.getBytes("@latest")
if err != nil {
if !errors.Is(err, fs.ErrNotExist) {
return nil, p.versionError("", err)
}
return p.latest()
}

I find $base/$module/@latest has a higher priority than $base/$module/@v/list both from the practice and the code, while the doc shows an opposite description.

@raygecao raygecao changed the title x/website: The calling order for finding the latest version of the modules is mismatch? x/website: The calling order for finding the latest version of the modules is mismatch. Feb 28, 2022
@mengzhuo mengzhuo added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2022
@mengzhuo
Copy link
Contributor

mengzhuo commented Feb 28, 2022

It's not about the question mark in the title, sorry for the triage error.
I will let @bcmills @matloob decise whether it's bug or not.

@mengzhuo mengzhuo reopened this Feb 28, 2022
@seankhliao
Copy link
Member

which go command did you run?
and go env output please

@bcmills
Copy link
Member

bcmills commented Feb 28, 2022

In the screenshot, I see two different requests for @v/list, and one in the middle for @latest.

The first @v/list call is likely the one looking for release versions. The @latest call in the middle checks for the latest available pseudo-version.

The second @v/list call is probably a caching bug of some sort, possibly to check for retractions. (We should really only send that request the first time, and then cache it.)

@raygecao
Copy link
Author

raygecao commented Mar 1, 2022

go env:

GO111MODULE=""
GOARCH="amd64"
GOBIN="/Users/leicao/go/bin"
GOCACHE="/Users/leicao/Library/Caches/go-build"
GOENV="/Users/leicao/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/leicao/go/pkg/mod"
GONOPROXY="git-pd.megvii-inc.com,go.megvii-inc.com"
GONOSUMDB="git-pd.megvii-inc.com,go.megvii-inc.com"
GOOS="darwin"
GOPATH="/Users/leicao/go"
GOPRIVATE="git-pd.megvii-inc.com,go.megvii-inc.com"
GOPROXY="https://goproxy.io,direct"
GOROOT="/usr/local/Cellar/go/1.17.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.17.2/libexec/pkg/tool/darwin_amd64"
GOVCS=""
GOVERSION="go1.17.2"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/leicao/chef/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch x86_64 -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/7p/jjvh83sx34bbvq1dmnqnwl6w0000gn/T/go-build2436077160=/tmp/go-build -gno-record-gcc-switches -fno-common"

cmd:

GOPROXY=http://web.raygecao.cn  GOSUMDB=off go get github.com/raygecao/6.824-test

@raygecao
Copy link
Author

raygecao commented Mar 1, 2022

I follow the code of modget/get.go and find the key process of queryProxy:

  • find the release/prerelease by @v/list (with *proxyRepo.Versions)
  • if neither exists, use @v/latest to get the version (in *proxyRepo.Latest)
  • if ErrNotExist returned by @latest, call @v/list again for the pseudo-version (in *proxyRepo.latest)

So I agree that the first @v/list and @latest work well as the doc describes.


The contradiction is that I find pseudo-versions should not returned by @v/list.

$base/$module/@v/list: Returns a list of known versions of the given module in plain text, one per line. This list should not include pseudo-versions.

In this way, I confuse the effect of the second @v/list. what I see is the processing of the pseudo-versions returned by @v/list.

I serve module cache as a proxy for the experiment. Though the layout is consistent with the GOPROXY protocol, pseudo-versions can be returned by @v/list not following the principle above. Is the second @v/list just compatible with this case?

@bcmills
Copy link
Member

bcmills commented Mar 4, 2022

The contradiction is that I find pseudo-versions should not returned by @v/list.

Indeed, but sometimes they are anyway, and we didn't want to break existing users by ignoring them entirely (see #32715).

In particular, sometimes they are needed in order to resolve @latest when using a module proxy constructed from an on-disk module cache, which typically does not provide the @latest endpoint.

@bcmills
Copy link
Member

bcmills commented Mar 4, 2022

It looks like the Get responses may have been cached by the web2 package before I removed it in CL 170879.

I think I didn't port the caching logic because at the time I believed that all such requests were cached at a higher level. 😅

@bcmills bcmills changed the title x/website: The calling order for finding the latest version of the modules is mismatch. cmd/go: duplicate proxy requests sent for @v/list Mar 4, 2022
@bcmills bcmills modified the milestones: Unreleased, Go1.19 Mar 4, 2022
@bcmills
Copy link
Member

bcmills commented Mar 4, 2022

(CC @matloob)

@bcmills bcmills self-assigned this Mar 31, 2022
@gopherbot
Copy link

Change https://go.dev/cl/403335 mentions this issue: cmd/go/internal/modfetch: cache proxy requests

@ianlancetaylor
Copy link
Contributor

@bcmills @matloob This issue is marked for 1.19. It's just been rolling forward in milestones. Should it move to Backlog? Thanks.

@bcmills
Copy link
Member

bcmills commented Jun 28, 2022

@oiooj had a fix pending review (thanks!), which has now been merged.

jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
The responses have been cached by the web2 package before removed
it in CL 170879. This change add latest revinfo cache in Versions
func.

Fixes golang#51391

Change-Id: I73597e0a6b4938238e69d85e1cbbaa9007776db3
Reviewed-on: https://go-review.googlesource.com/c/go/+/403335
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Lee Baokun <bk@golangcn.org>
Reviewed-by: David Chase <drchase@google.com>
@golang golang locked and limited conversation to collaborators Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. ToolSpeed
Projects
Development

No branches or pull requests

7 participants