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: importedby number and list don't agree #44965

Closed
danp opened this issue Mar 12, 2021 · 4 comments
Closed

x/pkgsite: importedby number and list don't agree #44965

danp opened this issue Mar 12, 2021 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite

Comments

@danp
Copy link
Contributor

danp commented Mar 12, 2021

(Opening from a convo in Slack)

https://pkg.go.dev/modernc.org/sqlite?tab=importedby says Known importers: 16 but the number of items in the list may not agree.

@julieqiu suggested it might be a difference between these places:

https://github.com/golang/pkgsite/blob/c16d5e9c5f440307d9cc4ca393d3ced9f25dbc20/internal/postgres/details.go#L80

https://github.com/golang/pkgsite/blob/5732477f0a15fbf76e03ab057305865af7547a3e/internal/postgres/search.go#L623

It looks like there is a bug in that we filter out certain paths when computing the count, which leads to the discrepancy.

@gopherbot gopherbot added this to the Unreleased milestone Mar 12, 2021
@jamalc jamalc added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 12, 2021
@jamalc jamalc modified the milestones: Unreleased, pkgsite/unplanned Mar 12, 2021
@jba
Copy link
Contributor

jba commented Mar 17, 2021

The fundamental reason for the discrepancy is that the importer count we use for search has a different purpose than the one displayed on the imported-by tab.

The count for search is used for ranking, so it is adjusted to produce a better ranking. In particular, it excludes importers that are not themselves in our set of search documents, such as internal packages (like github.com/frioux/leatherman/internal/notes) and modules whose import path differs from their go.mod file (like github.com/circonus-labs/circonus-unified-agent).

The imported-by tab shows the search count, leading to an obvious mismatch with the list of importers. However, if it displayed the actual number of importers in the list, then it there would still be a mismatch between that number and what you see in search.

Here are some fixes:

  1. Drop the condition when counting importers for search. Search quality will be reduced, but by how much? We could figure that out.
  2. Exclude the same importers on the imported-by tab. Not great if you're trying to track down users of a package.
  3. Use the adjusted importers for search ranking, but display the unadjusted number.

In any case, I think we should go back to displaying the actual length of the list on the imported-by tab. At least the discrepancy with the search results won't be quite as obvious.

@jba
Copy link
Contributor

jba commented Mar 17, 2021

Re point 1 above, the differences between raw and adjusted importers can be significant. For an extreme example, k8s.io/apimachinery/pkg/types has about 46,000 importers, but only about 18,000 of those are in the search table, presumably because there are many clones of that module.

It's still not clear how much searching a particular term would be affected by the changes; that is much harder to quantify.

@gopherbot
Copy link

Change https://golang.org/cl/302949 mentions this issue: internal/frontend: make imported-by tab count match list

gopherbot pushed a commit to golang/pkgsite that referenced this issue Mar 19, 2021
The number shown in the imported-by tab should match the number of
packages actually displayed.

That might contradict the search result number, but at least it is
consistent with what is on the same page.

For golang/go#44965

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

jba commented Apr 4, 2021

We now show both counts on the importers page.

@jba jba closed this as completed Apr 4, 2021
@golang golang locked and limited conversation to collaborators Apr 4, 2022
@rsc rsc unassigned jba Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. pkgsite
Projects
None yet
Development

No branches or pull requests

4 participants