A query for name=%13 on the explore causes statement timeouts

added groupproject management infradev security labels

As discussed with @albertoramos during the call, I'm marking this as a priority1 severity1 for now since we did notice a degradation in the service when this was happening.

added priority1 severity1 labels

changed the description

added corrective action label

@gweaver @donaldcook @jlear @cmaxim This issue is ready for triage as per HackerOne process.

About this automation: AppSec Escalation Engine

added security-set-milestone label

changed due date to November 01, 2020

@reprazent thank you for opening this. There is a parallel discussion to be had about the future of explore on GitLab.com. https://gitlab.com/gitlab-org/gitlab/-/issues/260334

Could we just ensure we run .strip on that search string?

It's a bit late here so I haven't delved too far but we could try adding it here.

Because this search string is less than min_chars_for_partial_matching (3), it does a query like:

SELECT "projects".* FROM "projects" WHERE (EXISTS (SELECT 1 FROM "project_authorizations" WHERE "project_authorizations"."user_id" = 2880930 AND (project_authorizations.project_id = projects.id)) OR projects.visibility_level IN (10,20)) AND (("projects"."path" ILIKE '' OR "projects"."name" ILIKE '') OR "projects"."description" ILIKE '') AND "projects"."archived" = FALSE ORDER BY "projects"."last_activity_at" DESC LIMIT 21 OFFSET 0

(There's a non-printable character there)

For some reason, it's fast on #database-lab: https://explain.depesz.com/s/oJtr Perhaps Slack is filtering the character and it's just searching by empty string.

In general though, searching with only 1 character is slow. Like if I search for a: https://explain.depesz.com/s/KDar

The search for a does not timeout on GitLab.com but other single characters timeout. Like symbols $, !, etc.. Query plan for searching for $: https://explain.depesz.com/s/icMP

In that case could we just block searching for single-character strings? Is there a use-case for it being possible?

@gitlab-org/database-team Hi - I'm hoping y'all might be able to help us understand the performance @engwan referenced here: #260330 (comment 423009510)

I think the appropriate immediate path forward to resolve this problem is to limit this search to 3 or more characters, but I'm not entirely sure why this is fast on #database-lab, so I just want to make sure that putting a minimum on the term length will resolve the slowness.

Thank you @jlear,

As far as I can see, those queries are so slow because they access too much data.

From the query plans that @engwan posted for a and $:

$ --> accesses 14,413,195 entries of the trigram (text) index

->  Bitmap Index Scan using index_projects_on_description_trigram  
    (actual time=107793.930..107793.930 rows=14413195 loops=1)
     Index Cond: (projects.description ~~* '$'::text)
     Buffers: shared hit=4 read=119672
     I/O Timings: read=82133.545

a --> accesses 587,098 entries of the trigram (text) index but there are also matches in the other indexes so more time is spent above during the scan of the results and applying the filter:

...
->  Bitmap Heap Scan on public.projects  
   (actual time=581.479..13424.452 rows=230 loops=1)
     Filter: ((NOT projects.archived) AND 
              ((alternatives: SubPlan 1 or hashed SubPlan 2) 
               OR (projects.visibility_level = ANY ('{10,20}'::integer[]))))
     Rows Removed by Filter: 1453
     Buffers: shared hit=3536 read=379455 dirtied=11983 written=1169
     I/O Timings: read=8275.779 write=45.462

    ... ...
    
    ->  Bitmap Index Scan using index_projects_on_description_trigram  
        (actual time=295.020..295.021 rows=587098 loops=1)
         Index Cond: (projects.description ~~* 'a'::text)
         Buffers: shared hit=618 read=1126
         I/O Timings: read=45.418

...

I think that we should limit the minimum length of the search string sent back with this query. I am not sure what the limit should be, but the query provided does not even work for the most common English trigrams.

Edit: I understand that we switch to a different type of query above 3 characters, but FYI the query provided goes bellow 5sec execution times in #database-lab at 5 character strings and above.

The initial plan by @engwan was fast on #database-lab because, as correctly guessed, the empty string was sent:

...
Filter: ((NOT projects.archived) AND (((projects.path)::text ~~* ''::text) OR ((projects.name)::text ~~* ''::text) OR (projects.description ~~* ''::text)) AND ((alternatives: SubPlan 1 or hashed SubPlan 2) OR (projects.visibility_level = ANY ('{10,20}'::integer[]))))
...

I run an explain using a and I got a more suitable for #database-lab execution time of 396 seconds (plan)

Thanks @iroussos! I'll update the ticket to add a minimum to the string length.

Edit: I understand that we switch to a different type of query above 3 characters, but FYI the query provided goes bellow 5sec execution times in #database-lab at 5 character strings and above.

Yeah, for 3 characters and above it becomes a partial search like: ILIKE '%SEARCHSTRING%'.

So is it fine to set a minimum of 3 characters for this search?

Setting label(s) devopsplan sectiondev based on groupproject management.

added devopsplan sectiondev labels

@gweaver This severity1 security issue has no milestone yet. (For remediation goals, please see Severity and Priority Labels on ~security Issues.)

About this automation: AppSec Escalation Engine

changed milestone to %13.6

removed security-set-milestone label

added [deprecated] Accepting merge requests label

A query for name=%13 on the explore causes statement timeouts

Designs

Child items ...

Activity