Stage 7 — Search and Aggregation Issues

What this stage teaches

Stage 7 is the read path: turning a query into Lucene work on each shard, then merging per-shard results on the coordinating node. The skill is reasoning about a fan-out / reduce pipeline where the hard bugs live in the merge — especially aggregation reduce, which must produce the same answer whether a shard returned data, returned nothing, or did not exist at all.

  • QueryPhase / FetchPhase — per-shard execution. QueryPhase runs the query and collects top docs and aggregation buckets; FetchPhase hydrates the matched docs. SearchService.executeQueryPhase / executeFetchPhase are the entry points.
  • SearchPhaseController — the coordinating-node reduce: merges top docs, calls InternalAggregation.reduce(...), computes the final response.
  • Aggregations — AggregatorFactory → Aggregator (per shard) → InternalAggregation (the serializable result) → reduce(...) (the cross-shard merge). The merge is where empty-shard and partial-result bugs hide.
  • QueryBuilder / AbstractQueryBuilder — the DSL → Lucene Query translation via QueryShardContext, including query rewrite (rewrite(...)), where another family of bugs lives.

The two classic Stage 7 bugs: an aggregation reduce edge case (wrong result when some shards are empty or absent, or when partial reduce runs) and a query rewrite bug (a builder that rewrites to the wrong query, or is not equal/serializable round-trip).

Prerequisite: Stage 4 plus the search deep dives: Search execution, Aggregations, and Query DSL & QueryBuilders.

Fan-out and reduce

flowchart TD
  C[TransportSearchAction<br/>coordinating node] --> F1[shard 1: QueryPhase]
  C --> F2[shard 2: QueryPhase]
  C --> F3[shard N: QueryPhase]
  F1 --> RED[SearchPhaseController reduce<br/>InternalAggregation.reduce]
  F2 --> RED
  F3 --> RED
  RED --> FET[FetchPhase on matched docs]
  FET --> RESP[final SearchResponse]

The invariant that breaks most often:

reduce must be associative and handle the empty/absent case. It runs in batches (partial reduce of M shard results at a time, then a final reduce of the partials), so reduce(reduce(a, b), c) must equal reduce(a, reduce(b, c)), and reduce of an empty or all-empty input must produce a well-formed (often empty) aggregation, not a null, not a throw, not a wrong default.

A shard can contribute no documents (empty index), and with index filtering a shard may not be queried at all. Both must reduce cleanly.

Finding Stage 7 issues

is:issue is:open label:bug no:assignee label:"Search:Aggregations"
is:issue is:open label:bug no:assignee label:"Search:Query Capabilities"
is:issue is:open label:bug no:assignee "aggregation" "empty" in:body
is:issue is:open label:bug no:assignee "rewrite" "query" in:body

Area labels include Search:Aggregations, Search:Query Capabilities, Search:Relevance, Search:Performance. Check the current set.

Fallback grep — find a reduce that does not guard the empty case, or a builder whose doRewrite is suspect:

grep -rn "public InternalAggregation reduce" server/src/main/java/org/opensearch/search/aggregations/ | head
grep -rln "doRewrite\|protected QueryBuilder doRewrite" server/src/main/java/org/opensearch/index/query/

Walked example — an aggregation reduce edge case

Illustrative of the pattern. The grep finds the real aggregation; treat the class as a stand-in for "some aggregation whose reduce mishandles empty shards."

Symptom: an issue reports that an aggregation returns a wrong value (or NaN, or a null) when at least one shard has no matching documents — the empty shard's InternalAggregation is folded into the reduce incorrectly. A common shape: a metric agg that averages by summing values and dividing by count, where an empty shard contributes count == 0 and the reduce divides by a total that ignored it, or includes a sentinel that poisons the sum.

Locate the reduce

ls server/src/main/java/org/opensearch/search/aggregations/metrics/
grep -n "reduce\|getProperty\|buildEmptyAggregation\|InternalAggregation" \
  server/src/main/java/org/opensearch/search/aggregations/metrics/InternalAvg.java | head
git log --oneline -n 5 -- server/src/main/java/org/opensearch/search/aggregations/metrics/InternalAvg.java

The schematic of a metric reduce:

@Override
public InternalAvg reduce(List<InternalAggregation> aggregations, ReduceContext reduceContext) {
    CompensatedSum sum = new CompensatedSum(0, 0);
    long count = 0;
    for (InternalAggregation aggregation : aggregations) {
        InternalAvg avg = (InternalAvg) aggregation;
        count += avg.count;
        sum.add(avg.sum);     // empty shard contributes count=0, sum=0 — must be a no-op
    }
    return new InternalAvg(getName(), sum.value(), count, format, getMetadata());
}

If an empty shard contributes a non-zero sentinel sum, or if buildEmptyAggregation() returns the wrong neutral element, the average skews. Read both reduce and buildEmptyAggregation() — they must agree on the identity element.

Diff

--- a/server/src/main/java/org/opensearch/search/aggregations/metrics/InternalAvg.java
+++ b/server/src/main/java/org/opensearch/search/aggregations/metrics/InternalAvg.java
@@  public InternalAvg reduce(...) {
         for (InternalAggregation aggregation : aggregations) {
             InternalAvg avg = (InternalAvg) aggregation;
+            // An empty shard has count == 0; its sum must be the additive identity (0),
+            // never a sentinel like Double.NaN, or it poisons the merged average.
             count += avg.count;
             sum.add(avg.sum);
         }
-        return new InternalAvg(getName(), sum.value(), count, format, getMetadata());
+        // With zero documents across all shards, the average is defined as NaN by contract;
+        // make that explicit rather than dividing 0/0 implicitly.
+        return new InternalAvg(getName(), count == 0 ? Double.NaN : sum.value(), count, format, getMetadata());

The reasoning that generalises beyond this one aggregation:

  1. Find the identity element. For sum it is 0; for min it is +inf; for max it is -inf; for a sketch it is the empty sketch. buildEmptyAggregation() must return it and reduce must treat it as a no-op.
  2. Define the all-empty result by contract, not by accident. avg of nothing is NaN, sum of nothing is 0, min/max of nothing is null/absent. Make it explicit.
  3. Associativity. Because reduce runs in partial batches, the per-shard order must not change the result. Test that.

Test with AggregatorTestCase

AggregatorTestCase runs an aggregation over an in-memory Lucene index, including the multi-shard reduce, with no cluster:

public void testAvgWithEmptyShardIsCorrect() throws IOException {
    // One "shard" with values, one with no documents.
    testCase(new MatchAllDocsQuery(), iw -> {
        iw.addDocument(singleton(new NumericDocValuesField("value", 10)));
        iw.addDocument(singleton(new NumericDocValuesField("value", 20)));
        // ... and a segment/leaf with no "value" docs ...
    }, agg -> {
        InternalAvg avg = (InternalAvg) agg;
        assertEquals(15.0, avg.getValue(), 0.0);   // empty shard did not skew it
    }, new NumberFieldMapper.NumberFieldType("value", NumberFieldMapper.NumberType.LONG),
       avgBuilder("value"));
}

public void testAvgOfNothingIsNaN() throws IOException {
    testCase(new MatchAllDocsQuery(), iw -> { /* add no value docs */ }, agg -> {
        assertTrue(Double.isNaN(((InternalAvg) agg).getValue()));
    }, /* field */, avgBuilder("value"));
}

For the reduce specifically, InternalAggregationTestCase round-trips and reduces InternalAggregation instances directly — use it to assert associativity across an empty element:

grep -rln "extends AggregatorTestCase\|extends InternalAggregationTestCase" \
  server/src/test/java/org/opensearch/search/aggregations/
./gradlew :server:test --tests "*InternalAvgTests" --tests "*AvgAggregatorTests" -q

Confirm end to end

./gradlew run &
curl -s -XPUT 'localhost:9200/idx' -H 'content-type: application/json' -d '{"settings":{"number_of_shards":3}}'
curl -s -XPOST 'localhost:9200/idx/_doc?refresh' -H 'content-type: application/json' -d '{"value":10}'
curl -s -XPOST 'localhost:9200/idx/_doc?refresh' -H 'content-type: application/json' -d '{"value":20}'
curl -s 'localhost:9200/idx/_search' -H 'content-type: application/json' -d '{
  "size":0, "aggs":{"a":{"avg":{"field":"value"}}}
}' | jq '.aggregations.a.value'

With 3 shards and 2 docs, at least one shard is empty — the average must read 15, proving the empty-shard reduce.

A second pattern — a query rewrite bug

Illustrative of the pattern.

QueryBuilders rewrite themselves before execution (e.g. a range over a constant field rewrites to match_none or match_all). A rewrite bug returns the wrong simplified query, or breaks the equals/hashCode/serialization round-trip that the framework requires.

grep -n "doRewrite\|Rewriteable\|protected Query doToQuery" \
  server/src/main/java/org/opensearch/index/query/RangeQueryBuilder.java | head

These are tested with AbstractQueryTestCase, which auto-checks serialization round-trip, equals/hashCode, XContent parse, and toQuery:

// Extend AbstractQueryTestCase<RangeQueryBuilder>; override doAssertLuceneQuery
// to assert the rewritten/translated Lucene Query is what you expect.
protected void doAssertLuceneQuery(RangeQueryBuilder qb, Query query, QueryShardContext context) {
    assertThat(query, instanceOf(/* expected Lucene query class */));
}

grep -rln "extends AbstractQueryTestCase" server/src/test/java/org/opensearch/index/query/
./gradlew :server:test --tests "*RangeQueryBuilderTests" -q

AbstractQueryTestCase is strict: if your change breaks Writeable round-trip or equals, the base class fails the test for you — which is exactly the safety you want.

Pitfalls

  • reduce that throws/returns null on empty input. It runs on empty and all-empty inputs. The identity element and the all-empty contract must be explicit.
  • Disagreement between reduce and buildEmptyAggregation. They must share the same identity element. Fix both in one PR.
  • Ignoring partial reduce. Large searches reduce in batches; a non-associative reduce gives different answers under different batched_reduce_size. Test associativity.
  • Breaking the query round-trip. Any new field on a QueryBuilder must be added to equals, hashCode, doWriteTo/constructor-StreamInput, and doXContent. AbstractQueryTestCase enforces this — do not suppress its checks.
  • Forgetting BWC on a serialized field. A new field on an InternalAggregation or QueryBuilder that crosses the wire needs a Version guard (Stage 11).
  • Hand-rolling Lucene fixtures. Use AggregatorTestCase/AbstractQueryTestCase; they build the leaf readers, field types, and shard context correctly.
  • Confusing query phase and fetch phase. Scoring/aggregation bugs are QueryPhase; source/highlight/fields bugs are FetchPhase. Diagnose which phase before you edit.

Exit criteria — when you're ready for Stage 8

  • One search or aggregation fix is merged with an AggregatorTestCase / InternalAggregationTestCase / AbstractQueryTestCase test that exercises the edge case (empty shard, all-empty, rewrite boundary).
  • You can name the identity element for the aggregation you fixed and you reconciled reduce with buildEmptyAggregation.
  • You can explain why aggregation reduce must be associative and how partial reduce makes that observable.
  • For a QueryBuilder change, you let AbstractQueryTestCase prove the round-trip rather than disabling its checks.

You have now worked both the write and read paths of core. Stage 8 steps to the boundary where core meets the plugins that build on it.