Search backend: remove `PriorityJob`
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