Step 4: Root Cause Identification
A symptom is "the test fails." A root cause is "this specific line, in this specific state, when this specific event arrives, performs this specific incorrect operation, because of this specific design assumption that no longer holds." If your statement does not have that shape, you have not found root cause yet.
This step is mostly thinking. The tools are five-whys, git log -S/-G,
git blame, and git bisect. The output is a 200–500 word root-cause document
and a tested hypothesis. Unlike a JIRA-based project, your archaeology ends not
at a JIRA comment thread but at a GitHub Pull Request discussion — read it
the same way.
Five Whys, Applied to an OpenSearch Bug
The five-whys technique sounds trite. It is not. The discipline of asking "why" five times in a row forces you past the first plausible explanation (almost always wrong) and into the actual design defect (almost always two or three levels deeper than you initially thought).
Pick the flavor that matches your Capstone bug. Three are worked below; do the one that fits.
Worked example A: a ClusterApplierService listener race
Symptom from Step 2: after a fast index-create-then-delete, a component that
holds a per-index resource (a cache, a directory handle) occasionally leaks it,
because its ClusterStateListener observed the "created" state but not the
"deleted" state.
Why 1: Why did the listener miss the delete?
Because ClusterApplierService applies committed cluster states in order on the
clusterApplierService#updateTask thread, and the listener allocates the
resource in response to the create delta. The probe trace from Step 3 shows
the listener ran for the create state (version N) but the delete state (N+1) was
batched away.
Why 2: Why was the delete delta not delivered to this listener?
Because the listener computes its delta from
ClusterChangedEvent.indicesDeleted(), and when create (N) and delete (N+1) are
coalesced into one applied state transition, the diff this listener sees can
jump from "before create" straight past, depending on which states actually got
applied versus skipped.
Why 3: Why is the resource allocated on clusterChanged rather than torn
down deterministically?
Because the component couples resource lifecycle to observed state deltas
instead of to a reconciliation against the current Metadata. Delta-driven
listeners assume they see every intermediate state; they do not — the applier is
allowed to skip states it never had to apply locally.
Why 4: Why does the code assume every intermediate state is observed?
Because when the listener was written, ClusterApplierService applied states
one-by-one with no coalescing for this path, so "see every delta" was true. A
later change began allowing the local applier to advance past states it did not
need, breaking the invariant the listener silently relied on.
Why 5: Why was the invariant not asserted or documented?
Because the contract of ClusterStateListener — "you may not see every
intermediate state; reconcile, don't accumulate" — lives in tribal knowledge and
a comment in ClusterApplier, not in a type or assertion. The listener author
reasonably read the local behavior, not the contract.
Root cause statement: The component drives a resource lifecycle off
ClusterChangedEvent deltas, which is unsound because ClusterApplierService
may advance the local applied state past intermediate cluster states without
delivering each delta; under a fast create→delete the teardown delta is never
observed and the resource leaks. The fix is to reconcile against the current
Metadata in clusterChanged (compute what should exist now and close
anything that should not), rather than to react to indicesDeleted().
Worked example B: a seqno/version edge in InternalEngine
Symptom: under two concurrent updates to the same _id, a stale version
occasionally wins, or a VersionConflictEngineException is thrown when it should
not be.
- Why 1: Two
indexoperations for the same_idinterleave inInternalEngine.index(...). - Why 2: Both read the live version from the
LiveVersionMapbefore either writes its own, so they resolve against the same base version. - Why 3: The
acquireLock(uid)window does not cover both the version resolution and the seqno assignment for one of the paths (e.g. an optimization that resolves version outside the per-uidlock). - Why 4: That optimization was added to reduce lock contention on the hot indexing path, under the assumption that the version map read was idempotent.
- Why 5: It is idempotent in isolation but not under the specific ordering
where the
LocalCheckpointTrackeradvances between the two reads.
Root cause shape: a lock window narrower than the read-modify-write it must protect, introduced as a contention optimization, valid until a second writer arrives at exactly the wrong checkpoint boundary.
Worked example C: an aggregation reduce error merging empty shards
Symptom (from the Step 2 / Step 3 examples): a terms aggregation with
min_doc_count: 0 and a missing value drops the synthetic bucket when one or
more shards contribute zero matching documents.
- Why 1: The merged bucket list from
InternalTerms.reduce(...)omits themissingbucket. - Why 2:
reduceonly emits buckets it sees from at least one shard, but a shard with zero docs returns no buckets at all for that field. - Why 3: The
min_doc_count: 0"produce empty buckets" responsibility lives on the shard-local aggregator, which short-circuits when the field has no values in its segment. - Why 4: The short-circuit was a performance optimization for the common
case (
min_doc_count >= 1), where empty buckets are discarded anyway. - Why 5: The optimization did not special-case
min_doc_count == 0combined withmissing, so the synthetic bucket that should exist for every shard is produced by no shard, and reduce has nothing to merge.
Root cause shape: a shard-local fast path that is correct for the dominant parameterization but wrong for one parameter combination the reduce step then cannot recover from.
Any of these three is "a root cause." The fix direction is now obvious-ish. You can argue between options — but you know what each one changes.
Git Archaeology
Once you have a candidate cause, ask: when did this break? And why did the person who wrote it think it was correct?
git log -S / -G (the pickaxe)
Find every commit that introduced or removed a specific string, or that touched a regex:
cd ~/OpenSearch
# Every commit that changed how many times this expression appears (-S = pickaxe).
git log -p -S "indicesDeleted" \
-- server/src/main/java/org/opensearch/cluster/ClusterChangedEvent.java
# -G matches commits whose diff text matches a regex (broader than -S).
git log -p -G "min_doc_count|minDocCount" \
-- server/src/main/java/org/opensearch/search/aggregations/bucket/terms/
# Follow a file through renames (the fork renamed many files).
git log --follow --oneline \
-- server/src/main/java/org/opensearch/index/engine/InternalEngine.java
-S matches commits where the count of that string changed — added or
removed. It is the single most powerful git command in this chapter. Learn it.
git blame -L <start>,<end>
Once you know the file and lines (from Step 3), find the commit and author:
git blame -L 2740,2770 \
server/src/main/java/org/opensearch/index/engine/InternalEngine.java
Output looks like:
a1b2c3d4 (Dev A 2022-08-14 09:34:18 +0000 2745) try (Releasable ignored = versionMap.acquireLock(op.uid().bytes())) {
a1b2c3d4 (Dev A 2022-08-14 09:34:18 +0000 2746) ...
Then read the commit and find its PR:
git show a1b2c3d4
git log -1 --format="%B" a1b2c3d4 # full message; OpenSearch commits embed "(#NNNN)"
OpenSearch squash-merges PRs, so the commit subject usually ends with the PR
number: ... (#4567). That number is your portal:
# Open the introducing PR's discussion in the browser, or via gh:
gh pr view 4567 --repo opensearch-project/OpenSearch --comments
Read every comment on that PR. Often you will discover:
- The change was made to fix a different bug (or as a perf optimization) and introduced your bug as collateral.
- A reviewer flagged the exact concern you are now hitting ("does this still hold when the applier skips a state?") and it was deferred.
- The fix you are considering was discussed and rejected for a reason you must now address.
git bisect Against Tags
If the bug is a regression — passed on 2.11.0, fails on main (you proved this
in Step 2) — bisect finds the exact introducing
commit. This is the highest-confidence signal in root-cause work.
cd ~/OpenSearch
git fetch origin --tags
git bisect start
git bisect bad main
git bisect good 2.11.0
# git checks out a midpoint. Build only what you need and run the repro:
./gradlew :server:test \
--tests "org.opensearch.*.TestThing.testIssueNNNNRepro" -Dtests.seed=DEADBEEF -q
# If the test FAILS here, the bug exists at this commit:
git bisect bad
# If it PASSES here, the bug came later:
git bisect good
# Repeat. git narrows to one commit in log2(N) steps.
Once converged:
a1b2c3d4 is the first bad commit
commit a1b2c3d4
Author: Dev A <a@example.org>
Date: Sun Aug 14 09:34:18 2022 +0000
Optimize version map locking on the indexing hot path (#4567)
Automate it once your repro returns a clean exit code:
git bisect run ./gradlew :server:test --tests "org.opensearch.*.testIssueNNNNRepro" -q
Now you know the introducing PR (#4567), the author (a natural reviewer to
request — @-mention them), and the exact diff to study.
Note: Bisecting across the Elasticsearch→OpenSearch fork boundary (pre-1.0) is rarely useful — the package rename (
org.elasticsearch→org.opensearch) means the codebase won't build the same way. Bisect within OpenSearch tags (1.x,2.x,main). If the bug predates 1.0, say so in the doc and lean ongit log -Sandblameinstead.
Writing the Root-Cause Statement
This document goes into your PR description and your write-up. 200–500 words. Use this template:
## Root cause: #NNNN
### Symptom
<one sentence — what the user sees>
### Trigger conditions
- <condition 1, e.g. fast create-then-delete of an index>
- <condition 2, e.g. the applier coalesces the two states locally>
- <condition 3 if any>
### Affected code
- server/src/main/java/org/opensearch/.../SomeComponent.java (the listener)
- server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java
(where state application may advance past intermediate states)
### Mechanism
<three to five sentences explaining the actual defect. Use "because",
"as a result", "however". This is the part most people get wrong — they
describe the symptom again instead of the mechanism. The mechanism answers:
of the many ways this code could have been written, why does the current way
produce this wrong answer?>
### Introducing change
- PR #4567 (commit a1b2c3d4) added <X> under the assumption <Y>, which no
longer holds because <Z>.
- A reviewer raised this on the PR (link to comment); resolution was deferred.
### Fix direction
Three options considered:
1. **<smallest change>.** Risk: <...>.
2. **<narrower-blast-radius change>.** Recommended.
3. **<cheapest / push-burden-elsewhere change>.** Rejected because <...>.
Recommended: option 2. See Step 5 for the diff.
Save as capstone-work/root-cause.md.
A note on the Mechanism section, because it is where most contributors fail the rubric: for the listener-race example, the symptom is "resource leaks." The mechanism is "the component accumulates lifecycle state from per-delta events, but the applier's contract permits skipping deltas, so a coalesced create→delete delivers no teardown delta and the accumulated state never gets corrected." Notice the mechanism explains the design assumption that broke, not the user-visible effect.
Validating the Hypothesis
A root cause is not validated until you have demonstrated it. Two ways.
1. Revert the introducing commit and re-run the repro
git checkout main
git revert --no-commit a1b2c3d4 # introducing commit from bisect
./gradlew :server:test --tests "org.opensearch.*.testIssueNNNNRepro" -q
If the test now PASSES (because you reverted the change that introduced the bug), your root cause is at least partially correct. If it still FAILS, the introducing commit is not the root cause — there is a deeper issue. Reset before you go further:
git reset --hard origin/main
2. A minimal one-line "patch" that confirms the mechanism
You are not writing the real fix yet. You are confirming the mechanism. For the aggregation example:
--- a/server/src/main/java/org/opensearch/search/aggregations/bucket/terms/TermsAggregator.java
+++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/terms/TermsAggregator.java
@@
- if (segmentHasNoValuesForField(field)) {
- return; // fast path: nothing to do
+ if (segmentHasNoValuesForField(field) && bucketCountThresholds.getMinDocCount() != 0) {
+ return; // fast path only when empty buckets are discarded anyway
}
Apply it, re-run the repro. If it passes, the mechanism is confirmed — even if this exact diff is not the final, conventionally-correct fix.
If the test still fails: your mechanism is wrong. Go back to the five-whys.
If the test now passes but breaks 14 other tests: your fix direction is too broad. Go back to "fix direction" and pick a narrower option. That signal is valuable — it is the cheap version of the feedback you would otherwise get in Step 7 validation or a reviewer's comment.
Validation / Self-check
Before advancing to Step 5:
capstone-work/root-cause.mdexists, follows the template, is 200–500 words, and its Mechanism section explains the broken design assumption — not the symptom restated.- You can name the introducing commit (full SHA) and PR number.
- You ran
git bisectto convergence against real tags (or documented why bisect doesn't apply — bug predates 1.0, or existed since the file's first commit). - You ran a "revert introducing commit" experiment and saw the test go green (or documented why the revert doesn't apply).
- You wrote a one-line throwaway "mechanism confirmation" patch and saw the test pass on it.
- You read every comment on the introducing PR (
gh pr view <N> --comments). - You can articulate three fix directions and explain in one sentence each why you rejected two.
Then go to Step 5: Implementation.