Skip to content

Search backend: remove `PriorityJob`

Warren Gifford requested to merge cc/remove-priority-job into main

Created by: camdencheek

This removes PriorityJob because the logic it represents is almost never exercised and it adds complexity to constructing our job tree.

Stacked on https://github.com/sourcegraph/sourcegraph/pull/34774

Discussion:

We currently have a job called PriorityJob. Its purpose is to run groups of jobs in parallel, but cancel the optional group of jobs soon after the required group finishes. This allowed us to run potentially expensive or long-running jobs without blocking returning search results for an unreasonably long amount of time.

However, now that our backend is fully stream-enabled, I've convinced myself that this almost never does anything useful anymore and only adds complexity. Explanation below.

The only jobs that are ever optional are symbol and commit search. The only time these are optional is when useFullDeadline is false or when they are the only result type requested. useFullDeadline is true whenever count: is set, whenever timeout: is set, and whenever we are using the streaming API.

All UI-generated search requests use the streaming API, meaning that no user search through the UI will ever create an optional job. Additionally, even for the GraphQL API, only searches like type:commit type:repo test will ever generate optional jobs. However, even for GraphQL searches, we stream all the way through the backend and just collect results right before sending them in the HTTP response. In particular, things like limits and timeouts apply to the whole job tree, and if we hit a limit with results from a faster-running job, we will cancel all other jobs immediately. If we don't hit a limit with our required job and it returns quickly (for example, type:repo type:commit sample commit message), I think it's better behavior to continue searching commits until we hit the default limit (because remember, if there is a non-default limit with count:, no optional jobs will ever be created).

TL;DR: it is difficult to even create optional jobs on purpose, and when they are created I don't think they do what most people will want.

Test plan

Manually checked regenerated autogold tests. Pushed backend integration branch

Merge request reports

Loading