Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

Conversation

@suejung-sentry
Copy link
Contributor

@suejung-sentry suejung-sentry commented Oct 21, 2024

We run branch search by name on the Configuration page of a repo - it's currently slow (~50 seconds in worst case, median ~10+seconds - Sentry board).

The branch name search is the GetBranches graphQL query and the below db query.

SELECT branches.branch, branches.repoid, branches.authors, branches.head, branches.base,
  branches.updatestamp
FROM branches
WHERE (
  branches.repoid = 111 AND UPPER(branches.branch::text) LIKE UPPER('%codecov%')
)
ORDER BY branches.updatestamp DESC
LIMIT 21;

I ran some tests against postgres prod and the EXPLAIN ANALYZE shows significant difference between running ILIKE vs. LIKE for the search (~10 ms vs. 5 seconds).
There is an index on branches.repoid and one on branches.repoid, branches.branch. From the query plans it looks like running with LIKE is running a custom columnar scan (full table scan on all 27 million rows of branches table) before then using the repoid index and filtering those rows by name. The query with ILIKE just uses the index then filters, so it's much faster.
explain analyze with LIKE:
Screenshot 2024-10-21 at 1 09 29 PM
explain analyze with ILIKE:
Screenshot 2024-10-21 at 1 08 10 PM

So this PR moves over to using ILIKE in the query. I suspect the reason for the difference between running with LIKE vs. ILIKE has to do with our collation settings for the table but those are risky to alter so I'd rather move to using ILIKE than trying to fix LIKE.

Other stuff:

  • Django's docs say the existing icontains methods translates to a query of ILIKE, but it clearly doesn't. Some troubleshooting says maybe it's because our DATABASE ENGINE settings.py is not set to postgres, but everything I checked seems to imply that is done already.
  • This from the Django forums seems to confirm their doc is just wrong. The Django folks recommend redefining the ORM hooks, but I gave it a shot and didn't do what I expected. Also seems risky to mess with it at that level.
  • So the alternative I went with here is the .extra api that gets us what we need. Technically the docs say to use it as a last resort, but it seems to be the best solution for this now without restructuring our ORM in bigger ways.
  • On the frontend we have a debounce (500 ms) for the query already, so unfortunately no further gains that way

Closes codecov/engineering-team#2537

@codecov-notifications
Copy link

codecov-notifications bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@codecov
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.26%. Comparing base (e30e3ed) to head (5771daa).
Report is 1 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #909   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files         823      823           
  Lines       19040    19042    +2     
=======================================
+ Hits        18329    18331    +2     
  Misses        711      711           
Flag Coverage Δ
unit 92.54% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 92.54% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@suejung-sentry suejung-sentry marked this pull request as ready for review October 21, 2024 23:24
@suejung-sentry
Copy link
Contributor Author

suejung-sentry commented Oct 23, 2024

RE: attempting to adjust the ORM overrides instead of using .extra as done in this PR - spun off this ticket to continue looking into it - codecov/engineering-team#2752 as that would be nicer if it worked.

Copy link
Contributor

@ajay-sentry ajay-sentry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@codecov codecov deleted a comment from codecov-qa bot Oct 23, 2024
@codecov codecov deleted a comment from codecov-public-qa bot Oct 23, 2024
@suejung-sentry suejung-sentry added this pull request to the merge queue Oct 23, 2024
Merged via the queue into main with commit cd1ef89 Oct 23, 2024
32 of 33 checks passed
@suejung-sentry suejung-sentry deleted the sshin/fix/2537 branch October 23, 2024 23:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default Branch Search is Too Slow

3 participants