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

x/pkgsite: fails to update latest to reflect latest version #46985

Open
kortschak opened this issue Jun 30, 2021 · 12 comments
Open

x/pkgsite: fails to update latest to reflect latest version #46985

kortschak opened this issue Jun 30, 2021 · 12 comments

Comments

@kortschak
Copy link
Contributor

@kortschak kortschak commented Jun 30, 2021

What is the URL of the page with the issue?

https://pkg.go.dev/gonum.org/v1/gonum@v0.9.3

What is your user agent?

Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

Screenshot

Screenshot from 2021-06-30 21-21-48

Screenshot from 2021-06-30 21-22-02

Screenshot from 2021-06-30 21-26-46

What did you do?

Navigate to the linked page.

What did you expect to see?

The LATEST text mark on the latest version.

What did you see instead?

A "GO TO LATEST" link on the latest version pointing to the previous version and the previous version being marked as latest.

@gopherbot gopherbot added this to the Unreleased milestone Jun 30, 2021
@jamalc
Copy link

@jamalc jamalc commented Jul 12, 2021

Fixed.

@jamalc jamalc closed this Jul 12, 2021
@mvdan
Copy link
Member

@mvdan mvdan commented Aug 6, 2021

@jamalc how was this fixed? I'm seeing it with https://pkg.go.dev/github.com/ipld/go-car/v2@v2.0.1, which still points "latest" to v2.0.0, even though v2.0.1 was out over fifteen minutes ago.

The versions tab also insists that v2.0.0 is the latest:
image

Reopening as, at the very least, this seems like a UI/UX bug to me.

@mvdan mvdan reopened this Aug 6, 2021
@mvdan
Copy link
Member

@mvdan mvdan commented Aug 6, 2021

Note that v2.0.0 lacks a LICENSE file, due to a mistake on my part, but v2.0.1 has one. I don't think that should matter here.

@mvdan
Copy link
Member

@mvdan mvdan commented Aug 6, 2021

Two hours later, and the "latest" info is still outdated, so it's certainly not a cache problem.

@jamalc jamalc self-assigned this Aug 6, 2021
@jamalc jamalc removed this from the Unreleased milestone Aug 6, 2021
@jamalc jamalc added this to the pkgsite/unplanned milestone Aug 6, 2021
@jamalc jamalc assigned jba and unassigned jamalc Aug 6, 2021
@jamalc
Copy link

@jamalc jamalc commented Aug 6, 2021

I had fixed this for gonum.org/v1/gonum by reprocessing the latest version of the module but there might be an issue with how we're calculating latest. Looking into it now.

@mvdan
Copy link
Member

@mvdan mvdan commented Aug 6, 2021

What might be different about my module is that I pushed the newer tag, and instead of waiting for pkgsite to naturally pick it up some time later, I manually navigated to https://pkg.go.dev/github.com/ipld/go-car/v2@v2.0.1 to force it to see the new tag.

I guess that's not what happens with most newer tag versions for modules. But I also didn't want to wait for 20m for the new version to be picked up :)

@cespare
Copy link
Contributor

@cespare cespare commented Aug 6, 2021

Just as a general matter, if someone reports a processing bug, and we do some kind of manual one-off repair, it doesn't seem like we should close the issue if there's no reason to believe the underlying bug was fixed, right? Or am I missing something?

@jba
Copy link
Contributor

@jba jba commented Aug 6, 2021

When we process a module, we follow these steps:

  1. Query the proxy to find the latest version information, and update our DB with that.
  2. Download the module zip and process it.

Normally that is fine: we find out about most modules from the index, so the proxy already knows about the module version by the time we see it.

But with "frontend fetch," where a user requests the module pkg.go.dev, that may not be true: the proxy may not learn about the new version until we ask for the zip in step 2, after we've asked the proxy about the latest version. Hence the odd-looking logs:

LatestModuleVersions("github.com/ipld/go-car/v2") => (raw="v2.0.0" cooked="v2.0.0", <nil>)
github.com/ipld/go-car/v2: not updating latest module versions
...
fetch.FetchModule succeeded for github.com/ipld/go-car/v2@v2.0.1
...

We should probably get the zip first, then ask for latest versions, then process it. That will require a bit of refactoring.

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 6, 2021

Change https://golang.org/cl/340123 mentions this issue: internal/worker: on fetch, hit proxy info first

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 12, 2021
Ask the proxy for info on the requested module version before doing
anything else, even computing the latest versions. This will make the
proxy aware of the version if it isn't already.

This prevents the failure mode described in the linked issue: the user
does frontend fetch on a new, latest version before the proxy sees it,
so the latest-version info doesn't know about it.

For golang/go#46985

Change-Id: I681da5404cea2d391836d876ac807b714ce88f90
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/340123
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Aug 13, 2021

Change https://golang.org/cl/341892 mentions this issue: internal/worker: insert module if not in module_version_states

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 13, 2021

Change https://golang.org/cl/341891 mentions this issue: migrations: drop module_version_states.index_timestamp not null

@gopherbot
Copy link

@gopherbot gopherbot commented Aug 13, 2021

Change https://golang.org/cl/341860 mentions this issue: internal/postgres: change UpsertModuleVersionStates to update

gopherbot pushed a commit to golang/pkgsite that referenced this issue Aug 13, 2021
When a module is requested for the first time through frontend fetch,
the index_timestamp should be null, since the module has never appeared
in the index before.

Drop the not null constraint on module_version_states.index_timestamp to
make this possible.

For golang/go#46985

Change-Id: I7f08c35307aec485fb212e490031e809971c9341
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/341891
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 14, 2021
If a module is introduced for the first time to pkgsite through frontend
fetch, and not the index, it won't have a row in module_version_states.

A row is now inserted before the fetch process begins if the proxy's
info endpoint tells us that this is a valid module.

This fixes a bug in the fetch flow where
module_version_states.last_processed_at is not updated in
upsertModuleVersionState if a row did not already exist.

For golang/go#46985

Change-Id: I2a166228df20b1fb935bbabebb5a651da0e2c1ba
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/341892
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
gopherbot pushed a commit to golang/pkgsite that referenced this issue Sep 14, 2021
UpsertModuleVersionStates is changed to UpdateModuleVersionStates. There
should never be a situation where UpsertModuleVersionStates is called
and a row does not already exist for that module.

If that happens, an error is now returned.

For golang/go#46985
Fixes golang/go#39628

Change-Id: I357396cee6eb743513ae249609f76f4cd4c19e9b
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/341860
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants