Cap search pagination at the Elasticsearch result window

What does this MR do and why?

Searching merge requests past the Elasticsearch result window returns 400 Bad Request from the indexer, which surfaces to the user as a 500:

[400] {"error":{"root_cause":[{"type":"illegal_argument_exception",
"reason":"Result window is too large, from + size must be less than or equal to: [10000] but was [10100]."}]}}

Root cause

The issue description attributes this to Search::Elastic::MergeRequestQueryBuilder not registering the :page format. I do not think that is where the offset comes from, so I want to lay out the trace.

MergeRequestQueryBuilder receives its options from scope_results:

def scope_results(scope, klass, count_only:)
  options = scope_options(scope).merge(count_only: count_only)
  ...
end

Neither base_options nor merge_request_scope_options contains page or per_page, and Formats.page returns early without both:

def page(query_hash:, options:)
  return query_hash unless options[:page] && options[:per_page]
  ...
end

So registering :page on that builder would not change the generated query.

The offset is applied later, by Kaminari in eager_load:

def eager_load(es_result, page, per_page, preload_method, eager)
  paginated_base = es_result.page(page).per(per_page)

SearchService#page is [1, params[:page].to_i].max, which has no upper bound, so from grows without limit until Elasticsearch rejects the search.

For reference, this is page in the elasticsearch-model version we bundle (7.2.1), at lib/elasticsearch/model/response/pagination/kaminari.rb:

def page(num=nil)
  @results  = nil
  @records  = nil
  @response = nil
  @page     = [num.to_i, 1].max
  @per_page ||= __default_per_page

  self.search.definition.update size: @per_page,
                                from: @per_page * (@page - 1)
  self
end

and limit, which per resolves to, recomputes from against the final per_page:

def limit(value)
  return self if value.to_i <= 0
  ...
  @per_page = value.to_i

  search.definition.update :size => @per_page
  search.definition.update :from => @per_page * (@page - 1) if @page
  self
end

Setting @response = nil means the search is re-executed rather than sliced in memory, so from and size reach Elasticsearch. After es_result.page(page).per(per_page) the definition therefore holds from + size == per_page * page, which is exactly the quantity max_result_window rejects.

This splits the scopes into two groups:

Path	Scopes	Where pagination is applied
execute_search with page/per_page in options	issues, work items, epics	in the query hash, via Formats.page
scope_results then eager_load	merge requests, milestones, users, notes, projects	Kaminari, on the response

Fix

Cap the page in eager_load so from + size stays within ELASTIC_COUNT_LIMIT, the constant this class already uses for the same Elasticsearch limit when formatting counts as 10,000+.

Since from + size is per_page * page, the cap is ELASTIC_COUNT_LIMIT / per_page: page 101 at 100 per page becomes page 100, giving exactly 10,000.

This fixes every scope that paginates through eager_load, not only merge requests.

Resolves #597243 (closed)

Changelog: fixed

References

Related Issue: #597243 (closed)

Discussion points

1. Capping compared with erroring. This clamps silently, so a request for page 600 returns page 500. That matches how the class already caps counts at 10,000+, and it turns a 500 into a usable page. If the group would rather show an explicit "results are limited to the first 10,000" message, that is a small change on top and I am happy to make it.

2. The other path is not fixed here. Scopes going through Gitlab::Search::Client.execute_search compute from in Formats.page with no bound either, so they should be reachable the same way at a deep enough page. I have kept this MR to the reported scope, since the issue is backport::required. Worth a follow-up issue if that is confirmed.

Screenshots or screen recordings

Before	After
Searching merge requests past page 100 (at 100 per page) returns a 500.	The page is capped at the result window and results render.

How to set up and validate locally

bundle exec rspec ee/spec/lib/gitlab/elastic/search_results_spec.rb -e "deep pagination"

The added table-driven spec asserts the page passed to Kaminari for a range of per_page values, and that page * per_page never exceeds ELASTIC_COUNT_LIMIT.

Note

I have not been able to run the suite locally: this checkout cannot complete bundle install because the lockfile pins Bundler 4.0.18 and the self-restart resolves Ruby 3.2.9, which gitlab-labkit rejects. Both changed files pass ruby -c, and the capping arithmetic was verified in isolation, but I am relying on CI for the spec run. Flagging that explicitly rather than implying a green local run.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

• I have evaluated the MR acceptance checklist for this MR.