zoekt/shards/ at 931974dd953e07ce8fbe28e3b615323c2d156e40 · boltless.me/zoekt · Tangled

boltless.me / zoekt

0

fork of https://github.com/sourcegraph/zoekt

0

zoekt / shards /

at 931974dd953e07ce8fbe28e3b615323c2d156e40 9 files

Keegan Carruthers-Smith shards: respect scheduler and use smarter synchronization for List (#750) 2y ago

shards: respect scheduler and use smarter synchronization for List (#750) Previously List would never call proc.Yield, which broke the co-operative scheduler in the case of slow List calls. Additionally, we used a naive concurrency (a large buffered channel) which shows up in the profiler as 30% of CPU spent on chan_ related operations under List. This commit follows how Search used to respect proc.Yield. See sched.go in 90ed7bfd07e0a3137e3ee627cbf6824446df9c4d. We did not copy Search since it uses a more complicated implementation than we need since it supports streaming, while List is still batch only. We needed to use errgroup to ensure we drained all channels in the case of an error. Previously we did not need to do this since the channels had a buffer size of len(shards), which gaurenteed nothing would ever block. Now channels are never larger than the number of workers (<= GOMAXPROCS). Test Plan: go test covers the no error cases. In the case of errors we manually tested by running zoekt-webserver and adding a random context cancellation. We observed the error being reported and no List goroutines running. Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>

2 years ago

shards: always aggregate stats for tracing (#729) Many a time I have seen a slow search without tracing turned on. I then go to visit the net/trace page to see what happened but it is missing the stats to try work out why. This commit will ensure we always log the aggregated statistics. This should be cheap to do given Add is relatively fast and LazyPrintf will only do the stringer operation if someone loads the debug page. It does mean a Stats object lives a bit longer in memory, but it is small. Test Plan: go test

2 years ago

all: rename module to github.com/sourcegraph/zoekt We are a hard enough for now that we might as well have our own module name. This will also make it possible to more directly use go tooling.

3 years ago

scheduler: add some clarifying comments (#508)

3 years ago

all: fix errcheck lints in test files

4 years ago

shards: respect scheduler and use smarter synchronization for List (#750) Previously List would never call proc.Yield, which broke the co-operative scheduler in the case of slow List calls. Additionally, we used a naive concurrency (a large buffered channel) which shows up in the profiler as 30% of CPU spent on chan_ related operations under List. This commit follows how Search used to respect proc.Yield. See sched.go in 90ed7bfd07e0a3137e3ee627cbf6824446df9c4d. We did not copy Search since it uses a more complicated implementation than we need since it supports streaming, while List is still batch only. We needed to use errgroup to ensure we drained all channels in the case of an error. Previously we did not need to do this since the channels had a buffer size of len(shards), which gaurenteed nothing would ever block. Now channels are never larger than the number of workers (<= GOMAXPROCS). Test Plan: go test covers the no error cases. In the case of errors we manually tested by running zoekt-webserver and adding a random context cancellation. We observed the error being reported and no List goroutines running. Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>

2 years ago

all: gofumpt -l -w . gofumpt is a stricter gofmt. I took a look at the changes and in general they are nice. I don't think we need to enforce the use of gofumpt, but I like the idea of running it every once in a while. Test Plan: go test ./...

2 years ago

Fix no-shards-loaded being reported as crashes (#512) Full debug context here: https://sourcegraph.slack.com/archives/C023ELQLV7F/p1672835804462349 Commit 9899a9b3f475ef066ed70c395f8b303268f5d00c changed what the response looks like when Zoekt has never loaded a shard: it now reports a `Crashes = 1`, even if everything's fine. That leads to upstream errors where we show an error message in the Sourcegraph admin UI because the customer hasn't added any repositories to their instance yet. The fix here changes the `loader` to also mark the `shardedSearcher` as ready if there was nothing to load. Previously the `markReady` was skipped, the searcher wasn't marked as "ready" and every search query was replied to with a `Crashes = 1`

3 years ago

watcher_test.go

all: gofumpt -l -w . gofumpt is a stricter gofmt. I took a look at the changes and in general they are nice. I don't think we need to enforce the use of gofumpt, but I like the idea of running it every once in a while. Test Plan: go test ./...

2 years ago