diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java index 524edb3a7983..dfe385e1694f 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/OSSiteSearchAPI.java @@ -60,6 +60,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import javax.enterprise.context.ApplicationScoped; import javax.enterprise.inject.Default; @@ -156,12 +157,25 @@ public OSSiteSearchAPI() { @Override public List listIndices() { + return siteSearchIndices(indexApi.listIndices()); + } + + /** + * Same listing, but a failure to reach OpenSearch propagates instead of reading as "no indices" + * (issue #37636). + */ + @Override + public List listIndicesOrThrow() { + return siteSearchIndices(indexApi.listIndicesOrThrow()); + } + + private List siteSearchIndices(final Set allIndices) { if (LicenseUtil.getLevel() < LicenseLevel.STANDARD.level) { return Collections.emptyList(); } // The physical OS indices are .os-tagged; strip back to logical names so the ES∪OS merge in // SiteSearchAPIImpl.listIndices() deduplicates and no .os leaks to the portlet (issue #36672). - final List indices = indexApi.listIndices().stream() + final List indices = allIndices.stream() .filter(IndexType.SITE_SEARCH::is) .map(IndexTag::strip) .distinct() diff --git a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/util/DotRestHighLevelClientProvider.java b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/util/DotRestHighLevelClientProvider.java index 980cdc516cd4..4e043d1d44cd 100644 --- a/dotCMS/src/main/java/com/dotcms/content/elasticsearch/util/DotRestHighLevelClientProvider.java +++ b/dotCMS/src/main/java/com/dotcms/content/elasticsearch/util/DotRestHighLevelClientProvider.java @@ -174,8 +174,15 @@ private RestClientBuilder getClientBuilder(final BasicCredentialsProvider creden return httpClientBuilder; }) .setFailureListener(new RestClient.FailureListener() { - public void onFailure(Node node) { - Logger.error(this, node.toString()); + /** + * Called by the low-level client when a request to a node fails and the node is + * marked dead. The exception is not passed here — it reaches the caller — so say + * what happened in words an operator would search for (issue #37636). + */ + @Override + public void onFailure(final Node node) { + Logger.error(this, "Elasticsearch node failed a request and was marked dead " + + "by the client; the caller receives the error: " + node); } }); diff --git a/dotCMS/src/main/java/com/dotcms/content/index/IndexAPI.java b/dotCMS/src/main/java/com/dotcms/content/index/IndexAPI.java index 924727bd2513..3d451f18ab4c 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/IndexAPI.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/IndexAPI.java @@ -69,6 +69,19 @@ public String getStatus() { */ Map getIndicesStats(); + /** + * Same as {@link #getIndicesStats()}, but a failure to read the engine propagates instead of being + * answered with an empty map. For callers that must tell "this engine holds no indices" from "this + * engine could not be asked" — the migration readiness report, where confusing the two prescribes + * a reindex over an outage (issue #37636). Implementations whose {@code getIndicesStats()} already + * propagates need not override it. + * + * @return a map of index names to their statistics + */ + default Map getIndicesStatsOrThrow() { + return getIndicesStats(); + } + /** * Flushes field and filter caches for the specified indices. * This operation can take up to a minute to complete. @@ -116,6 +129,17 @@ public String getStatus() { */ Set listIndices(); + /** + * Same as {@link #listIndices()}, but a failure to read the engine propagates instead of being + * answered with an empty set — see {@link #getIndicesStatsOrThrow()} for why that difference + * matters. + * + * @return set containing all index names + */ + default Set listIndicesOrThrow() { + return listIndices(); + } + /** * Checks if the specified index is closed. * diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java index c9231b77a375..b7fc20b5b500 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java @@ -22,6 +22,7 @@ import com.google.common.annotations.VisibleForTesting; import io.vavr.control.Try; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -45,7 +46,7 @@ * from the other there would report "Elasticsearch has no copy" for an index that exists and holds * content, so both stores are read and the generation split is reported as what it is (issue #37635).

* - *

Existence comes from each engine leaf's {@code getIndicesStats()} — one call per + *

Existence comes from each engine leaf's {@code getIndicesStatsOrThrow()} — one call per * engine covering the whole index set, so both slots are decided from a single snapshot. Those stats * maps are keyed by the cluster-stripped name (Elasticsearch un-tagged, OpenSearch carrying * {@code .os}), so each raw name is stripped of the cluster prefix and then, for the OpenSearch lookup, @@ -117,10 +118,23 @@ public record DatabaseCounts(Long working, Long live) {} * collapsing them into one value would hand the operator a confident instruction derived from an * unknown (issue #37635). {@code unreadableReason} is present only in the second case.

* - * @param statuses per-index mirror status; empty when no active pointers were resolved - * @param unreadableReason why the store could not be read, when that is what happened + *

{@code unreachableEngines} is the per-engine counterpart of {@code unreadableReason}: the + * pointers were read, but one engine's index stats could not be, so its side of every row is + * unknown while the other side is still reported (issue #37636).

+ * + * @param statuses per-index mirror status; empty when no active pointers were resolved + * @param unreadableReason why the store could not be read, when that is what happened + * @param unreachableEngines engine name → why that engine could not be read; empty when both were */ - public record ContentMirrors(List statuses, Optional unreadableReason) {} + public record ContentMirrors(List statuses, Optional unreadableReason, + Map unreachableEngines) { + + /** Both engines were read (or never needed to be). */ + public ContentMirrors(final List statuses, + final Optional unreadableReason) { + this(statuses, unreadableReason, Map.of()); + } + } /** * Per-index mirror status for the active working and live content indices. @@ -141,17 +155,45 @@ public ContentMirrors mirrors() { if (lookup.hasNoPointers()) { return new ContentMirrors(List.of(), Optional.empty()); } - return new ContentMirrors(statusesFor(lookup), Optional.empty()); + // One engine-wide stats call per engine. Either can fail on its own — the old cluster retired + // at Phase 3, or any outage — and the other engine's half of the report is still worth + // having, so a failure marks that engine unreachable instead of escaping (issue #37636). + final Map unreachable = new LinkedHashMap<>(); + final Map esStats = + statsOrUnreachable(esImpl, MirrorStatus.ELASTICSEARCH, unreachable); + final Map osStats = + statsOrUnreachable(osImpl, MirrorStatus.OPENSEARCH, unreachable); + return new ContentMirrors(statusesFor(lookup, esStats, osStats, unreachable), + Optional.empty(), Map.copyOf(unreachable)); + } + + /** + * One engine's index stats, or {@code null} after recording why that engine could not be read. + */ + private static Map statsOrUnreachable(final IndexAPI engine, + final String engineName, final Map unreachable) { + try { + // The propagating variant: the default OpenSearch getIndicesStats() answers an outage with + // an empty map, which would read here as "no copies" and prescribe a reindex. + return engine.getIndicesStatsOrThrow(); + } catch (Exception e) { + final String reason = MirrorStatus.reasonOf(e); + Logger.warn(ContentIndexMirrorReconciler.class, engineName + + " could not be reached for migration readiness; reporting its side of the " + + "content indices as unavailable: " + reason, e); + unreachable.put(engineName, reason); + return null; + } } - private List statusesFor(final PointerLookup pointers) { - final Map esStats = esImpl.getIndicesStats(); - final Map osStats = osImpl.getIndicesStats(); + private List statusesFor(final PointerLookup pointers, + final Map esStats, final Map osStats, + final Map unreachable) { final DatabaseCounts dbCounts = databaseCountsSupplier.get(); final List out = new ArrayList<>(2); - addStatus(out, IndexKind.CONTENT_WORKING, pointers.working(), esStats, osStats, + addStatus(out, IndexKind.CONTENT_WORKING, pointers.working(), esStats, osStats, unreachable, dbCounts == null ? null : dbCounts.working()); - addStatus(out, IndexKind.CONTENT_LIVE, pointers.live(), esStats, osStats, + addStatus(out, IndexKind.CONTENT_LIVE, pointers.live(), esStats, osStats, unreachable, dbCounts == null ? null : dbCounts.live()); return out; } @@ -251,9 +293,15 @@ private static PointerLookup readFailure(final String store, final Exception e) return PointerLookup.failed(store + " could not be read: " + e.getMessage()); } + /** + * Adds the row for one slot. A {@code null} stats map means that engine could not be read: its + * side is reported as unavailable (existence unknown, no count query sent) and the row's verdict + * is {@link Verdict#UNMEASURED}, so nothing downstream mistakes "unknown" for "missing". + */ private void addStatus(final List out, final IndexKind kind, final SlotPointers pointers, final Map esStats, - final Map osStats, final Long databaseDocCount) { + final Map osStats, final Map unreachable, + final Long databaseDocCount) { if (pointers.isUnset()) { return; } @@ -264,9 +312,9 @@ private void addStatus(final List out, final IndexKind kind, final String osBare = pointers.os() == null ? null : esImpl.removeClusterIdFromName(pointers.os()); // Existence from the stats snapshot; the count from a live count query (see class javadoc). - final boolean esExists = esBare != null && esStats.containsKey(esBare); + final boolean esExists = esStats != null && esBare != null && esStats.containsKey(esBare); final long esCount = esExists ? countQuietly(esOps, pointers.es()) : 0L; - final boolean osExists = osBare != null && osStats.containsKey(osBare); + final boolean osExists = osStats != null && osBare != null && osStats.containsKey(osBare); final long osCount = osExists ? countQuietly(osOps, pointers.os()) : 0L; // The row is named after the engine that owns the content in this phase, with the .os tag @@ -275,14 +323,23 @@ private void addStatus(final List out, final IndexKind kind, final String name = IndexTag.strip( IndexConfigHelper.isMigrationComplete() && osBare != null ? osBare : esBare); - final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - final String recommendation = recommend(name, verdict, osExists) + final MirrorStatus.EngineCopy esCopy = esStats == null + ? MirrorStatus.EngineCopy.unavailable(pointers.es(), null, + unreachable.get(MirrorStatus.ELASTICSEARCH)) + : new MirrorStatus.EngineCopy(esExists, esCount, pointers.es()); + final MirrorStatus.EngineCopy osCopy = osStats == null + ? MirrorStatus.EngineCopy.unavailable(pointers.os(), null, + unreachable.get(MirrorStatus.OPENSEARCH)) + : new MirrorStatus.EngineCopy(osExists, osCount, pointers.os()); + + final Verdict verdict = MirrorStatus.verdictFor(esCopy, osCopy); + final String recommendation = (verdict == Verdict.UNMEASURED + ? MirrorStatus.unmeasuredAdvice("content index", name, esCopy, osCopy) + : recommend(name, verdict, osExists)) + incompleteNote("Elasticsearch", esExists, esCount, databaseDocCount) + incompleteNote("OpenSearch", osExists, osCount, databaseDocCount); - out.add(new MirrorStatus(name, kind, - new MirrorStatus.EngineCopy(esExists, esCount, pointers.es()), - new MirrorStatus.EngineCopy(osExists, osCount, pointers.os()), - verdict, recommendation, databaseDocCount)); + out.add(new MirrorStatus(name, kind, esCopy, osCopy, verdict, recommendation, + databaseDocCount)); } /** diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java index 10d361f6b3dc..7bde45314387 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadiness.java @@ -1,5 +1,6 @@ package com.dotcms.content.index.migration; +import com.fasterxml.jackson.annotation.JsonInclude; import java.util.List; import java.util.Map; @@ -18,13 +19,19 @@ * @param siteSearch the Site Search indices as a list — an open set with no natural key, so a list * (each entry carries its own {@code indexName}) * @param verdict the overall go/no-go for advancing and rolling back, with reasons + * @param unreachableEngines the engines that could not be read while building this report, keyed by + * engine name ({@code Elasticsearch} / {@code OpenSearch}) with the failure reason. + * The rows are still reported for the engine that answered; the other engine's side + * of each row carries the same reason (issue #37636). Omitted from the JSON when + * both engines were read. */ public record MigrationReadiness( String clusterId, PhaseInfo phase, Map content, List siteSearch, - Verdict verdict) { + Verdict verdict, + @JsonInclude(JsonInclude.Include.NON_EMPTY) Map unreachableEngines) { /** * @param current the current phase ordinal (0–3) diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java index 92b4fbcd2220..166006a63084 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java @@ -4,6 +4,8 @@ import com.dotcms.enterprise.cluster.ClusterFactory; import com.dotcms.content.index.migration.ContentIndexMirrorReconciler.ContentMirrors; import com.dotcms.content.index.migration.MirrorStatus.IndexKind; +import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.content.index.migration.SiteSearchMirrorReconciler.SiteSearchMirrors; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -60,8 +62,14 @@ public MigrationReadinessService() { public MigrationReadiness evaluate() { final MigrationPhase phase = MigrationPhase.current(); final ContentMirrors contentMirrors = contentReconciler.mirrors(); + final SiteSearchMirrors siteSearchMirrors = siteSearchReconciler.mirrors(); final List content = new ArrayList<>(contentMirrors.statuses()); - final List siteSearch = new ArrayList<>(siteSearchReconciler.statuses()); + final List siteSearch = new ArrayList<>(siteSearchMirrors.statuses()); + // An engine that failed on either half is unreachable for the whole report; the first reason + // recorded is kept, since both halves see the same outage. + final Map unreachable = new LinkedHashMap<>(contentMirrors.unreachableEngines()); + siteSearchMirrors.unreachableEngines().forEach(unreachable::putIfAbsent); + final boolean elasticsearchUnreachable = unreachable.containsKey(MirrorStatus.ELASTICSEARCH); final List all = new ArrayList<>(content.size() + siteSearch.size()); all.addAll(content); @@ -78,8 +86,9 @@ public MigrationReadiness evaluate() { // nothing to report on — a hard no-go, independent of the sync check, which would otherwise // pass vacuously when there are no active indices at all. Site Search is an open set that may // legitimately be empty, so it is not required here. - final List missingContent = - requiredContentBlockers(content, phase, contentMirrors.unreadableReason()); + final List missingContent = requiredContentBlockers(content, phase, + contentMirrors.unreadableReason(), unreachable); + final List unreachableBlockers = unreachableEngineBlockers(phase, unreachable); final boolean safeToAdvance; final String summary; @@ -92,6 +101,7 @@ public MigrationReadiness evaluate() { // of health over zero measurements: outOfSyncCount reads 0 because nothing was counted and // the rollback verdict reads safe because no index could show Elasticsearch behind // OpenSearch (issue #37635). Report the gap instead of the vacuous all-clear. + blockers.addAll(unreachableBlockers); blockers.addAll(missingContent); safeToAdvance = blockers.isEmpty(); summary = safeToAdvance @@ -103,7 +113,13 @@ public MigrationReadiness evaluate() { + "measured, or holds materially less than the database; run a " + "full reindex. ", plural(outOfSync.size(), "index", "indices"), outOfSync.size() == 1 ? "has" : "have")) - + (esBehindAnywhere + // A retired Elasticsearch is the runbook's end state, not a fault — but it + // was not measured, so claiming OpenSearch is "ahead" of it would be a guess. + + (elasticsearchUnreachable + ? String.format("Elasticsearch could not be reached (%s), so whether a " + + "downgrade is safe cannot be judged; do not downgrade on this " + + "reading.", unreachable.get(MirrorStatus.ELASTICSEARCH)) + : esBehindAnywhere ? "WARNING: OpenSearch holds content Elasticsearch does not; a downgrade " + "would hide it until a full reindex." : "No index shows Elasticsearch behind OpenSearch; still verify before any " @@ -115,7 +131,9 @@ public MigrationReadiness evaluate() { + "attempted on this reading.", plural(blockers.size(), "blocker")); } else if (phase.isMigrationNotStarted()) { // Phase 0: OpenSearch counterparts are built later (during dual-write), so their absence is - // expected and NOT a blocker; only the mandatory Elasticsearch content pair is required. + // expected and NOT a blocker; only the mandatory Elasticsearch content pair is required — + // and both engines reachable, since advancing starts writing to OpenSearch. + blockers.addAll(unreachableBlockers); blockers.addAll(missingContent); safeToAdvance = blockers.isEmpty(); summary = safeToAdvance @@ -133,13 +151,17 @@ public MigrationReadiness evaluate() { outOfSync.size() == 1 ? "is" : "are")) : String.format("Not safe to advance from Phase 0: %s to resolve first (see the " + "blockers list). Dual-write needs an active Elasticsearch content index " - + "to mirror from.", plural(blockers.size(), "blocker")); + + "to mirror from and a reachable OpenSearch to mirror into.", + plural(blockers.size(), "blocker")); } else { // Phases 1/2 (dual-write): require the mandatory content pair AND every mirror in sync. Drop // out-of-sync rows already reported as missing content (an ES-missing content slot surfaces // both as a missing-source blocker and as MISSING_COUNTERPART) so it is not reported twice. + // Rows left unmeasured by an unreachable engine are covered by that engine's one blocker. + blockers.addAll(unreachableBlockers); blockers.addAll(missingContent); outOfSync.stream() + .filter(s -> s.verdict() != Verdict.UNMEASURED) .filter(s -> !(isContent(s.kind()) && !s.es().exists())) .forEach(s -> blockers.add(String.format("%s '%s': %s", s.kind(), s.indexName(), s.recommendation()))); @@ -158,7 +180,9 @@ public MigrationReadiness evaluate() { // saw the mandatory content pair. Without it, "no index shows Elasticsearch behind OpenSearch" // is vacuously true over an empty list — and the rollback verdict is the one decision that // reading exists to protect (issue #37635). - final boolean safeToRollback = missingContent.isEmpty() && !esBehindAnywhere; + // Rollback compares the two engines, so it cannot be judged with either one unread. + final boolean safeToRollback = missingContent.isEmpty() && !esBehindAnywhere + && unreachable.isEmpty(); final MigrationReadiness.Verdict verdict = new MigrationReadiness.Verdict( safeToAdvance, safeToRollback, outOfSync.size(), summary, blockers); @@ -170,7 +194,7 @@ public MigrationReadiness evaluate() { contentBySlot.put(contentSlot(s.kind()), s); } return new MigrationReadiness(clusterIdSupplier.get(), phaseInfo, contentBySlot, - siteSearch, verdict); + siteSearch, verdict, Map.copyOf(unreachable)); } /** @@ -191,8 +215,9 @@ private static boolean needsAttentionIn(final MigrationPhase phase, final Mirror if (!status.needsAttention()) { return false; } - final boolean expectedMissingCounterpart = - phase.isMigrationNotStarted() && status.es().exists() && !status.os().exists(); + // "Not built yet" only applies to an OpenSearch that was actually read and found without it. + final boolean expectedMissingCounterpart = phase.isMigrationNotStarted() + && status.es().exists() && status.os().wasRead() && !status.os().exists(); return !expectedMissingCounterpart; } @@ -257,9 +282,16 @@ private static String contentSlot(final IndexKind kind) { * from it would be asserted over zero measurements (issues #36360 and #37635).

*/ private static List requiredContentBlockers(final List content, - final MigrationPhase phase, final Optional unreadableReason) { + final MigrationPhase phase, final Optional unreadableReason, + final Map unreachable) { final boolean openSearchOwnsContent = phase.isMigrationComplete(); - final String engine = openSearchOwnsContent ? "OpenSearch" : "Elasticsearch"; + final String engine = openSearchOwnsContent ? MirrorStatus.OPENSEARCH : MirrorStatus.ELASTICSEARCH; + // The owning engine could not be read, so whether its copies exist is unknown. Its own blocker + // (see unreachableEngineBlockers) says so; a per-slot "no copy — reindex" here would prescribe + // a rebuild over an unknown (issue #37636). + if (unreadableReason.isEmpty() && unreachable.containsKey(engine)) { + return List.of(); + } // A store that could not be read is NOT a store with no indices. Both leave the report with // no rows, but the operator action is the opposite — fix the read, do not reindex — and the // per-slot messages below would confidently prescribe the wrong one (issue #37635). @@ -287,6 +319,26 @@ private static List requiredContentBlockers(final List con return out; } + /** + * One blocker per engine that could not be read, except Elasticsearch in Phase 3: nothing there + * depends on it any more, and switching it off is the runbook's last step, so its absence is + * reported in the summary and in {@code safeToRollback} rather than as a reason not to advance + * (issue #37636). + */ + private static List unreachableEngineBlockers(final MigrationPhase phase, + final Map unreachable) { + final List out = new ArrayList<>(unreachable.size()); + unreachable.forEach((engine, reason) -> { + if (phase.isMigrationComplete() && MirrorStatus.ELASTICSEARCH.equals(engine)) { + return; + } + out.add(String.format("%s could not be reached: %s. Its side of every index below is " + + "unknown, so this reading cannot support changing the phase. Restore access and " + + "re-run this check; do not reindex because of it.", engine, reason)); + }); + return out; + } + private static boolean isContent(final IndexKind kind) { return kind == IndexKind.CONTENT_WORKING || kind == IndexKind.CONTENT_LIVE; } diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java index eecb3be5a0b0..bf05b296ce75 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/MirrorStatus.java @@ -49,6 +49,12 @@ public record MirrorStatus( */ public static final double INCOMPLETE_INDEXED_THRESHOLD = 95.0; + /** Engine name as the report spells it, in {@code unreachableEngines} and in its messages. */ + public static final String ELASTICSEARCH = "Elasticsearch"; + + /** Engine name as the report spells it, in {@code unreachableEngines} and in its messages. */ + public static final String OPENSEARCH = "OpenSearch"; + /** A row with no database denominator — the shape the Site Search indices use. */ public MirrorStatus(final String indexName, final IndexKind kind, final EngineCopy es, final EngineCopy os, final Verdict verdict, final String recommendation) { @@ -65,7 +71,13 @@ public enum Verdict { /** The index exists on one engine but its counterpart is missing on the other. */ MISSING_COUNTERPART, /** Both copies exist but hold a different number of documents. */ - COUNT_DRIFT + COUNT_DRIFT, + /** + * One engine could not be read at all (unreachable, timing out, refusing the request), so the + * two copies could not be compared. Says nothing about whether the copy exists: the fix is to + * restore access and re-run the check, never to reindex on the strength of it (issue #37636). + */ + UNMEASURED } /** @@ -85,13 +97,45 @@ public enum Verdict { * @param alias the alias this engine has attached to the index, or {@code null} when it has * none — and always {@code null} for the content indices, which are addressed by * name only. Omitted from the JSON when {@code null}. + * @param unavailableReason why this engine could not be read at all, or {@code null} when it was + * read. When set, {@code exists} is {@code false} and {@code docCount} is + * {@code -1} because nothing is known, not because the copy is gone — read this + * field before either of them (issue #37636). Omitted from the JSON when + * {@code null}. */ public record EngineCopy(boolean exists, long docCount, String physicalName, - @JsonInclude(JsonInclude.Include.NON_NULL) String alias) { + @JsonInclude(JsonInclude.Include.NON_NULL) String alias, + @JsonInclude(JsonInclude.Include.NON_NULL) String unavailableReason) { /** An engine copy with no alias — the shape the content indices use. */ public EngineCopy(final boolean exists, final long docCount, final String physicalName) { - this(exists, docCount, physicalName, null); + this(exists, docCount, physicalName, null, null); + } + + /** An engine copy that was read, with the alias that engine attached to it. */ + public EngineCopy(final boolean exists, final long docCount, final String physicalName, + final String alias) { + this(exists, docCount, physicalName, alias, null); + } + + /** + * The copy on an engine that could not be read: existence unknown, count unmeasurable. + * + * @param physicalName the name to look for once the engine answers again (may be {@code null}) + * @param alias the alias, when known from elsewhere; usually {@code null} + * @param reason why the engine could not be read, as reported to the operator + */ + public static EngineCopy unavailable(final String physicalName, final String alias, + final String reason) { + return new EngineCopy(false, -1L, physicalName, alias, reason); + } + + /** + * Whether this engine was actually read. Not a JSON property: {@code unavailableReason} already + * carries the same fact. + */ + public boolean wasRead() { + return unavailableReason == null; } } @@ -204,6 +248,49 @@ private Double indexedPercentOf(final EngineCopy copy) { * would answer "safe to advance" about a mirror it never actually measured. Unknown degrades to * drift, which is what {@code needsAttention()} and {@code safeToAdvance} act on.

*/ + public static Verdict verdictFor(final EngineCopy es, final EngineCopy os) { + if (!es.wasRead() || !os.wasRead()) { + return Verdict.UNMEASURED; + } + return verdictFor(es.exists(), os.exists(), es.docCount(), os.docCount()); + } + + /** + * The recommendation for an {@link Verdict#UNMEASURED} row: which engine could not be read, why, + * and that the fix is access, not a rebuild. + * + * @param what a short label for the index family, e.g. {@code "content index"} + */ + public static String unmeasuredAdvice(final String what, final String name, final EngineCopy es, + final EngineCopy os) { + final String engines; + final String reason; + if (!es.wasRead() && !os.wasRead()) { + engines = "Both Elasticsearch and OpenSearch"; + reason = ELASTICSEARCH + ": " + es.unavailableReason() + "; " + OPENSEARCH + ": " + + os.unavailableReason(); + } else if (!es.wasRead()) { + engines = ELASTICSEARCH; + reason = es.unavailableReason(); + } else { + engines = OPENSEARCH; + reason = os.unavailableReason(); + } + return String.format("%s could not be reached (%s), so the two copies of %s '%s' could not " + + "be compared. Restore access and re-run this check; do not rebuild the index on the " + + "strength of this reading.", engines, reason, what, name); + } + + /** A failure as the report shows it: the message, or the exception type when there is none. */ + public static String reasonOf(final Throwable e) { + final String message = e.getMessage(); + return message == null || message.isBlank() ? e.getClass().getSimpleName() : message; + } + + /** + * Same classification from raw values, for two copies that were both read. A copy on an engine + * that could not be read goes through {@link #verdictFor(EngineCopy, EngineCopy)} instead. + */ public static Verdict verdictFor(final boolean esExists, final boolean osExists, final long esDocCount, final long osDocCount) { if (!esExists || !osExists) { diff --git a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java index 80e8e150775e..e385039416e7 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/migration/SiteSearchMirrorReconciler.java @@ -8,9 +8,11 @@ import com.dotcms.enterprise.publishing.sitesearch.OSSiteSearchAPI; import com.dotmarketing.business.APILocator; import com.dotmarketing.sitesearch.business.SiteSearchAPI; +import com.dotmarketing.util.Logger; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.TreeSet; @@ -48,23 +50,68 @@ public SiteSearchMirrorReconciler() { this.clusterPrefixSupplier = clusterPrefixSupplier; } + /** + * The Site Search half of the report, together with the engines that could not be read. + * + * @param statuses per-index mirror status + * @param unreachableEngines engine name → why that engine could not be read; empty when both were + */ + public record SiteSearchMirrors(List statuses, + Map unreachableEngines) {} + /** * The per-index mirror status for every logical site-search index that exists on either - * engine (so a counterpart missing on one side still appears). Purely factual and phase-independent. + * engine (so a counterpart missing on one side still appears), plus the engines that could not be + * read. Purely factual and phase-independent. + * + *

An engine whose index list cannot be read is recorded as unreachable and asked nothing else: + * the rows then come from the other engine alone, with this engine's side marked unavailable, so + * one engine's outage no longer takes the whole report down (issue #37636).

*/ - public List statuses() { - final TreeSet names = new TreeSet<>(esImpl.listIndices()); - names.addAll(osImpl.listIndices()); + public SiteSearchMirrors mirrors() { + final Map unreachable = new LinkedHashMap<>(); + final List esNames = namesOrUnreachable(esImpl, MirrorStatus.ELASTICSEARCH, unreachable); + final List osNames = namesOrUnreachable(osImpl, MirrorStatus.OPENSEARCH, unreachable); + final TreeSet names = new TreeSet<>(); + if (esNames != null) { + names.addAll(esNames); + } + if (osNames != null) { + names.addAll(osNames); + } // One alias lookup per engine for the whole set — not one per index. Operators identify a // site-search index by its alias, never by its sitesearch__ name, so the // report is unusable without it (issue #36983). - final Map esAliases = indexToAlias(esImpl); - final Map osAliases = indexToAlias(osImpl); + final Map esAliases = esNames == null ? Map.of() : indexToAlias(esImpl); + final Map osAliases = osNames == null ? Map.of() : indexToAlias(osImpl); final List statuses = new ArrayList<>(names.size()); for (final String name : names) { - statuses.add(statusFor(name, esAliases.get(name), osAliases.get(name))); + statuses.add(statusFor(name, esAliases.get(name), osAliases.get(name), + unreachable.get(MirrorStatus.ELASTICSEARCH), + unreachable.get(MirrorStatus.OPENSEARCH))); + } + return new SiteSearchMirrors(statuses, Map.copyOf(unreachable)); + } + + /** Kept for callers that only need the rows. See {@link #mirrors()}. */ + public List statuses() { + return mirrors().statuses(); + } + + /** One engine's site-search index names, or {@code null} after recording why it could not be read. */ + private static List namesOrUnreachable(final SiteSearchAPI engine, final String engineName, + final Map unreachable) { + try { + // The propagating variant — see ContentIndexMirrorReconciler for why. + return engine.listIndicesOrThrow(); + } catch (Exception e) { + final String reason = MirrorStatus.reasonOf(e); + Logger.warn(SiteSearchMirrorReconciler.class, engineName + + " could not be reached for migration readiness; reporting its side of the " + + "Site Search indices as unavailable: " + reason, e); + unreachable.put(engineName, reason); + return null; } - return statuses; } /** @@ -77,20 +124,33 @@ private static Map indexToAlias(final SiteSearchAPI engine) { return reversed; } - private MirrorStatus statusFor(final String name, final String esAlias, final String osAlias) { - final boolean esExists = esImpl.existsOnAllWriteEngines(name); - final boolean osExists = osImpl.existsOnAllWriteEngines(name); - final long esCount = esExists ? esImpl.documentCount(name) : 0L; - final long osCount = osExists ? osImpl.documentCount(name) : 0L; + /** + * The row for one index. A non-null {@code esUnreachable} / {@code osUnreachable} is the reason that + * engine could not be read: its side is reported as unavailable and it is not queried. + */ + private MirrorStatus statusFor(final String name, final String esAlias, final String osAlias, + final String esUnreachable, final String osUnreachable) { // Physical names as stored: ES is the cluster-prefixed logical name; OS is that + the .os tag // (applied via IndexTag, the sole owner of the marker). final String esPhysical = clusterPrefixSupplier.get() + name; final String osPhysical = IndexTag.OS.tag(esPhysical); - final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); - return new MirrorStatus(name, IndexKind.SITE_SEARCH, - new MirrorStatus.EngineCopy(esExists, esCount, esPhysical, esAlias), - new MirrorStatus.EngineCopy(osExists, osCount, osPhysical, osAlias), - verdict, recommend(name, verdict, esAlias, osAlias)); + final MirrorStatus.EngineCopy es = copyOn(esImpl, name, esPhysical, esAlias, esUnreachable); + final MirrorStatus.EngineCopy os = copyOn(osImpl, name, osPhysical, osAlias, osUnreachable); + final Verdict verdict = MirrorStatus.verdictFor(es, os); + final String recommendation = verdict == Verdict.UNMEASURED + ? MirrorStatus.unmeasuredAdvice("site-search index", name, es, os) + : recommend(name, verdict, esAlias, osAlias); + return new MirrorStatus(name, IndexKind.SITE_SEARCH, es, os, verdict, recommendation); + } + + private static MirrorStatus.EngineCopy copyOn(final SiteSearchAPI engine, final String name, + final String physicalName, final String alias, final String unreachableReason) { + if (unreachableReason != null) { + return MirrorStatus.EngineCopy.unavailable(physicalName, alias, unreachableReason); + } + final boolean exists = engine.existsOnAllWriteEngines(name); + final long count = exists ? engine.documentCount(name) : 0L; + return new MirrorStatus.EngineCopy(exists, count, physicalName, alias); } /** diff --git a/dotCMS/src/main/java/com/dotcms/content/index/opensearch/OSIndexAPIImpl.java b/dotCMS/src/main/java/com/dotcms/content/index/opensearch/OSIndexAPIImpl.java index aa252a2da505..5c7aab7bc8eb 100644 --- a/dotCMS/src/main/java/com/dotcms/content/index/opensearch/OSIndexAPIImpl.java +++ b/dotCMS/src/main/java/com/dotcms/content/index/opensearch/OSIndexAPIImpl.java @@ -16,6 +16,7 @@ import com.dotmarketing.business.APILocator; import com.dotmarketing.business.DotStateException; import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.util.AdminLogger; import com.dotcms.content.index.IndexConfigHelper; import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; @@ -348,6 +349,20 @@ public void openIndex(String indexName) { @Override public Set listIndices() { + try { + return listIndicesOrThrow(); + } catch (Exception e) { + Logger.error(this.getClass(), "Error listing indices", e); + return new HashSet<>(); + } + } + + /** + * Lists the cluster's indices, propagating a failure to reach OpenSearch rather than answering + * with an empty set (issue #37636). + */ + @Override + public Set listIndicesOrThrow() { try { final GetIndexRequest request = GetIndexRequest.of(b -> b.index(getClusterPrefix() + "*") @@ -356,9 +371,8 @@ public Set listIndices() { return response.result().keySet().stream() .map(this::removeClusterIdFromName) .collect(Collectors.toSet()); - } catch (Exception e) { - Logger.error(this.getClass(), "Error listing indices", e); - return new HashSet<>(); + } catch (IOException e) { + throw new DotRuntimeException("Could not list OpenSearch indices: " + e.getMessage(), e); } } @@ -448,6 +462,20 @@ public Status getIndexStatus(String indexName) throws DotDataException { @Override public Map getIndicesStats() { + try { + return getIndicesStatsOrThrow(); + } catch (final Exception e) { + Logger.error(this.getClass(), "Error fetching OS indices stats: " + e.getMessage(), e); + return new HashMap<>(); + } + } + + /** + * Index statistics for the cluster, propagating a failure to reach OpenSearch rather than + * answering with an empty map (issue #37636). + */ + @Override + public Map getIndicesStatsOrThrow() { try { final org.opensearch.client.opensearch.indices.IndicesStatsResponse response = clientProvider.getClient().indices() @@ -473,9 +501,8 @@ public Map getIndicesStats() .build()); } return result; - } catch (final Exception e) { - Logger.error(this.getClass(), "Error fetching OS indices stats: " + e.getMessage(), e); - return new HashMap<>(); + } catch (final IOException e) { + throw new DotRuntimeException("Could not read OpenSearch index stats: " + e.getMessage(), e); } } diff --git a/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java b/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java index d20015f9e5ec..cda4a4458e8e 100644 --- a/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java +++ b/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java @@ -25,6 +25,16 @@ public interface SiteSearchAPI { List listIndices(); + /** + * Same as {@link #listIndices()}, but a failure to reach the engine propagates instead of being + * answered with an empty list — for callers that must tell "no site-search indices" from "could + * not ask", such as the migration readiness report (issue #37636). Implementations whose + * {@code listIndices()} already propagates need not override it. + */ + default List listIndicesOrThrow() { + return listIndices(); + } + /** * Whether {@code indexName} exists on every engine that receives writes in the current migration * phase (Phase 0 → ES only; Phases 1/2 → ES and OpenSearch; Phase 3 → OpenSearch diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java index 8851c84b16f6..40496fca6142 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/ContentIndexMirrorReconcilerTest.java @@ -6,6 +6,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -137,8 +138,8 @@ public void workingAndLive_inSync() { // trip Mockito's UnfinishedStubbingException. final Map esStats = Map.of("working_1", present(), "live_1", present()); final Map osStats = Map.of("working_1.os", present(), "live_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 100); count(osOps, "working_1", 100); count(esOps, "live_1", 50); count(osOps, "live_1", 50); @@ -164,8 +165,8 @@ public void workingAndLive_inSync() { public void missingOsCounterpart_onWorking() { final Map esStats = Map.of("working_1", present(), "live_1", present()); final Map osStats = Map.of("live_1.os", present()); // working_1.os absent - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 100); count(esOps, "live_1", 50); count(osOps, "live_1", 50); @@ -186,8 +187,8 @@ public void missingOsCounterpart_onWorking() { public void countDrift_onLive() { final Map esStats = Map.of("working_1", present(), "live_1", present()); final Map osStats = Map.of("working_1.os", present(), "live_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 100); count(osOps, "working_1", 100); count(esOps, "live_1", 50); count(osOps, "live_1", 40); @@ -216,8 +217,8 @@ public void phase3_sourcesTheActiveNamesFromTheOpenSearchStore() { Config.setProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, "3"); final Map osStats = Map.of("working_1.os", present(), "live_1.os", present()); - when(es.getIndicesStats()).thenReturn(Map.of()); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(Map.of()); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(osOps, "working_1", 683); count(osOps, "live_1", 682); final List statuses = reconciler(null, osStore( @@ -252,8 +253,8 @@ public void phase3_divergedGenerations_reportsEachEngineOnItsOwnIndex() { Config.setProperty(IndexConfigHelper.MigrationPhase.FLAG_KEY, "3"); final Map esStats = Map.of("working_OLD", present()); final Map osStats = Map.of("working_NEW.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); when(esOps.getIndexDocumentCount(PREFIX + "working_OLD")).thenReturn(600L); when(osOps.getIndexDocumentCount(PREFIX + "working_NEW.os")).thenReturn(683L); @@ -335,8 +336,8 @@ public void nullIndicies_emptyList() { public void unsetSlot_skipped() { final Map esStats = Map.of("live_1", present()); final Map osStats = Map.of("live_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "live_1", 50); count(osOps, "live_1", 50); final List statuses = reconciler(indicies(null, PREFIX + "live_1")).statuses(); @@ -356,8 +357,8 @@ public void docCount_comesFromTheLiveCountQuery_notFromStats() { // Build the maps first: present() calls when(), which cannot run inside another when(). final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 683); count(osOps, "working_1", 15); final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null)).statuses().get(0); @@ -377,8 +378,8 @@ public void docCount_comesFromTheLiveCountQuery_notFromStats() { public void coverage_isMeasuredAgainstTheDatabase() { final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 686); count(osOps, "working_1", 21); final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null), @@ -399,8 +400,8 @@ public void coverage_isMeasuredAgainstTheDatabase() { public void coverage_absentWithoutADatabaseDenominator() { final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 686); count(osOps, "working_1", 21); final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null)).statuses().get(0); @@ -419,8 +420,8 @@ public void coverage_absentWithoutADatabaseDenominator() { public void coverage_completeMirror_isNotFlagged() { final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 686); count(osOps, "working_1", 686); final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null), @@ -440,8 +441,8 @@ public void coverage_completeMirror_isNotFlagged() { public void countQueryFailure_isReportedAsUnmeasurable() { final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(esOps, "working_1", 683); when(osOps.getIndexDocumentCount("cluster_x.working_1.os")) .thenThrow(new DotRuntimeException("OS unreachable")); @@ -466,8 +467,8 @@ public void countQueryFailure_isReportedAsUnmeasurable() { public void bothCountQueriesFailing_isNeverInSync() { final Map esStats = Map.of("working_1", present()); final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(esStats); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); when(esOps.getIndexDocumentCount("cluster_x.working_1")) .thenThrow(new DotRuntimeException("ES unreachable")); when(osOps.getIndexDocumentCount("cluster_x.working_1.os")) @@ -491,8 +492,8 @@ public void bothCountQueriesFailing_isNeverInSync() { @Test public void absentCopy_reportsNoIndexedPercentage() { final Map osStats = Map.of("working_1.os", present()); - when(es.getIndicesStats()).thenReturn(Map.of()); - when(os.getIndicesStats()).thenReturn(osStats); + when(es.getIndicesStatsOrThrow()).thenReturn(Map.of()); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); count(osOps, "working_1", 686); final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null), @@ -502,4 +503,89 @@ public void absentCopy_reportsNoIndexedPercentage() { assertNull("an absent copy has no completeness to report", working.esIndexedPercent()); assertEquals(100.0, working.osIndexedPercent(), 0.001); } + + /** + * Elasticsearch cannot be reached — the runbook's "retire the old cluster" step, or any outage. + * The report must still answer for OpenSearch: before issue #37636 the stats call threw straight + * out of the reconciler and the endpoint returned nothing but the connection error. The + * Elasticsearch side is marked unavailable with the reason — not "missing", which would prescribe + * a reindex over an unknown — and no count query is sent to an engine already known to be down. + */ + @Test + public void elasticsearchUnreachable_reportsOpenSearchSide_andMarksElasticsearchUnavailable() { + final Map osStats = Map.of("working_1.os", present(), "live_1.os", present()); + when(es.getIndicesStatsOrThrow()).thenThrow( + new DotRuntimeException("elasticsearch: Name or service not known")); + when(os.getIndicesStatsOrThrow()).thenReturn(osStats); + count(osOps, "working_1", 100); + count(osOps, "live_1", 50); + + final ContentMirrors mirrors = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).mirrors(); + + assertTrue(mirrors.unreadableReason().isEmpty()); + assertEquals(List.of(MirrorStatus.ELASTICSEARCH), + List.copyOf(mirrors.unreachableEngines().keySet())); + assertTrue(mirrors.unreachableEngines().get(MirrorStatus.ELASTICSEARCH) + .contains("Name or service not known")); + assertEquals(2, mirrors.statuses().size()); + final MirrorStatus working = mirrors.statuses().get(0); + assertEquals("working_1", working.indexName()); + assertEquals(Verdict.UNMEASURED, working.verdict()); + assertFalse(working.es().wasRead()); + assertTrue(working.es().unavailableReason().contains("Name or service not known")); + assertFalse(working.es().exists()); + assertEquals(-1, working.es().docCount()); + assertEquals("cluster_x.working_1", working.es().physicalName()); + assertTrue(working.os().wasRead()); + assertTrue(working.os().exists()); + assertEquals(100, working.os().docCount()); + assertEquals(50, mirrors.statuses().get(1).os().docCount()); + assertFalse("an unknown copy must not be reported as missing", + working.recommendation().contains("is missing")); + verify(esOps, never()).getIndexDocumentCount(anyString()); + } + + /** The same, the other way round: OpenSearch down, Elasticsearch still reported. */ + @Test + public void openSearchUnreachable_reportsElasticsearchSide_andMarksOpenSearchUnavailable() { + final Map esStats = Map.of("working_1", present(), "live_1", present()); + when(es.getIndicesStatsOrThrow()).thenReturn(esStats); + when(os.getIndicesStatsOrThrow()).thenThrow(new DotRuntimeException("Connection refused")); + count(esOps, "working_1", 100); + count(esOps, "live_1", 50); + + final ContentMirrors mirrors = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).mirrors(); + + assertEquals(List.of(MirrorStatus.OPENSEARCH), + List.copyOf(mirrors.unreachableEngines().keySet())); + final MirrorStatus working = mirrors.statuses().get(0); + assertEquals(Verdict.UNMEASURED, working.verdict()); + assertEquals(100, working.es().docCount()); + assertFalse(working.os().wasRead()); + assertEquals(-1, working.os().docCount()); + assertEquals("cluster_x.working_1.os", working.os().physicalName()); + verify(osOps, never()).getIndexDocumentCount(anyString()); + } + + /** Both engines down: the rows are still there, both sides unavailable, nothing thrown. */ + @Test + public void bothEnginesUnreachable_rowsStillReported() { + when(es.getIndicesStatsOrThrow()).thenThrow(new DotRuntimeException("es down")); + when(os.getIndicesStatsOrThrow()).thenThrow(new DotRuntimeException("os down")); + + final ContentMirrors mirrors = + reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).mirrors(); + + assertEquals(2, mirrors.unreachableEngines().size()); + assertEquals(2, mirrors.statuses().size()); + assertEquals(Verdict.UNMEASURED, mirrors.statuses().get(0).verdict()); + assertFalse(mirrors.statuses().get(0).es().wasRead()); + assertFalse(mirrors.statuses().get(0).os().wasRead()); + final String advice = mirrors.statuses().get(0).recommendation(); + assertTrue(advice, advice.startsWith("Both Elasticsearch and OpenSearch could not be reached")); + assertTrue(advice, advice.contains("Elasticsearch: es down")); + assertTrue(advice, advice.contains("OpenSearch: os down")); + } } diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java index a70d1be96150..2f4d4e96eafb 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/MigrationReadinessServiceTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -11,8 +12,10 @@ import com.dotcms.content.index.migration.ContentIndexMirrorReconciler.ContentMirrors; import com.dotcms.content.index.migration.MirrorStatus.IndexKind; import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.content.index.migration.SiteSearchMirrorReconciler.SiteSearchMirrors; import com.dotmarketing.util.Config; import java.util.List; +import java.util.Map; import java.util.Optional; import org.junit.After; import org.junit.Before; @@ -43,6 +46,10 @@ public void setUp() { // Default to a healthy WORKING/LIVE pair so the mandatory-content precondition is satisfied; // tests that exercise missing content override this explicitly. stubContent(healthyContentPair()); + // The service reads mirrors(); most cases only care about the rows, so derive it from + // whatever statuses() is stubbed to, with both engines reachable. + when(siteSearch.mirrors()).thenAnswer( + inv -> new SiteSearchMirrors(siteSearch.statuses(), Map.of())); service = new MigrationReadinessService(siteSearch, content, () -> "cluster_x"); } @@ -536,4 +543,165 @@ private static MirrorStatus cc(final IndexKind kind, final String name, final bo MirrorStatus.verdictFor(esExists, osExists, esCount, osCount), "advice", databaseDocCount); } + + // ---- One engine unreachable (issue #37636) -------------------------------------------------- + + private static final String ES_DOWN = "elasticsearch: Name or service not known"; + private static final String OS_DOWN = "opensearch: Connection refused"; + + /** Stubs the content pair with one engine that could not be read. */ + private void stubContentUnreachable(final String engine, final String reason, + final List statuses) { + when(content.mirrors()).thenReturn( + new ContentMirrors(statuses, Optional.empty(), Map.of(engine, reason))); + } + + /** A content row whose Elasticsearch side could not be read. */ + private static MirrorStatus esUnreachable(final IndexKind kind, final String name, + final long osCount) { + return new MirrorStatus(name, kind, + MirrorStatus.EngineCopy.unavailable("cluster_x." + name, null, ES_DOWN), + new MirrorStatus.EngineCopy(true, osCount, "cluster_x." + name + ".os"), + Verdict.UNMEASURED, "advice", null); + } + + /** A content row whose OpenSearch side could not be read. */ + private static MirrorStatus osUnreachable(final IndexKind kind, final String name, + final long esCount) { + return new MirrorStatus(name, kind, + new MirrorStatus.EngineCopy(true, esCount, "cluster_x." + name), + MirrorStatus.EngineCopy.unavailable("cluster_x." + name + ".os", null, OS_DOWN), + Verdict.UNMEASURED, "advice", null); + } + + private static boolean prescribesAReindex(final List blockers) { + return blockers.stream().anyMatch(b -> b.contains("has no") || b.contains("No active")); + } + + /** + * Dual-write with Elasticsearch down: nothing can be concluded about either direction, so both + * verdicts are false — with ONE reason that names the engine and says why, instead of per-slot + * "no Elasticsearch copy, reindex" messages that would prescribe a reindex over an unknown. + */ + @Test + public void dualWrite_elasticsearchUnreachable_blocksBothWays_withoutPrescribingAReindex() { + setPhase(PHASE_1); + stubContentUnreachable(MirrorStatus.ELASTICSEARCH, ES_DOWN, List.of( + esUnreachable(IndexKind.CONTENT_WORKING, "working_1", 10), + esUnreachable(IndexKind.CONTENT_LIVE, "live_1", 5))); + + final MigrationReadiness r = service.evaluate(); + + assertEquals(Map.of(MirrorStatus.ELASTICSEARCH, ES_DOWN), r.unreachableEngines()); + assertEquals(2, r.content().size()); + assertFalse(r.verdict().safeToAdvance()); + assertFalse(r.verdict().safeToRollback()); + assertEquals(1, r.verdict().blockers().size()); + final String blocker = r.verdict().blockers().get(0); + assertTrue(blocker, blocker.contains("Elasticsearch could not be reached")); + assertTrue(blocker, blocker.contains(ES_DOWN)); + assertFalse(prescribesAReindex(r.verdict().blockers())); + } + + /** Dual-write with OpenSearch down: the same, naming OpenSearch. */ + @Test + public void dualWrite_openSearchUnreachable_blocksBothWays() { + setPhase(PHASE_2); + stubContentUnreachable(MirrorStatus.OPENSEARCH, OS_DOWN, List.of( + osUnreachable(IndexKind.CONTENT_WORKING, "working_1", 10), + osUnreachable(IndexKind.CONTENT_LIVE, "live_1", 5))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertFalse(r.verdict().safeToRollback()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("OpenSearch could not be reached")); + } + + /** + * Phase 0 with OpenSearch down. A missing OpenSearch copy is the expected Phase 0 state and is not + * a blocker — but an OpenSearch that cannot be reached is: advancing starts dual-write into it. + */ + @Test + public void phase0_openSearchUnreachable_blocksAdvance() { + setPhase(PHASE_0); + stubContentUnreachable(MirrorStatus.OPENSEARCH, OS_DOWN, List.of( + osUnreachable(IndexKind.CONTENT_WORKING, "working_1", 10), + osUnreachable(IndexKind.CONTENT_LIVE, "live_1", 5))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("OpenSearch could not be reached")); + } + + /** + * Phase 3 with Elasticsearch retired — the runbook's end state. Nothing in Phase 3 depends on + * Elasticsearch, so the report stays usable and not blocked; but whether a downgrade is safe cannot + * be judged, and the summary says exactly that rather than warning that OpenSearch is "ahead", + * which nobody measured. + */ + @Test + public void phase3_elasticsearchRetired_stillReports_butCannotJudgeRollback() { + setPhase(PHASE_3); + stubContentUnreachable(MirrorStatus.ELASTICSEARCH, ES_DOWN, List.of( + esUnreachable(IndexKind.CONTENT_WORKING, "working_1", 10), + esUnreachable(IndexKind.CONTENT_LIVE, "live_1", 5))); + + final MigrationReadiness r = service.evaluate(); + + assertEquals(Map.of(MirrorStatus.ELASTICSEARCH, ES_DOWN), r.unreachableEngines()); + assertTrue(r.verdict().blockers().isEmpty()); + assertTrue(r.verdict().safeToAdvance()); + assertEquals(0, r.verdict().outOfSyncCount()); + assertFalse(r.verdict().safeToRollback()); + final String summary = r.verdict().summary(); + assertTrue(summary, summary.contains("Elasticsearch could not be reached")); + assertFalse(summary, summary.contains("OpenSearch holds content Elasticsearch does not")); + } + + /** Phase 3 with OpenSearch down: the active content store itself is unknown — blocked. */ + @Test + public void phase3_openSearchUnreachable_blocks() { + setPhase(PHASE_3); + stubContentUnreachable(MirrorStatus.OPENSEARCH, OS_DOWN, List.of( + osUnreachable(IndexKind.CONTENT_WORKING, "working_1", 10), + osUnreachable(IndexKind.CONTENT_LIVE, "live_1", 5))); + + final MigrationReadiness r = service.evaluate(); + + assertFalse(r.verdict().safeToAdvance()); + assertFalse(r.verdict().safeToRollback()); + assertEquals(1, r.verdict().blockers().size()); + assertTrue(r.verdict().blockers().get(0).contains("OpenSearch could not be reached")); + assertFalse(prescribesAReindex(r.verdict().blockers())); + } + + /** + * An engine that failed only on the Site Search side is still an unreachable engine: it reaches + * the report and blocks, even when the content half read fine. + */ + @Test + public void siteSearchSideUnreachable_isReportedAndBlocks() { + setPhase(PHASE_2); + // doReturn, not when(): calling mirrors() to re-stub it would fire setUp's answer mid-stubbing. + doReturn(new SiteSearchMirrors(List.of(), Map.of(MirrorStatus.OPENSEARCH, OS_DOWN))) + .when(siteSearch).mirrors(); + + final MigrationReadiness r = service.evaluate(); + + assertEquals(Map.of(MirrorStatus.OPENSEARCH, OS_DOWN), r.unreachableEngines()); + assertFalse(r.verdict().safeToAdvance()); + assertFalse(r.verdict().safeToRollback()); + } + + /** Both engines reachable: nothing reported as unreachable. */ + @Test + public void bothReachable_noUnreachableEngines() { + setPhase(PHASE_1); + + assertTrue(service.evaluate().unreachableEngines().isEmpty()); + } } diff --git a/dotCMS/src/test/java/com/dotcms/content/index/migration/SiteSearchMirrorReconcilerTest.java b/dotCMS/src/test/java/com/dotcms/content/index/migration/SiteSearchMirrorReconcilerTest.java index 76418d7d37fe..ff1e2b66873c 100644 --- a/dotCMS/src/test/java/com/dotcms/content/index/migration/SiteSearchMirrorReconcilerTest.java +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/SiteSearchMirrorReconcilerTest.java @@ -6,12 +6,15 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.dotcms.UnitTestBase; import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.content.index.migration.SiteSearchMirrorReconciler.SiteSearchMirrors; +import com.dotmarketing.exception.DotRuntimeException; import com.dotmarketing.sitesearch.business.SiteSearchAPI; import java.util.List; import java.util.Map; @@ -39,8 +42,8 @@ public class SiteSearchMirrorReconcilerTest extends UnitTestBase { public void setUp() { es = mock(SiteSearchAPI.class); os = mock(SiteSearchAPI.class); - when(es.listIndices()).thenReturn(List.of(INDEX)); - when(os.listIndices()).thenReturn(List.of(INDEX)); + when(es.listIndicesOrThrow()).thenReturn(List.of(INDEX)); + when(os.listIndicesOrThrow()).thenReturn(List.of(INDEX)); when(es.existsOnAllWriteEngines(anyString())).thenReturn(true); when(os.existsOnAllWriteEngines(anyString())).thenReturn(true); when(es.documentCount(anyString())).thenReturn(10L); @@ -130,12 +133,54 @@ public void aliasStartingWithThePrefix_isNotFlagged() { /** The alias lookup is one call per engine for the whole set, not one per index. */ @Test public void aliasLookup_runsOncePerEngine() { - when(es.listIndices()).thenReturn(List.of(INDEX, "sitesearch_20260811090000")); - when(os.listIndices()).thenReturn(List.of(INDEX, "sitesearch_20260811090000")); + when(es.listIndicesOrThrow()).thenReturn(List.of(INDEX, "sitesearch_20260811090000")); + when(os.listIndicesOrThrow()).thenReturn(List.of(INDEX, "sitesearch_20260811090000")); assertEquals(2, reconciler().statuses().size()); verify(es, times(1)).getAliasToIndexMap(); verify(os, times(1)).getAliasToIndexMap(); } -} \ No newline at end of file + + /** + * Elasticsearch cannot be reached: the index list comes from OpenSearch alone, and the + * Elasticsearch side of each row is marked unavailable rather than missing — "missing" would tell + * the operator to re-crawl an index that may be perfectly fine (issue #37636). Nothing else is + * asked of the engine already known to be down. + */ + @Test + public void elasticsearchUnreachable_reportsOpenSearchIndices_andMarksElasticsearchUnavailable() { + when(es.listIndicesOrThrow()).thenThrow(new DotRuntimeException("elasticsearch: Name or service not known")); + + final SiteSearchMirrors mirrors = reconciler().mirrors(); + + assertEquals(List.of(MirrorStatus.ELASTICSEARCH), + List.copyOf(mirrors.unreachableEngines().keySet())); + assertEquals(1, mirrors.statuses().size()); + final MirrorStatus status = mirrors.statuses().get(0); + assertEquals(INDEX, status.indexName()); + assertEquals(Verdict.UNMEASURED, status.verdict()); + assertFalse(status.es().wasRead()); + assertEquals(-1, status.es().docCount()); + assertEquals(10, status.os().docCount()); + assertFalse(status.recommendation().contains("is missing")); + verify(es, never()).documentCount(anyString()); + verify(es, never()).getAliasToIndexMap(); + } + + /** The same, the other way round. */ + @Test + public void openSearchUnreachable_reportsElasticsearchIndices_andMarksOpenSearchUnavailable() { + when(os.listIndicesOrThrow()).thenThrow(new DotRuntimeException("Connection refused")); + + final SiteSearchMirrors mirrors = reconciler().mirrors(); + + assertEquals(List.of(MirrorStatus.OPENSEARCH), + List.copyOf(mirrors.unreachableEngines().keySet())); + final MirrorStatus status = mirrors.statuses().get(0); + assertEquals(Verdict.UNMEASURED, status.verdict()); + assertEquals(10, status.es().docCount()); + assertFalse(status.os().wasRead()); + verify(os, never()).documentCount(anyString()); + } +}