Zoekt: all-nodes-offline errors still surface as 500s in aggregations and blob search

Summary

MR !254462 (merged) added a rescue ::Search::Zoekt::Errors::ClientConnectionError in EE::SearchController#aggregations and in Search::Zoekt::SearchResults#known_languages, so a Zoekt node that is reachable but failing (connection refused, unparseable JSON) now returns a 503 instead of a 500. There is a second, uncovered failure mode: when there is no reachable node at all, a different error class is raised before any HTTP call is made, and it escapes both rescues. SearchController#aggregations still returns a 500 and still reports to Sentry as an application bug.

Why the current rescues do not cover it

  • Gitlab::Search::Zoekt::Client#load_balancer (ee/lib/gitlab/search/zoekt/client.rb:112-114) builds ::Search::Zoekt::LoadBalancer.new(::Search::Zoekt::Node.for_search.searchable).
  • Search::Zoekt::Node.searchable (ee/app/models/search/zoekt/node.rb:137-142) returns none when online is empty and ProcessHealth.active?.
  • Search::Zoekt::LoadBalancer#initialize (ee/lib/search/zoekt/load_balancer.rb:16) raises ArgumentError, 'no nodes available for zoekt load balancer' on an empty relation.
  • Gitlab::Search::Zoekt::Client#with_load_balanced_node (ee/lib/gitlab/search/zoekt/client.rb:66-81) has rescue StandardError => e that logs and re-raises unchanged. It also raises a bare RuntimeError (raise 'Node can not be found' unless node) when fetch_proxy_node returns nil.
  • The traversal-id path has the same shape: Search::Zoekt::NodeSelector raises ArgumentError, "No online nodes found for replica ... in namespace: ..." (ee/lib/search/zoekt/node_selector.rb:99-100), plus ArgumentError for no enabled namespace (:89) and no ready replica (:94).

None of these are ClientConnectionError, so they pass through both rescues added in that MR unchanged.

Reproduction

The MR's local repro (gdk stop gitlab-zoekt-webserver-development-1 gitlab-zoekt-webserver-development-2) leaves the indexer heartbeating, so last_seen_at stays fresh and only webserver_last_seen_at goes stale. ProcessHealth.unhealthy? then flags every node, Node.searchable hits its cascade-failure fallback and returns online anyway (ee/app/models/search/zoekt/node.rb:144-148), the HTTP request goes out, and the result is ECONNREFUSED -> ClientConnectionError -- the case this MR fixes.

To reach the uncovered case, no node record can be in online at all: every node's last_seen_at (not just webserver_last_seen_at) older than the 30-second ONLINE_DURATION_THRESHOLD (ee/app/models/search/zoekt/node.rb:12, :66), or the node records absent entirely (host down, record aged out, network partition).

Affected surfaces

SearchController#aggregations and the language tally are only reachable this way with the zoekt_language_aggregations feature flag enabled. The same gap reaches the GraphQL blobSearch resolver and SearchController#count through the same with_load_balanced_node call, and the plain blob-results path reaches the client regardless of the feature flag.

Blast radius

The whole affected chain is search-only, so a fix does not touch indexing:

  • Gitlab::Search::Zoekt::Client#with_load_balanced_node has exactly one caller, Client#search (ee/lib/gitlab/search/zoekt/client.rb:54).
  • Search::Zoekt::LoadBalancer.new has two call sites: Client#load_balancer (ee/lib/gitlab/search/zoekt/client.rb:113) and Search::Zoekt::ReplicaSelector (ee/lib/search/zoekt/replica_selector.rb:41).
  • Search::Zoekt::NodeSelector is used only by Search::Zoekt::SearchRequest#nodes (ee/lib/search/zoekt/search_request.rb:98, :103, :105).
  • ReplicaSelector is used only by NodeSelector#fetch_nodes (ee/lib/search/zoekt/node_selector.rb:68).

Proposed fix

Estimated size: about 6 files, 40 to 70 lines including specs.

  1. Add NodeUnavailableError = Class.new(BaseError) to ee/lib/search/zoekt/errors.rb, which currently defines only BaseError and ClientConnectionError.
  2. In Client#load_balancer (ee/lib/gitlab/search/zoekt/client.rb:112-114), raise NodeUnavailableError when ::Search::Zoekt::Node.for_search.searchable is empty, rather than letting LoadBalancer#initialize raise ArgumentError. This leaves LoadBalancer's own ArgumentError intact for its other caller, ReplicaSelector.
  3. Replace the bare raise 'Node can not be found' in with_load_balanced_node (ee/lib/gitlab/search/zoekt/client.rb:69) with NodeUnavailableError.
  4. In NodeSelector#validate_result!, convert only the No online nodes found for replica raise (ee/lib/search/zoekt/node_selector.rb:99-100).
  5. Widen the two rescues added by MR 254462 from ::Search::Zoekt::Errors::ClientConnectionError to ::Search::Zoekt::Errors::BaseError, so both node-unavailability and connection failures return a 503.
  6. Specs: update the assertion at ee/spec/lib/search/zoekt/node_selector_spec.rb:274, and add coverage in ee/spec/controllers/ee/search_controller_spec.rb and ee/spec/lib/search/zoekt/search_results_spec.rb for the new error class.

NodeSelector has three ArgumentError raise sites and they do not mean the same thing. This is the main reason this is a separate issue rather than an addition to MR 254462:

Raise site Meaning Disposition
ee/lib/search/zoekt/node_selector.rb:99 "No online nodes found for replica" — the backend is genuinely unavailable Convert, return 503
ee/lib/search/zoekt/node_selector.rb:94 "No ready replica found" — the namespace is not indexed yet Leave as is, stays a 500
ee/lib/search/zoekt/node_selector.rb:89 "No enabled namespace found" — Zoekt is not enabled for this namespace and the caller should have checked before searching Leave as is, stays a 500

Converting the latter two would turn a misconfiguration into a silent 503, which is the same failure mode that caused !251074 (closed) to be closed. Widening either rescue to StandardError is not an option, for the same reason: it would swallow genuine application bugs into a 503, as was rejected on the Elasticsearch side.

References