Skip to content

Commit

Permalink
Auto merge of rust-lang#1752 - sgrif:sg-stricter-search-for-short-que…
Browse files Browse the repository at this point in the history
…ries, r=jtgeibel

Use a stricter form of search for extremely short queries

Since rust-lang#1560 we've had various performance problems with search. While
we've addressed most of the biggest issues, we still have poor
performance when the search term is 2 characters or longer (between
100-200ms at the time of writing this commit, though there is a PR being
merged which can regress this another 30%).

This level of performance isn't the end of the world, but the majority
of our 1 or 2 character searches come from a handful of crawlers which
hit us very frequently, and I'd prefer not to spend this much time on
queries for crawlers. (There's quite a few of these, they all do things
like search for 's', then 'se', then 'ser', etc over and over again --
User Agent only identifies the HTTP lib they used, which varies).

The performance issues come from two places. The first problem is that
we can't use our indexes when the search term is 2 characters, due to
how trigrams work. This means that we fall back to doing a sequential
scan of the entire table, which will only get worse as time goes on. For
single letter searches, the second issue comes from the sheer number of
rows we get back, which have to go into an expensive hash join.

If you search for 'a', you get back 13k results. At the end of the day,
getting every crate with 'a' in its name is not useful, so I've tried to
go with a solution that both improves our performance and also return
more useful results. The operator I've used is meant to return whether
any words are sufficiently similar, but since our search term is shorter
than a trigram the behavior is a little different. For 2 letter
searches, it ends up being "any word begins with the term", and for 1
letter searches, it's "any word is equal to the term". Here are some
example results for "do" and "a"

```
 do
 afi_docf
 alice-download
 async_docker
 avocado_derive
 cargo-build-docker
 cargo_crates-io_docs-rs_test
 cargo_crates-io_docs-rs_test2
 cargo-do
 cargo-doc-coverage
 cargo-docgen
 cargo-dock
 cargo-docker
 cargo-docker-builder
 cargo-docserve
 cargo-docserver
 cargo-doctor
 cargo-download
 cargo-external-doc
 cargo-pack-docker
 cargo-serve-doc
 devboat-docker
 doapi
 do-async
 doc
 doc_9_testing_12345
 docbase_io
 doc-cfg
 doc-comment
 doccy
 doc_file
 docker
 docker4rs
```

```
 a
 a-range
 cortex-a
 jacques_a_dit
 magic-number-a
 manish_this_is_a_test
 poke-a-mango
 vmx-just-a-test-001-maincrate
 wasm-bindgen-test-crate-a
```

Drawbacks
---

The original motivation for switching to `LIKE` in search was to make
sure libssh shows up when searching for ssh. This will regress that for
any `lib*` crates with less than 2 letter names. There aren't very many
of these:

- lib
- libc
- libcw
- libdw
- libgo
- libjp
- libm
- libnv
- libpm
- libr
- libs
- libsm
- libxm

I'm less concerned about the single letter cases, as those are already
going to be buried on page 87, but a few of the 2 letter cases you might
legitimately search for. None of these crates have high traffic, and
fixing this generally isn't really possible without introducing some
special case indexes *only* for this case. We could also work around
this by always searching for "lib*" in addition to whatever you searched
for.

This also means that searching for `a` will no longer include the crate
`a1`. I'm not as concerned about this, if you want all crates starting
with the letter a, we already have `/crates?letter=a` for that.

With this change, our performance should be back to reasonable levels
for all search terms.

Before
--

```
                                                                         QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=3895.90..3896.04 rows=20 width=882) (actual time=164.823..164.838 rows=20 loops=1)
   ->  WindowAgg  (cost=3895.90..3985.84 rows=12848 width=882) (actual time=164.821..164.832 rows=20 loops=1)
         ->  Sort  (cost=3895.90..3902.32 rows=12848 width=874) (actual time=155.012..156.425 rows=12599 loops=1)
               Sort Key: ((replace(lower((crates.name)::text), '-'::text, '_'::text) = 'a'::text)) DESC, crates.name
               Sort Method: quicksort  Memory: 9996kB
               ->  Hash Right Join  (cost=3410.76..3720.54 rows=12848 width=874) (actual time=95.457..116.592 rows=12599 loops=1)
                     Hash Cond: (recent_crate_downloads.crate_id = crates.id)
                     ->  Seq Scan on recent_crate_downloads  (cost=0.00..276.87 rows=25958 width=12) (actual time=0.012..2.753 rows=25958 loops=1)
                     ->  Hash  (cost=3365.79..3365.79 rows=12848 width=865) (actual time=95.417..95.417 rows=12599 loops=1)
                           Buckets: 16384  Batches: 1  Memory Usage: 6985kB
                           ->  Seq Scan on crates  (cost=0.00..3365.79 rows=12848 width=865) (actual time=0.015..85.416 rows=12599 loops=1)
                                 Filter: ((''::tsquery @@ textsearchable_index_col) OR (replace(lower((name)::text), '-'::text, '_'::text) ~~ '%a%'::text))
                                 Rows Removed by Filter: 13359
 Planning Time: 0.555 ms
 Execution Time: 165.998 ms
```

After
--

```
                                                                                   QUERY PLAN
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Limit  (cost=159.61..159.75 rows=20 width=882) (actual time=0.321..0.325 rows=9 loops=1)
   ->  WindowAgg  (cost=159.61..159.80 rows=26 width=882) (actual time=0.320..0.324 rows=9 loops=1)
         ->  Sort  (cost=159.61..159.63 rows=26 width=874) (actual time=0.310..0.311 rows=9 loops=1)
               Sort Key: ((replace(lower((crates.name)::text), '-'::text, '_'::text) = 'a'::text)) DESC, crates.name
               Sort Method: quicksort  Memory: 30kB
               ->  Nested Loop Left Join  (cost=10.10..159.49 rows=26 width=874) (actual time=0.076..0.288 rows=9 loops=1)
                     ->  Bitmap Heap Scan on crates  (cost=10.04..59.84 rows=26 width=865) (actual time=0.057..0.196 rows=9 loops=1)
                           Recheck Cond: ((''::tsquery @@ textsearchable_index_col) OR (replace(lower((name)::text), '-'::text, '_'::text) %> 'a'::text))
                           Heap Blocks: exact=9
                           ->  BitmapOr  (cost=10.04..10.04 rows=26 width=0) (actual time=0.046..0.046 rows=0 loops=1)
                                 ->  Bitmap Index Scan on index_crates_name_search  (cost=0.00..0.00 rows=1 width=0) (actual time=0.001..0.001 rows=0 loops=1)
                                       Index Cond: (''::tsquery @@ textsearchable_index_col)
                                 ->  Bitmap Index Scan on index_crates_name_tgrm  (cost=0.00..10.04 rows=26 width=0) (actual time=0.044..0.044 rows=9 loops=1)
                                       Index Cond: (replace(lower((name)::text), '-'::text, '_'::text) %> 'a'::text)
                     ->  Index Scan using recent_crate_downloads_crate_id on recent_crate_downloads  (cost=0.06..3.83 rows=1 width=12) (actual time=0.008..0.008 rows=1 loops=9)
                           Index Cond: (crate_id = crates.id)
 Planning Time: 0.553 ms
 Execution Time: 0.386 ms
```
  • Loading branch information
bors committed May 22, 2019
2 parents 960ced1 + 36893f6 commit 727788d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
2 changes: 1 addition & 1 deletion src/controllers/krate/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
query = query.filter(
q.clone()
.matches(crates::textsearchable_index_col)
.or(Crate::like_name(&q_string)),
.or(Crate::loosly_matches_name(&q_string)),
);

query = query.select((
Expand Down
29 changes: 22 additions & 7 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use chrono::NaiveDateTime;
use diesel::associations::Identifiable;
use diesel::pg::Pg;
use diesel::prelude::*;
use diesel::sql_types::Bool;
use url::Url;

use crate::app::App;
Expand Down Expand Up @@ -86,8 +87,6 @@ pub const MAX_NAME_LENGTH: usize = 64;
type CanonCrateName<T> = self::canon_crate_name::HelperType<T>;
type All = diesel::dsl::Select<crates::table, AllColumns>;
type WithName<'a> = diesel::dsl::Eq<CanonCrateName<crates::name>, CanonCrateName<&'a str>>;
/// The result of a loose search
type LikeName = diesel::dsl::Like<CanonCrateName<crates::name>, CanonCrateName<String>>;
type ByName<'a> = diesel::dsl::Filter<All, WithName<'a>>;
type ByExactName<'a> = diesel::dsl::Filter<All, diesel::dsl::Eq<crates::name, &'a str>>;

Expand Down Expand Up @@ -236,12 +235,28 @@ impl<'a> NewCrate<'a> {
}

impl Crate {
/// SQL filter with the `like` binary operator. Adds wildcards to the beginning and end to get
/// substring matches.
pub fn like_name(name: &str) -> LikeName {
let wildcard_name = format!("%{}%", name);
canon_crate_name(crates::name).like(canon_crate_name(wildcard_name))
/// SQL filter based on whether the crate's name loosly matches the given
/// string.
///
/// The operator used varies based on the input.
pub fn loosly_matches_name<QS>(
name: &str,
) -> Box<dyn BoxableExpression<QS, Pg, SqlType = Bool> + '_>
where
crates::name: SelectableExpression<QS>,
{
if name.len() > 2 {
let wildcard_name = format!("%{}%", name);
Box::new(canon_crate_name(crates::name).like(canon_crate_name(wildcard_name)))
} else {
diesel_infix_operator!(MatchesWord, "%>");
Box::new(MatchesWord::new(
canon_crate_name(crates::name),
name.into_sql::<Text>(),
))
}
}

/// SQL filter with the = binary operator
pub fn with_name(name: &str) -> WithName<'_> {
canon_crate_name(crates::name).eq(canon_crate_name(name))
Expand Down
4 changes: 4 additions & 0 deletions src/tests/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,10 @@ fn loose_search_order() {
for (lhs, rhs) in search_temp.crates.iter().zip(ordered) {
assert_eq!(lhs.name, rhs.name);
}

let search_temp = anon.search("q=te");
assert_eq!(search_temp.meta.total, 3);
assert_eq!(search_temp.crates.len(), 3);
}

#[test]
Expand Down

0 comments on commit 727788d

Please sign in to comment.