Skip to content

Resolve "Reduce Redis usage from merge request diffs caching"

Sean McGivern requested to merge 44191-reduce-redis-usage-from-merge-request-diffs-caching into master

What does this MR do?

  1. First commit: change to only caching the highlight results of the latest MR diff, and explicitly delete older highlight results.
  2. Second commit: reduce the TTL to a week, instead of the default two weeks.

Between them, these reduce the amount of memory we'll use on these highlighted diff results in Redis.

As an example, I had these cached MR diffs:

redis /Users/seanmcgivern/Code/gitlab-development-kit/redis/redis.socket> keys cache:gitlab:merge_request_diffs/*
1) "cache:gitlab:merge_request_diffs/12-20180201121855831702000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
2) "cache:gitlab:merge_request_diffs/47-20180313140710462936000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
3) "cache:gitlab:merge_request_diffs/39-20180201121912645797000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
4) "cache:gitlab:merge_request_diffs/48-20180313140745621010000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
5) "cache:gitlab:merge_request_diffs/10-20180201121854548624000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"

47 and 48 belonged to the same MR, and the others to a different MR each. When I pushed to the MR for 47 and 48, then re-ran the command, I got:

redis /Users/seanmcgivern/Code/gitlab-development-kit/redis/redis.socket> keys cache:gitlab:merge_request_diffs/*
1) "cache:gitlab:merge_request_diffs/12-20180201121855831702000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
2) "cache:gitlab:merge_request_diffs/39-20180201121912645797000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
3) "cache:gitlab:merge_request_diffs/49-20180314160247882384000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"
4) "cache:gitlab:merge_request_diffs/10-20180201121854548624000/highlighted-diff-files/expanded=false/ignore_whitespace_change=false/max_files=1000/max_lines=50000"

Are there points in the code the reviewer needs to double check?

Don't think so.

Why was this MR needed?

https://gitlab.com/gitlab-com/infrastructure/issues/3840 was an outage. This may have contributed.

Does this MR meet the acceptance criteria?

  • Changelog entry added, if necessary
  • Tests added for this feature/bug
  • Review
    • Has been reviewed by Backend

What are the relevant issue numbers?

Closes #44191 (closed).

Edited by Sean McGivern

Merge request reports