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

Conversation

keegancsmith
Copy link
Member

@keegancsmith keegancsmith commented Jun 28, 2023

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
Contributor

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

@keegancsmith
Copy link
Member Author

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

@jhchabran
Copy link
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
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
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
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
Member Author

@jhchabran mind stamping? rebased :)

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

sourcegraph-bot commented Jun 29, 2023

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
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
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
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
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