Skip to content

Limit the number of results requested when searching for commits

What does this MR do?

Improve performance of non-Elasticsearch commit search by limiting the number of results requested.

In #216908 (closed), it was pointed out that limit is not passed when we call find_commits_by_message. If not given, find_commits_by_message defaults the limit to 1000, but we don't actually need that many results right away.

This MR changes commit search to use the same limiting strategy that is used by blob search. We start by limiting to 100 results (defined by the COUNT_LIMIT constant), and increase the limit as additional pages are requested.

Performance test results for web search (60s_20rps against development environment)

Before
✓ { endpoint:commits_count }...: avg=215.78ms  min=149.52ms  med=168.04ms  max=1090.57ms p(90)=271.44ms  p(95)=293.89ms 
✗ { endpoint:commits }.........: avg=1440.24ms min=1286.12ms med=1413.65ms max=1996.03ms p(90)=1528.53ms p(95)=1675.51ms
After
✓ { endpoint:commits_count }...: avg=198.30ms  min=150.31ms  med=175.71ms  max=738.41ms  p(90)=279.20ms  p(95)=302.85ms 
✗ { endpoint:commits }.........: avg=1007.94ms min=866.59ms  med=996.73ms  max=1666.52ms p(90)=1103.04ms p(95)=1118.90ms

~28% improvement in p90 response times when searching for commits. The commits_count endpoint seems relatively unchanged.

A change was also made to the formatting of the number of commit results, to make it consistent with other scopes. This is shown in the screenshots below.

Screenshots

Before After
Screen_Shot_2020-05-21_at_2.43.01_PM Screen_Shot_2020-05-21_at_2.44.28_PM

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team

Closes #216908 (closed)

Edited by 🤖 GitLab Bot 🤖

Merge request reports

Loading