Draft: Cache the total count on the group merge requests API

Marc Shawrequested to merge
fix/cache-group-mr-api-total-count into master

Behind a new gitlab_com_derisk flag cache_total_count_on_group_merge_requests_api (default off, group actor), GET /groups/:id/merge_requests caches the count behind X-Total for 15 minutes instead of running it on every page.

  • All pagination headers stay. A client walking every page pays for a count on the first and last page only.
  • Alternative to !257173, which removes the headers and was ruled a breaking change.
Detailed context for AI agents

Problem

  • GET /groups/:id/merge_requests runs an exact COUNT (Kaminari total_count_with_limit, capped at LIMIT 10001) on every request to fill X-Total and X-Total-Pages.
  • The count strips offset, limit and order, so every page of the same query runs the identical count.
  • On gitlab-org it took 1240ms of 2400ms DB time in one request.
  • Heavy callers walk every page: 5,400 open MRs at per_page=100 is 54 identical counts per walk.

Why not the earlier approach

  • !257173 skips the count and removes X-Total and X-Total-Pages.
  • API owners said removing them by default is a breaking change that needs VP approval (!257173 (comment 3961105985) and !257173 (comment 3983043278)).
  • This MR keeps the headers and caches the count instead.
  • The earlier cache version of that MR used a preset_total_count patch to Kaminari. This one needs no Kaminari patch.

What changes

  • Flag: cache_total_count_on_group_merge_requests_api, type gitlab_com_derisk, default off, actor is the requested group.
  • With the flag on, these headers are all still returned: X-Total, X-Total-Pages, X-Page, X-Per-Page, X-Next-Page, X-Prev-Page and the Link header (first/prev/next/last).
  • Project and global (/merge_requests) endpoints are unchanged. With the flag off the group endpoint is unchanged.

Implementation

  • lib/gitlab/pagination/offset_pagination.rb: new opt-in keyword cache_total_count: on paginate. When true (and without_count is false and the input is an ActiveRecord::Relation), it reads the count from Rails.cache, or runs total_count_with_limit and writes it with a 15 minute TTL (constant TOTAL_COUNT_CACHE_TTL).
  • Cache key: SHA256 of relation.except(:offset, :limit, :order).to_sql. The SQL already contains the user id (the project_authorizations subquery), the effect of admin mode, and every filter, so the same SQL means the same rows. Different users, filters, states or groups get different keys. Sort order does not change the key, which is correct because it does not change the count.
  • Headers are built directly with Gitlab::Pagination::OffsetHeaderBuilder from the cached total. next_page and prev_page are computed the same way Kaminari does (next only when current page < total pages, prev only when current page > 1 and not out of range). This avoids Kaminari's next_page and prev_page, which would run the count again.
  • Last-page recount: if the request is on or past the last page according to the cached count, it recounts and rewrites the cache. A stale count that is too low can never drop the next link before the real last page, so clients never miss MRs.
  • Staleness: before the last page, X-Total and X-Total-Pages can be up to 15 minutes old. If many MRs were closed or merged during the TTL (more than per_page), a walking client can get some empty pages at the end before it reaches the cached last page, which recounts and ends the walk.
  • The recount only protects clients that follow the next link. A client that reads X-Total-Pages from page 1 and fetches the remaining pages in parallel can miss trailing pages, or fetch an extra empty one, for up to 15 minutes.
  • Results that fit on one page are always on the last page, so they recount every time. Those counts are cheap.
  • Over 10,000 results: same as today. The totals are omitted and the next page comes from Kaminari's without_count lookahead.
  • Why not use the without_count lookahead for the next link in the normal case: find_merge_requests chains .preload(...) and .with_api_entity_associations after paginate, which creates a fresh unloaded relation. The lookahead's loaded page would be thrown away and the page query would run twice.
  • The existing pagination branch in paginate_with_limit_optimization was only refactored to share a small apply_pagination helper. Behaviour with the flag off is unchanged.
  • lib/api/merge_requests.rb: find_merge_requests(cache_total_count: false, **args) passes the option to paginate. The group route passes Feature.enabled?(:cache_total_count_on_group_merge_requests_api, user_group). The signature uses **args because a (args = {}, keyword: ...) signature makes find_merge_requests(project_id: ...) raise ArgumentError on Ruby 3.
  • Precedent for serving a cached group MR count: Groups::MergeRequestsCountService already serves the group open MR count (sidebar) from a 24 hour cache once it is over 1,000.

Open question for API owners

  • Does an X-Total that can be up to 15 minutes old on non-final pages count as "no change" for the API contract?

Rollout

  • Rollout issue not created yet (rollout_issue_url in the flag YAML is empty).

Not in scope

  • An opt-in without_count=true request parameter (approved in !257173 (comment 3961105985)). This is a separate change.
  • Per merge request mergeability check costs inside detailed_merge_status.

Verification done

  • spec/lib/gitlab/pagination/offset_pagination_spec.rb: 47 examples, 0 failures. New examples check:
    • cached and uncached requests produce identical headers on pages 1, 2 and 3 (out of range), with and without a warm cache
    • one COUNT, then reuse
    • the key changes with the query
    • the count expires after the TTL
    • a stale low count is served before the last page and recounted on the last page, so the next link is kept
    • over MAX_COUNT_LIMIT the totals are omitted
    • without_count still skips the count
    • with the cache branch disabled, the two cache-specific examples fail
  • spec/requests/api/merge_requests_spec.rb: new group endpoint context. The second request runs no merge_requests COUNT and returns the same X-Total, X-Total-Pages and X-Next-Page. A member and a non-member of a private project in the group get different X-Total values, so the count is not shared between users. With the flag disabled every request counts. Group endpoint block: 137 examples, 0 failures. Global and project list blocks: 162 examples, 0 failures.
  • RuboCop clean on changed files.

How to set up and validate locally

  1. In a Rails console on GDK, enable the flag for a group: Feature.enable(:cache_total_count_on_group_merge_requests_api, Group.find_by_full_path('<group>')). Make sure the group has a few merge requests.
  2. Call the endpoint twice and compare X-Total: curl -sI --header "PRIVATE-TOKEN: <token>" "http://gdk.test:3000/api/v4/groups/<group_id>/merge_requests?per_page=1".
  3. Run tail -f log/development.log while doing this. You should see the SELECT COUNT(*) FROM (SELECT 1 AS one FROM "merge_requests" ... query only on the first request (and again on the last page).
  4. Walk the pages with page=1, page=2 and so on up to the last page. Check that X-Next-Page and the Link header are correct on every page and that X-Next-Page is empty on the last page.
  5. Disable the flag with Feature.disable(:cache_total_count_on_group_merge_requests_api, Group.find_by_full_path('<group>')) and repeat. Every request should now run the count.

References

Merge request reports