Handle deleted rollover index exclusions

[Pybsama]

Why

ElasticGraph caches the concrete indices related to a rollover template. If one
of those indices is then deleted manually, a later range-filtered search can
still include the deleted name as an exclusion in its _msearch index
expression. OpenSearch rejects that request with index_not_found_exception.

What

• Set ignore_unavailable: true only on _msearch metadata headers whose
  index expression contains exclusions. Unlike allow_no_indices, this option
  ignores a missing concrete target such as the deleted index in an exclusion
  expression, while ordinary positive-index queries retain their existing
  failure behavior.
• Add a real-datastore regression that caches two monthly rollover indices,
  deletes one, and verifies a range-filtered search still returns the document
  from the remaining index.
• Add a unit assertion covering both the exclusion header and an adjacent
  ordinary header.

Validation

• Targeted unit specs: 27 examples, 0 failures.
• GraphQL unit suite: 1,352 examples, 0 failures.
• New real-datastore regression on OpenSearch 2.19.0 (with the repository's
  mapper-size and analysis-icu plugins): 1 example, 0 failures.
• script/lint on all three changed files.
• script/type_check.
• script/spellcheck.
• bundle exec rake schema_artifacts:check.
• Direct OpenSearch 2.19.0 _msearch verification confirmed the original
  wildcard,-deleted-index expression returns a 404 sub-response with
  allow_no_indices: true and a 200 sub-response with
  ignore_unavailable: true.
• The new :no_vcr datastore regression is also included in the CI matrix for
  Elasticsearch 9.0/9.4 and OpenSearch 2.19/3.6.

Risk assessment

Low. The change adds an officially supported _msearch metadata option only
when the generated index expression contains exclusions. Ordinary positive
index queries keep their current strict behavior. Index expressions, routing,
request bodies, query grouping, and public APIs are unchanged. The integration
regression preserves a remaining live index, so it also verifies that successful
results are not swallowed.

References

Closes #589.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[myronmarston]

🤖 Diagnosis is right, and ignore_unavailable over allow_no_indices is the correct call — non-obvious given #589 recommends the other flag. I re-derived it independently and agree.

Two problems though: the excluding_indices? condition leaves a real hole, and closing it surfaces a second issue in the search router.

What I tested

Three scenarios, each against three versions of the code, on OpenSearch:

• reverted = this PR's datastore_query.rb change undone (i.e. main today)
• this PR = ignore_unavailable set only when the index expression has exclusions
• unconditional = ignore_unavailable: true always set, no condition

#	Scenario	reverted	this PR	unconditional
A	Exclusion of a deleted index (the #589 shape)	index_not_found	PASS	PASS
B	Empty time set + aggregation (no exclusions)	index_not_found	index_not_found	ExecutionError: not configured
C	Every cached index deleted (wildcard matches nothing)	index_not_found	PASS	PASS

Row B is the hole — see inline comment. Row B's "unconditional" column is why removing the condition isn't sufficient by itself.

Recommended fix

Step 1 — always set ignore_unavailable (inline comment has the suggestion).

Step 2 — datastore_search_router.rb needs a matching change. This isn't in the diff so I can't comment inline; it's line 101.

When ignore_unavailable successfully ignores a deleted index, the response has _shards.total: 0. That guard reads 0 shards as "indices were never configured" unless query.excluding_indices?. In scenario B there are no exclusions, so the error just changes from index_not_found_exception to the misleading "The datastore indices have not been configured".

A stale cache and a never-configured cluster are different conditions, and exclusions aren't what separates them. Concretely, replace:

if shards_total == 0 && !query.excluding_indices?
  raise ::GraphQL::ExecutionError, INDICES_NOT_CONFIGURED_MESSAGE
end

with:

if shards_total == 0 && !query.searches_only_deleted_indices?
  raise ::GraphQL::ExecutionError, INDICES_NOT_CONFIGURED_MESSAGE
end

and in datastore_query.rb, replace excluding_indices? with:

# Indicates if this query targets indices that our cached index state says exist. When true, a response
# reporting zero searched shards means those indices have been deleted since we cached them--not that the
# cluster was never configured.
def searches_only_deleted_indices?
  !search_index_expression.empty? && narrowed_search_index_definitions.any? do |index_def|
    index_def.rollover_index_template? && !index_def.known_related_query_rollover_indices.empty?
  end
end

narrowed_ rather than initial_ is deliberate: the predicate has to reason about the same definitions the index expression was built from, and search_index_expression uses the narrowed set. With a __typename filter the two differ, and consulting a definition we aren't actually searching gives the wrong answer. No extra cost either — it's memoized, and the first clause here already forces it.

excluding_indices? has only one caller (that guard), so it can be replaced outright rather than added alongside.

I verified this across five cases: scenario B passes; a genuinely unconfigured rollover index still raises ExecutionError; a genuinely unconfigured non-rollover index still raises (my first attempt at this predicate got that wrong, so it's worth keeping a test on it); and normal rollover and non-rollover searches are unaffected. The existing test at misc_spec.rb:186 stays green — worth noting since simply deleting the guard would silently break it.

Elasticsearch vs OpenSearch (informational)

This bug only exists on OpenSearch. The two engines disagree on the expression at the heart of this PR:

my_index_rollover__*,-my_index_rollover__2017-02 (2017-02 deleted)	Result
Elasticsearch 9.4.2	200 OK — excluding a missing index is silently fine
OpenSearch 2.19.0 and 3.6.0	index_not_found_exception

So on Elasticsearch the new integration test passes whether or not the bug is fixed; only the ignore_unavailable header assertion can fail there. That's fine and I wouldn't change the approach — running on both backends is what we want, and sending a flag one engine doesn't need costs nothing. Just be aware the header assertion is carrying the whole regression signal on Elasticsearch, so it shouldn't be dropped later as redundant.

On allow_no_indices — agreed, not needed

Adding it would be a no-op. On OpenSearch 2.19, for the #589 expression:

Option	Result
allow_no_indices=true (what #589 suggested)	still errors
ignore_unavailable=true (what you used)	OK

The flags guard different things and only one is ever violated: a wildcard matching zero indices is already allowed (allow_no_indices defaults to true), whereas excluding a concrete name that no longer exists is the actual failure. I also tested the case where the only index is deleted, so the wildcard matches nothing and a missing name is excluded (scenario C) — ignore_unavailable alone still fixes it, allow_no_indices alone still fails.

[myronmarston]

🤖 Setting this conditionally leaves a hole. IndexExpressionBuilder has a path (line 87) that emits a concrete index name with no exclusions: when a filter's time set is empty but an aggregation requires an index, it picks the first cached index by name and drops the wildcard. If that index was deleted, OpenSearch raises the same index_not_found_exception this PR fixes, but excluding_indices? is false so the flag never gets set.

Always setting it is simpler and closes this:

Suggested change

	ignore_unavailable: excluding_indices? || nil,
	def to_datastore_msearch_header
	@to_datastore_msearch_header ||= {
	index: search_index_expression,
	ignore_unavailable: true,
	routing: shard_routing_values&.join(",")
	}.compact
	end

Note this needs the datastore_search_router.rb change from my review comment too — on its own it converts scenario B's failure into a different one.

[myronmarston]

🤖 Is :no_vcr needed here? I removed it from this test and the two I'm suggesting below, and all three still fail with the real index_not_found_exception against main and pass under the fix — recording and replaying cassettes both clean, no UnhandledHTTPRequestError.

The tag is clearly warranted for the deadline test at the top of this file, which documents why (real slow query + client-side timeout). Nothing similar applies here — these just need real index create/delete round-trips, which VCR handles. Since VCR is "just to speed things up (as a smart cache)", opting out costs time for no gain.

Happy to be corrected if you hit flakiness that motivated it.

Suggested change

	it "continues searching after a cached rollover index has been manually deleted", :expect_index_exclusions, :no_vcr do
	it "continues searching after a cached rollover index has been manually deleted", :expect_index_exclusions do

[myronmarston]

🤖 Two more cases worth covering — both fail with index_not_found_exception on OpenSearch against main, and neither is caught by the current test. Suggestion adds them after the existing example:

Suggested change

	end
	it "continues aggregating after the only cached rollover index a filter can match has been manually deleted" do
	build_admin(datastore_core: graphql.datastore_core).cluster_configurator.configure_cluster(StringIO.new)
	index_into(graphql, january_widget = build(:widget, created_at: "2024-01-15T12:00:00Z"))
	pre_cache_index_state(graphql)
	index_def = graphql.datastore_core.index_definitions_by_name.fetch(unique_index_name)
	january_index_name = index_def.index_name_for_writes({"created_at" => january_widget.fetch(:created_at)})
	main_datastore_client.delete_indices(january_index_name)
	# This filter is impossible to satisfy, so the time set is empty. The aggregation requires an index to
	# search, so `IndexExpressionBuilder` names the first cached index directly, with no wildcard and no
	# exclusions. That index has been deleted, so the datastore has nothing to resolve the name against.
	results = search_datastore(
	index_def_name: unique_index_name,
	graphql: graphql,
	aggregations: [aggregation_query_of(name: "counts", groupings: [date_histogram_grouping_of("created_at", "month")])],
	client_filters: [{"created_at" => {"gte" => "2025-02-01T00:00:00Z", "lt" => "2025-01-01T00:00:00Z"}}]
	)
	expect(performed_search_metadata("main").last).to include("index" => january_index_name)
	expect(results.aggregations).to eq({})
	end
	it "continues searching after every cached rollover index has been manually deleted", :expect_index_exclusions do
	build_admin(datastore_core: graphql.datastore_core).cluster_configurator.configure_cluster(StringIO.new)
	index_into(graphql, january_widget = build(:widget, created_at: "2024-01-15T12:00:00Z"))
	pre_cache_index_state(graphql)
	index_def = graphql.datastore_core.index_definitions_by_name.fetch(unique_index_name)
	january_index_name = index_def.index_name_for_writes({"created_at" => january_widget.fetch(:created_at)})
	main_datastore_client.delete_indices(*index_def.known_related_query_rollover_indices.map(&:name))
	# With every index gone, the rollover wildcard resolves to nothing at all. That alone is fine (the
	# datastore allows a wildcard to match no indices by default), but the expression also excludes the
	# now-missing January index by name, and resolving that name is what fails.
	results = search_datastore(
	index_def_name: unique_index_name,
	graphql: graphql,
	client_filters: [{"created_at" => {"gte" => "2024-02-01T00:00:00Z"}}]
	)
	expect(results.to_a).to eq []
	expect_to_have_excluded_indices("main", [january_index_name])
	end

Two gotchas if you adjust the first one: a needs_doc_count: true aggregation isn't enough (empty aggregations body, so require_indices stays false and the expression collapses to "") — it needs a real grouping. And the filter has to be genuinely unsatisfiable to make the time set empty; a merely non-matching range still takes the normal wildcard path.

Also worth adding a case for a genuinely unconfigured non-rollover index, to lock in the router predicate from my review comment.

[myronmarston]

Suggested change

	it "raises a `GraphQL::ExecutionError` indicating they need to be configured" do
	it "raises a `GraphQL::ExecutionError` indicating they need to be configured if they have not yet been" do

(Previously, the context was "when indices have not yet been configured" but you changed that.).