Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

search: update zoekt with removal of RepositoryRankRequest#54394

Merged
keegancsmith merged 4 commits into
mainfrom
k/update-zoekt
Jun 29, 2023
Merged

search: update zoekt with removal of RepositoryRankRequest#54394
keegancsmith merged 4 commits into
mainfrom
k/update-zoekt

Conversation

@keegancsmith

@keegancsmith keegancsmith commented Jun 28, 2023

Copy link
Copy Markdown
Member

This endpoint was unused which was why it was removed in Zoekt. The changes from Zoekt included in this dep bump:

Test Plan: CI

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith there are some issues with the back compat tests, I'll fix them tomorrow morning 👍

@keegancsmith

Copy link
Copy Markdown
Member Author

@jhchabran any luck? I'm just about to update this PR I would love to deploy soon.

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith I'm on it, stumbled across new problems when I bumped the target for the back compat tests to 5.1.

I haven't touch your PR and I don't think I'll have to, as it'll simply be about rebasing main once I get it working. I'll keep in touch if it takes me too long 🙏

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith I think it might take me more time to fix this, though I should have it done before EOD.

What about this: you disable the backcompat tests for now, I'll re-enable them on my side. This way you can keep moving with this PR.

@keegancsmith

Copy link
Copy Markdown
Member Author

@jhchabran I will do that approach if I'm still blocked tomorrow morning. For now I am context switched onto something else :)

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith my PR is up, so once it's merged, I should be able to ensure this one is green as well :)

This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch

Test Plan: CI
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups
@keegancsmith

Copy link
Copy Markdown
Member Author

@jhchabran mind stamping? rebased :)

@keegancsmith keegancsmith enabled auto-merge (squash) June 29, 2023 16:27
@sourcegraph-bot

sourcegraph-bot commented Jun 29, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 2ec4a57...1d0d79e.

Notify File(s)
@camdencheek internal/search/backend/metered_searcher.go
@jtibshirani internal/search/backend/metered_searcher.go

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith I'm running a branch with the same thing, it's broken, need to pin the zoekt dep for the backcompat, incoming fix, gimme a minute :)

@keegancsmith

Copy link
Copy Markdown
Member Author

no worries, FYI I force pushed just before making that comment with the rebase + one extra tiny change to include something new from zoekt in a trace event.

@jhchabran

Copy link
Copy Markdown
Contributor

@keegancsmith it passed on my branch, pushed the commit here :)

@keegancsmith keegancsmith merged commit 49aff3d into main Jun 29, 2023
@keegancsmith keegancsmith deleted the k/update-zoekt branch June 29, 2023 17:17
@github-actions

Copy link
Copy Markdown
Contributor

The backport to 5.1 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-5.1 5.1
# Navigate to the new working tree
cd .worktrees/backport-5.1
# Create a new branch
git switch --create backport-54394-to-5.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 49aff3d61c8479a296ba3379e7cabab97635be07
# Push it to GitHub
git push --set-upstream origin backport-54394-to-5.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-5.1

Then, create a pull request where the base branch is 5.1 and the compare/head branch is backport-54394-to-5.1.

@github-actions github-actions Bot added backports failed-backport-to-5.1 release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases labels Aug 18, 2023
ggilmore pushed a commit that referenced this pull request Aug 21, 2023
This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI

(cherry picked from commit 49aff3d)
camdencheek pushed a commit that referenced this pull request Aug 21, 2023
…uest (#54394) (#56077)

search: update zoekt with removal of RepositoryRankRequest (#54394)

This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI

(cherry picked from commit 49aff3d)
burmudar pushed a commit that referenced this pull request Aug 24, 2023
This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI
burmudar pushed a commit that referenced this pull request Aug 24, 2023
This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI
burmudar pushed a commit that referenced this pull request Aug 24, 2023
This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI
jdpleiness pushed a commit that referenced this pull request Aug 24, 2023
…e ngram sort optimization (#56200)

* gomod: update zoekt to include ngram sort optimization (#54999)

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>

* search: update zoekt with removal of RepositoryRankRequest (#54394)

This endpoint was unused which was why it was removed in Zoekt. The
changes from Zoekt included in this dep bump:

- 1686b50d50 indexserver: remove unused GetRepoRank
- 7078a585a9 shards: populate RepoList.Stats.Repos
- b9e6d9433e zoekt-indexserver: Check stderr for git fetch
- 93f7b0c983 matchtree: capture Stats before pruning
- 7643f3b313 matchiter: capture metric NgramLookups

Test Plan: CI

* fix bazel deps and patches

* fix bad merge and update backcompat zoekt

* update go.sum for github.com/jackc/pgproto3/v2

* fix go.sum dependency

* restore backcompat throttled repo

* Revert "restore backcompat throttled repo"

This reverts commit e5f66b3.

* Fix backcompat by porting #54748

* allow mozilla 2.0 license for retryablehttp

---------

Co-authored-by: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
Co-authored-by: ggilmore <geoffrey@sourcegraph.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backports cla-signed release-blocker Prevents us from releasing: https://about.sourcegraph.com/handbook/engineering/releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants