[backport 5.1] gomod: update zoekt to include ngram sort optimization (#54999)#56088
[backport 5.1] gomod: update zoekt to include ngram sort optimization (#54999)#56088ggilmore wants to merge 7 commits into
Conversation
Main motivation is to see the effect on performance for attribution search. Note that this included a bump in the otel version used since zoekt is using a new version. This had a bit of a cascading effect on our third party deps since they removed a package. So this bumps most 3rd party packages that directly interacted with otel. The changed commits are sourcegraph/zoekt@7643f3b...45f608f - a176bde1a3 go get -u -t ./... - e2e8aede00 Fix template documentation comments - 25c1ea5177 all: observe missing Stats RegexpsConsidered and FlushReason - 9abbb8b0d3 zoekt-indexserver: Prevent invalid config from causing an NPE - 008a775ba8 zoekt-indexserver: use value format directive for bad conf warning - f9d3a0e2e4 zoekt: add fgprof for full profiling - 3d0bdd5c9c remove ngram offset code - 45f608ff95 sort ngrams before looking them up Test Plan: tested in the zoekt repo. Our CI will handle the dep updates. I eyeballed them and they all look low risk. Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr> (cherry picked from commit 766a8e1)
…patch"' This file doesn't exist on the 5.1 branch, and only seems to affect PRs on main. See https://github.com/sourcegraph/sourcegraph/pull/54486
a8ca2e6 to
8522805
Compare
|
@ggilmore I fixed this by regenerating the patches and did a test run here https://buildkite.com/sourcegraph/sourcegraph/builds/240089#018a26fa-8315-4871-86da-59a3313c47c8 |
|
@bobheadxi I remember you following up a bit with an OTEL PR after I bumped some of the deps. Do we need to backport that as well? I think the PR was #54969 |
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 974669e...5cfa260.
|
|
We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added. Sourcegraph teammates should refer to Accepting contributions for guidance. |
|
Codenotify: Notifying subscribers in OWNERS files for diff 974669e...5cfa260.
|
This is an automated pull request generated by [this run](https://github.com/sourcegraph/sourcegraph/actions/runs/5578826248). Learn more about our GitHub Actions for managing licenses [here](https://docs.sourcegraph.com/dev/background-information/ci#third-party-licenses). You're safe to merge this pull request when the required checks are passing. Test plan: CI should pass with any updates.
b90c137 to
358e1e0
Compare
This is what we have on the main branch
Cherry-picking is not as fun as copy paste.
|
@ggilmore @burmudar this is passing CI now. Should we hold of until the next patch release, or land this now? Is it alright if this goes in before https://github.com/sourcegraph/sourcegraph/pull/56200 |
|
@keegancsmith I am happy to merge this, but I don't want to derail @burmudar 's work without his ok. It seems unlikely that his fixes will land before this patch anyway and I don't want him to crunch. This PR on its own isn't useful for me, it's a stepping stone for being able to merge in the other zoekt dependency updates with the gRPC fixes. It seems unlikely that we can merge all of those in before the release, so I am okay with closing this one. |
Main motivation is to see the effect on performance for attribution
search.
Note that this included a bump in the otel version used since zoekt is
using a new version. This had a bit of a cascading effect on our third
party deps since they removed a package. So this bumps most 3rd party
packages that directly interacted with otel.
The changed commits are
sourcegraph/zoekt@7643f3b...45f608f
Test Plan: tested in the zoekt repo. Our CI will handle the dep updates.
I eyeballed them and they all look low risk.
Co-authored-by: Jean-Hadrien Chabran jh@chabran.fr
(cherry picked from commit 766a8e1)
Test plan