Skip to content

fix(opensearch): readiness report degrades instead of failing when one engine is unreachable (#37636) - #37724

Merged
fabrizzio-dotCMS merged 4 commits into
mainfrom
37636-readiness-unreachable-engine
Sep 24, 2026
Merged

fabrizzio-dotCMS merged 4 commits into
mainfrom
37636-readiness-unreachable-engine

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Closes #37636.

The problem

GET /api/v1/index/migration/readiness goes dark whenever one search engine cannot be reached: the response is only the connection error ({"message":"elasticsearch: Name or service not known"}), with no phase, no verdict and no rows for the engine that is up. That includes retiring the old Elasticsearch cluster at phase 3, the runbook's last step.

Two causes:

  • Both reconcilers call one engine-wide method per engine with no failure handling: ContentIndexMirrorReconciler calls getIndicesStats(), and SiteSearchMirrorReconciler calls listIndices(). Elasticsearch throws, and the whole evaluate() goes with it.
  • OpenSearch's OSIndexAPIImpl.getIndicesStats() / listIndices() do the opposite and answer any failure with an empty result, so a stopped OpenSearch read as "OpenSearch has no indices": a false "safe to advance" at phase 0, and a "no OpenSearch copy — reindex" blocker at phase 3.

The change

  • An engine that cannot be read is recorded in a new unreachableEngines field on the report (engine → reason, omitted when both answered). Its side of every row becomes EngineCopy.unavailable(...) (exists=false, docCount=-1, new unavailableReason), it is asked nothing else, and the row's verdict is a new UNMEASURED rather than MISSING_COUNTERPART, so the report never prescribes a reindex or re-crawl over an unknown.
  • IndexAPI.getIndicesStatsOrThrow() / listIndicesOrThrow() and SiteSearchAPI.listIndicesOrThrow() are new default methods that delegate to the existing ones; the OpenSearch implementations override them to propagate. Only the two reconcilers use them — every other caller (router merge, index-stats JSPs) keeps today's behaviour.
  • The Elasticsearch client's node-failure log line now says what happened instead of printing only [host=...].

With both engines reachable the report is unchanged (the new fields are omitted).

Verdict rules — the decision to review

The issue does not say what the verdict should be over a partial view. This PR sets:

  • safeToAdvance is false in phases 0–2 whenever either engine could not be read (every next phase writes to both), with one blocker per engine that names it and the reason.
  • At phase 3, an unreachable Elasticsearch is reported but does not block — nothing depends on it, and switching it off is the intended end state. The summary says rollback safety cannot be judged instead of warning that OpenSearch is "ahead". An unreachable OpenSearch does block.
  • safeToRollback is false whenever either engine could not be read. This follows the existing rule that an unmeasurable count on either side makes a downgrade unsafe. It is conservative: with only OpenSearch down, Elasticsearch's completeness could still be judged against the database, so this could be relaxed to "false only when Elasticsearch is unread". Left conservative here; happy to relax it.

About the logging half of the issue

The issue also reports the deprecated $estool.esSearch() path failing with a single unsearchable log line. That does not reproduce on main: at phase 3 the path no longer reaches Elasticsearch (#37667 fails it on the phase with a descriptive message), and in phases 0–2, reproduced with Elasticsearch stopped, the bare listener line is always followed by a WARN naming esSearch, the DotStateException and the cause — on page render, the Page API and /api/vtl/dynamic. Only the listener wording is changed here.

Verified

Unit: 13 new tests (content and Site Search reconcilers, readiness service per phase), all failing first against the old code; 84 tests across the five affected classes plus 44 in the neighbouring OpenSearch classes pass.

Live, Elasticsearch 7.10.2 + OpenSearch 3.4.0, stopping one engine at a time — every case HTTP 200, the stopped engine in unreachableEngines with its reason, rows UNMEASURED:

Phase Stopped safeToAdvance safeToRollback
0 Elasticsearch false false
0 OpenSearch false false
1 Elasticsearch false false
3 Elasticsearch true false
3 OpenSearch false false

The live runs needed #37723: on current main the Docker image cannot reach OpenSearch at all (#37722), so they ran on an image with jdk.net added to the runtime.

Out of scope, noted: with OpenSearch down at phase 3 the summary says "an outOfSyncCount of 0 here" while the field reads 2 — that wording predates this PR and belongs to #37638.

🤖 Generated with Claude Code

This PR fixes: #37636

fabrizzio-dotCMS and others added 3 commits September 23, 2026 14:16
…e engine is unreachable (#37636)

GET /api/v1/index/migration/readiness returned only the connection error
whenever Elasticsearch or OpenSearch could not be reached, because both
reconcilers called getIndicesStats()/listIndices() on each engine with no
failure handling. That includes retiring the old cluster at phase 3, the
runbook's last step.

An engine that cannot be read is now recorded in unreachableEngines, its
side of every row is EngineCopy.unavailable(...) with the reason, and the
row's verdict is UNMEASURED rather than MISSING_COUNTERPART, so the report
never prescribes a reindex or re-crawl over an unknown. The verdict:
safeToRollback is false while either engine is unread; safeToAdvance is
false in phases 0-2; at phase 3 an unreachable Elasticsearch is reported
in the summary but does not block, an unreachable OpenSearch does.

The Elasticsearch client's node-failure log line now says what happened
instead of printing only the host.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…"no indices" (#37636)

OSIndexAPIImpl.getIndicesStats() and listIndices() answer any failure with
an empty result, so with OpenSearch down the reconcilers read "OpenSearch
holds no indices": a false "safe to advance" at phase 0 and a "no
OpenSearch copy, reindex" blocker at phase 3.

Add getIndicesStatsOrThrow()/listIndicesOrThrow() to IndexAPI and
listIndicesOrThrow() to SiteSearchAPI as default methods that delegate to
the existing ones, override them on the OpenSearch side to propagate, and
use them from the two reconcilers only. Every other caller keeps today's
behaviour.

Verified against a running stack (ES 7.10.2 + OS 3.4.0) at phases 0, 1
and 3 with each engine stopped: HTTP 200, the stopped engine listed in
unreachableEngines with its reason, rows UNMEASURED.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The previous commit rewrote the file with LF endings, turning a 10-line
addition into a whole-file diff.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @fabrizzio-dotCMS's task in 2m 43s —— View job


PR Review

I reviewed the diff (git diff 63965801...HEAD) across the reconcilers, readiness service, MirrorStatus, and the new *OrThrow API surface. The verdict logic is internally consistent with the table in the PR body, and I confirmed the fix's premise for content: ESIndexAPI.getIndicesStats() propagates on failure (via performLowLevelRequest), so the default getIndicesStatsOrThrow() correctly surfaces an ES outage rather than reading as "no indices".

One confirmed regression, matching @dario-daza's earlier comment.

New Issues

  • 🟠 High: dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java:159-169 — The license gate now runs after the cluster call, not before. listIndices() and listIndicesOrThrow() pass indexApi.listIndices() / indexApi.listIndicesOrThrow() as an argument, so the OpenSearch round trip happens before siteSearchIndices(...) reaches the LicenseUtil.getLevel() < STANDARD check on line 173. Before this PR the check returned early and never touched the cluster. Consequences on a Community-license install:

    • Every listIndices() / downstream getAliasToIndexMap() call makes a throwaway OpenSearch round trip.
    • With OpenSearch down, OSIndexAPIImpl.listIndices() logs an ERROR with a full stack trace on each of those calls (line 353).
    • In the readiness report, SiteSearchMirrorReconciler.namesOrUnreachable calls listIndicesOrThrow(), which now throws for an unlicensed install with OpenSearch down — so the Site Search half marks OpenSearch unreachable even though Site Search isn't licensed and there is nothing to reconcile.

    Move the license check ahead of the indexApi call so nothing hits the cluster when unlicensed — e.g. have each public method check the license first and return Collections.emptyList() before delegating, or have siteSearchIndices take a Supplier<Set<String>> it only invokes past the gate. Fix this →

Notes (non-blocking)

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java:100-116 — The ES side of the Site Search half relies on ESSiteSearchAPI.listIndicesOrThrow() throwing when ES is down, but ESSiteSearchAPI doesn't override it, so it uses the default that delegates to listIndices(). This works only if ESSiteSearchAPI.listIndices() actually propagates on an unreachable engine (the content side does — I verified ESIndexAPI.getIndicesStats() throws). Worth confirming the Site Search ES path throws rather than returning empty; otherwise an ES outage would read as "no site-search indices" for Site Search only, the same failure this PR fixes for OpenSearch. What to verify: stop Elasticsearch and confirm the Site Search half reports ES in unreachableEngines, not zero rows.

  • The safeToRollback conservatism (false whenever either engine is unread) and the phase-3 outOfSyncCount-vs-summary wording are already called out as deliberate/out-of-scope in the PR description; no action needed here.

The UNMEASURED verdict, EngineCopy.unavailable(...)/wasRead(), the per-engine blocker (with the Phase-3 ES exception), and the expectedMissingCounterpart guard requiring os().wasRead() all hang together correctly across phases 0/1-2/3.
• 37636-readiness-unreachable-engine

dario-daza
dario-daza previously approved these changes Sep 24, 2026
…37636)

The both-down message read "Neither Elasticsearch nor OpenSearch could not be
reached", a double negative that states the opposite. It now says both could
not be reached and gives each engine's reason.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Sep 24, 2026
@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit f7d3c08 Sep 24, 2026
78 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the 37636-readiness-unreachable-engine branch September 24, 2026 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : Backend PR changes Java/Maven backend code Team : Scout

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Retiring the source engine: deprecated search fails with an unsearchable log line, and the readiness report stops responding

2 participants