Skip to content

Commit e0fbb1d

Browse files
fix(opensearch): fail loudly on migration init failure + surface ES shard failures (#36237) (#36240)
## Proposed Changes The two diagnostics/robustness follow-ups tracked in #36237 (the bootstrap-idempotency fix is #36238). Both target the same incident class where a migration startup problem silently degraded into a confusing downstream symptom. ### 1. `ContentletIndexAPIImpl.checkAndInitializeIndex()` — fail loudly during migration The catch-all swallowed **every** bootstrap exception — including the Phase 3 `DotRuntimeException` that is thrown a few lines above specifically to "abort loudly". The instance then came up **half-initialised**, and the read provider queried an unfinished/missing index, surfacing downstream as `all shards failed`. Now, when a migration is active (`isMigrationStarted() || isReadEnabled() || isMigrationComplete()`), the failure is re-thrown to abort startup so the operator restores connectivity/config and restarts. **Legacy ES-only installs (migration not started) keep the historical best-effort boot** — no behavior change for non-migration deployments. ### 2. `ContentFactoryIndexOperationsES` — surface the per-shard root cause Both the search and count catch blocks logged only `e.getCause().getMessage()`, hiding the actionable detail behind the generic `search_phase_execution_exception / all shards failed` wrapper. Added `describeEsFailure(ElasticsearchException)` which renders: - `getDetailedMessage()` (full ES type/reason chain), - the per-shard `ShardSearchFailure` reasons (capped at 5, with an "... and N more" summary), - any suppressed server-side causes. The enriched message still contains the substrings `shouldQueryCache(...)` keys off, so the error-caching behavior is unchanged. ## Testing - `./mvnw compile -pl :dotcms-core` (JDK 25) — compiles clean. - No public/interface signatures changed. ## Relationship to other PRs - #36238 — makes index bootstrap idempotent (removes the most common trigger of the swallowed failure). This PR is the defense-in-depth + observability layer on top; the two are independent and can merge in any order. Part of #36237 (does not close it — the parent issue is also addressed by #36238). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent b4216cb commit e0fbb1d

2 files changed

Lines changed: 58 additions & 3 deletions

File tree

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentFactoryIndexOperationsES.java

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.elasticsearch.action.search.SearchPhaseExecutionException;
3636
import org.elasticsearch.action.search.SearchRequest;
3737
import org.elasticsearch.action.search.SearchResponse;
38+
import org.elasticsearch.action.search.ShardSearchFailure;
3839
import org.elasticsearch.client.RequestOptions;
3940
import org.elasticsearch.client.core.CountRequest;
4041
import org.elasticsearch.client.core.CountResponse;
@@ -83,6 +84,48 @@ private boolean shouldQueryCache(final String exceptionMsg) {
8384
exception.contains("search_phase_execution_exception");
8485
}
8586

87+
/** Cap on per-shard failures rendered in a log line; the rest are summarised as a count. */
88+
private static final int MAX_SHARD_FAILURES_LOGGED = 5;
89+
90+
/**
91+
* Builds a diagnostic message that surfaces the <em>root</em> cause of an ES search/count
92+
* failure rather than the generic wrapper. For {@code search_phase_execution_exception}
93+
* ("all shards failed") the actionable detail lives in the per-shard failures and the
94+
* suppressed server-side exceptions — {@code getMessage()}/{@code getCause()} alone hide it,
95+
* which previously made these failures undiagnosable from the logs.
96+
*
97+
* @param e the ES exception caught from a search or count request
98+
* @return a multi-line message including the detailed message, per-shard reasons (capped),
99+
* and any suppressed causes
100+
*/
101+
private static String describeEsFailure(final ElasticsearchException e) {
102+
final StringBuilder sb = new StringBuilder(e.getDetailedMessage());
103+
104+
if (e instanceof SearchPhaseExecutionException) {
105+
final ShardSearchFailure[] failures = ((SearchPhaseExecutionException) e).shardFailures();
106+
if (failures != null && failures.length > 0) {
107+
final int shown = Math.min(failures.length, MAX_SHARD_FAILURES_LOGGED);
108+
for (int i = 0; i < shown; i++) {
109+
sb.append('\n')
110+
.append(" shard[").append(i).append("]: ").append(failures[i].reason());
111+
}
112+
if (failures.length > shown) {
113+
sb.append('\n')
114+
.append(" ... and ").append(failures.length - shown).append(" more shard failure(s)");
115+
}
116+
}
117+
}
118+
119+
for (final Throwable suppressed : e.getSuppressed()) {
120+
// A suppressed Throwable may carry a null message (e.g. a bare NPE); fall back to
121+
// its toString() so the line never renders the literal "null".
122+
final String suppressedMsg = suppressed.getMessage();
123+
sb.append('\n').append(" caused by: ")
124+
.append(null != suppressedMsg ? suppressedMsg : suppressed.toString());
125+
}
126+
return sb.toString();
127+
}
128+
86129
/**
87130
* if enabled SearchRequests are executed and then cached
88131
* @param searchRequest
@@ -105,7 +148,7 @@ private SearchHits cachedIndexSearch(final SearchRequest searchRequest) {
105148
}
106149
return hits;
107150
} catch (final ElasticsearchStatusException | IndexNotFoundException | SearchPhaseExecutionException e) {
108-
final String exceptionMsg = (null != e.getCause() ? e.getCause().getMessage() : e.getMessage());
151+
final String exceptionMsg = describeEsFailure(e);
109152
Logger.warn(this.getClass(), "----------------------------------------------");
110153
Logger.warn(this.getClass(), String.format("Elasticsearch SEARCH error in index '%s'", (searchRequest.indices()!=null) ? String.join(",", searchRequest.indices()): "unknown"));
111154
Logger.warn(this.getClass(), String.format("Thread: %s", Thread.currentThread().getName() ));
@@ -171,7 +214,7 @@ public Long cachedIndexCount(final CountRequest countRequest) {
171214
}
172215
return count;
173216
} catch (final ElasticsearchStatusException | IndexNotFoundException | SearchPhaseExecutionException e) {
174-
final String exceptionMsg = (null != e.getCause() ? e.getCause().getMessage() : e.getMessage());
217+
final String exceptionMsg = describeEsFailure(e);
175218
Logger.warn(this.getClass(), "----------------------------------------------");
176219
Logger.warn(this.getClass(), String.format("Elasticsearch error in index '%s'", (countRequest.indices()!=null) ? String.join(",", countRequest.indices()): "unknown"));
177220
Logger.warn(this.getClass(), String.format("ES Query: %s", String.valueOf(countRequest.source()) ));

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -521,7 +521,7 @@ private boolean indexReadyOS() {
521521
@CloseDBIfOpened
522522
public synchronized void checkAndInitializeIndex() {
523523
try {
524-
if (isMigrationStarted() || isReadEnabled() || isMigrationComplete()) {
524+
if (isMigrationStarted()) {
525525
if (!IndexStartupValidator.validateIndexingConfig()) {
526526
if (isMigrationComplete()) {
527527
// Phase 3: ES is decommissioned. Rolling back to Phase 0 would route
@@ -554,6 +554,18 @@ public synchronized void checkAndInitializeIndex() {
554554
} catch (Exception e) {
555555
Logger.fatal(this.getClass(), "Failed to create new indexes:" + e.getMessage(),e);
556556

557+
// During an active OpenSearch migration a half-initialised index store is worse
558+
// than a hard stop: the read provider would serve an unfinished or missing index
559+
// (surfacing downstream as "all shards failed"), and a Phase 3 validation failure
560+
// intentionally throws above (line ~530) only to be swallowed here. Fail loudly so
561+
// the operator restores connectivity/config and restarts, rather than silently
562+
// coming up broken. Legacy ES-only installs (migration not started) keep the
563+
// historical best-effort behaviour and still boot.
564+
if (isMigrationStarted()) {
565+
throw new DotRuntimeException(
566+
"Index initialization failed during OpenSearch migration; aborting startup"
567+
+ " to avoid serving an unfinished index store. Cause: " + e.getMessage(), e);
568+
}
557569
}
558570
}
559571

0 commit comments

Comments
 (0)