Tags: doovemax/sourcegraph
Tags
Address Postgres test plan feedback (sourcegraph#2195) * docs: Emphasize Postgres upgrade docs CTA * Emojify Postgres docs * cmd/server: Improve UX of error messages in Postgres upgrade * Document downtime requirement * postgres: Fix permissions of upgrade dir * De-emojify * More clear switch statement logic * Much clearer upgrade instructions * Update doc/admin/postgres.md Co-Authored-By: tsenart <[email protected]> * Update doc/admin/postgres.md
search: significantly improve the performance of file search suggesti… …ons (sourcegraph#2177) * search: significantly improve the performance of file search suggestions This change significantly improves the performance and code quality of the implementation providing file search _suggestions_. It reduces IO load on gitserver, but primarily removes load from repo-updater and redis which occurred due to a bug in the old implementation trying to update all repositories involved in the search. Fixes sourcegraph#2089 **How?** Most importantly, the old implementation issued requests erroneously which caused repo-updater, redis, and gitserver more IO load than was optimal. You can read more about this in the linked issue above. File search suggestions now also benefit from the same zoekt indexing that we do for search otherwise, because we are now using the same code path that provides actual search results. In `sourcegraph/server` instances, which do not use indexed search by default, performance is roughly identical to before. **Code quality gains** Prior to this change file search suggestions were provided by an entirely separate code path. This old code path was responsible for listing all files in every repository your query matched, then matching those files against your search query. In contrast, after this change: 1. File search suggestions are provided by the same code-path which provides text search results. 2. Because the code-path is the same, you no longer get different ordering in search suggestions compared to when you actually run the query. **Change in fuzzy matching behavior / more consistent matching** Before this change, you could type `src/http.go` (internally converted to `file:src/http.go`) and get fuzzy suggestions matching `src/net/http/http.go` even though your query was missing the `net/http` part of the path. After this change, you must type a substring match, or use regex. i.e. `src/http.go` will now not fuzzy match / provide any suggestions, but these would: - `http.go` - `http/http.go` - `net/http/http.go` - `src/net/http/http.go` - `src.*http.go` The prior behavior was better in some regards because it acted more like your editor quick file open panel. However, the inconsistency with the results you would get when running your query was also bad. For example: 1. Visit https://sourcegraph.com/github.com/sourcegraph/[email protected] 2. Type `file:dev/main.go` in the search input and you will see two suggestions. 3. Actually press enter and run the query, you will recieve no results! We now have consistency and a shared implementation. In the future, we could gain this behavior back by changing the `file:` match operator to do fuzzy editor-like matching, instead of (or in addition to) using regex. However, it is unclear to me if doing that would be worth it or if this behavior is generally good enough (I suspect it is).
server: Copy all built binaries We don't need to individually list each binary, we can just copy all of them.
gitserver: suppress false clone error log message (sourcegraph#1948) * gitserver: suppress false clone error log message This code used to assume `req.URL` existed and try to clone it. In reality, it only exists in certain circumstances. The end result (request failure) was the correct behavior, and that behavior remains, but attempting the clone is needless and results in a log message that is very misleading and looks like a bug on our end. Unless you want to understand how I've confirmed this, etc. you may stop reading here. :) Under some circumstances (such as improperly configured SSH cloning), I have observed errors from gitserver like: ``` t=2019-01-16T07:25:26+0000 lvl=dbug msg="error cloning repo" repo=github.com/slimsag/private-repo err="error cloning repo: repo github.com/slimsag/private-repo ([email protected]:slimsag/private-repo.git) not cloneable: exit status 128 (output follows)\n\nHost key verification failed.\r\nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.\n" t=2019-01-16T07:25:26+0000 lvl=dbug msg="error cloning repo" repo=github.com/slimsag/private-repo err="error cloning repo: repo github.com/slimsag/private-repo () not cloneable: exit status 128 (output follows)\n\nfatal: no path specified; see 'git help pull' for valid url syntax\n" ``` Fundamentally, the problem here is that there is an inaccessible repository (e.g. the user has not configured authentication for that repo correctly). However, the second log message is very strange in nature and looks like we might have a serious bug: why would we try to clone something via git without specifying the URL? I've spent a long time debugging this and trying to track down the cause, and a user also thought this might indicate a more serious issue somewhere. Per the documentation on [`gitserver.Repo.URL`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@e6d353a1a6c7282da4c68573b6fafc7cdccc32fc/-/blob/pkg/gitserver/client.go#L224-227), the URL field is optional and not always present: ```Go // URL is the repository's Git remote URL. If the gitserver already has cloned the repository, // this field is optional (it will use the last-used Git remote URL). If the repository is not // cloned on the gitserver, the request will fail. URL string ``` Looking at [`gitserver.Repo`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@e6d353a1a6c7282da4c68573b6fafc7cdccc32fc/-/blob/pkg/gitserver/client.go#L221:6&tab=references) references shows this is indeed often the case. The behavior we have today when executing git commands can be simply summarized as: "If the repository doesn't exist, we try to clone it. If we can't clone it, the git exec request fails." But how does that actually happen in practice? During git exec requests, [we clone the repo if the directory doesn't exist](https://sourcegraph.com/github.com/sourcegraph/sourcegraph@e6d353a/-/blob/cmd/gitserver/server/server.go#L565-568): ```Go if !repoCloned(dir) { cloneProgress, err := s.cloneRepo(ctx, req.Repo, req.URL, nil) if err != nil { log15.Debug("error cloning repo", "repo", req.Repo, "err", err) ``` And therein lies the problem: we don't have a `req.URL` and yet we're trying to clone using it. * make errcheck linter happy
PreviousNext