Enforce Elasticsearch client timeout to prevent 500s

Changzheng Liurequested to merge
fix/es-client-request-timeout-default-468168 into master

What does this MR do and why?

Fixes the 500 errors described in #468168 (closed), where Advanced Search requests can run unbounded past Elasticsearch's ?timeout=30s URL parameter (which is best-effort and not strictly enforced) and end up tripping Rack::Timeout at 60s, surfacing as a 500 to the user.

The root cause is that three timeout layers exist today, and only one is actually enforced:

Layer Value Enforced?
ES server-side ?timeout=30s URL param 30s (1s for count) Best-effort only
Faraday HTTP client request timeout elasticsearch_client_request_timeout setting, defaults to 0 (= no limit) Only when admin sets it
Rack::Timeout 60s Yes — produces the 500

This MR addresses all three sub-problems in one change:

  1. Default the Faraday request timeout to 30s. When elasticsearch_client_request_timeout is 0 (the default), use ELASTIC_REQUEST_TIMEOUT (30s) instead of leaving the HTTP call unbounded. This stops requests from ever reaching Rack's 60s limit.
  2. Route timeout exceptions to the existing timeout handler. The instrumentation interceptor previously treated every Faraday::Error (including Faraday::TimeoutError) and ES GatewayTimeout as a generic Gitlab::Search::Client::ConnectionError. Split out TIMEOUT_ERRORS and re-raise Elastic::TimeoutError, which EE::SearchController already rescues with render_timeout (returning a 408).
  3. Rescue ConnectionError / AuthorizationError in count. Other endpoints (aggregations, autocomplete, haml_search_results) already do this. count did not — so a transport error there would still 500. It now returns a 503 with { count: '0' } so users get a graceful fallback (matching the expectation in the issue).

Combined effect for the user: the long-running ES query path now produces a clean 408 ("timed out") or 503 ("unavailable") instead of a 500, and the timeout fires at the network layer (~30s) rather than via Rack at ~60s.

References

How to set up and validate locally

  1. With Elasticsearch enabled and indexed (gdk start, Gitlab::CurrentSettings.update!(elasticsearch_search: true, elasticsearch_indexing: true)):
  2. In the rails console, simulate a slow ES query to exercise each path: 
    # Path 1 (timeout) — should now render 408 instead of 500
allow_any_instance_of(SearchService).to receive(:search_results)
  .and_raise(Faraday::TimeoutError.new('execution expired'))

# Path 2 (connection error during count) — should now render 503 with count: 0 instead of 500
allow_any_instance_of(SearchService).to receive(:search_results)
  .and_raise(Gitlab::Search::Client::ConnectionError.new('boom'))
  3. Verify ApplicationSetting#elasticsearch_config[:client_request_timeout] returns 30 when elasticsearch_client_request_timeout is 0.
  4. Run targeted specs: 
    bundle exec rspec ee/spec/lib/gitlab/instrumentation/elasticsearch_transport_spec.rb \
                  ee/spec/controllers/ee/search_controller_spec.rb \
                  ee/spec/models/application_setting_spec.rb \
                  spec/controllers/search_controller_spec.rb

Behavior change for self-managed

Self-managed instances that left elasticsearch_client_request_timeout at 0 (the default) will start enforcing a 30s Faraday timeout. This is strictly an improvement over hanging until Rack kills the request, but operators with intentionally slow queries can still raise the value via the existing admin setting. No flag is added because the change converts an existing 500 path into a graceful 408/503 — the worst-case behavior is the same as today, just earlier and friendlier.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports