Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -156,12 +157,25 @@ public OSSiteSearchAPI() {

@Override
public List<String> listIndices() {
return siteSearchIndices(indexApi.listIndices());
Comment thread
fabrizzio-dotCMS marked this conversation as resolved.
}

/**
* Same listing, but a failure to reach OpenSearch propagates instead of reading as "no indices"
* (issue #37636).
*/
@Override
public List<String> listIndicesOrThrow() {
return siteSearchIndices(indexApi.listIndicesOrThrow());
}

private List<String> siteSearchIndices(final Set<String> 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<String> indices = indexApi.listIndices().stream()
final List<String> indices = allIndices.stream()
.filter(IndexType.SITE_SEARCH::is)
.map(IndexTag::strip)
.distinct()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});

Expand Down
24 changes: 24 additions & 0 deletions dotCMS/src/main/java/com/dotcms/content/index/IndexAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ public String getStatus() {
*/
Map<String, IndexStats> 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<String, IndexStats> getIndicesStatsOrThrow() {
return getIndicesStats();
}

/**
* Flushes field and filter caches for the specified indices.
* This operation can take up to a minute to complete.
Expand Down Expand Up @@ -116,6 +129,17 @@ public String getStatus() {
*/
Set<String> 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<String> listIndicesOrThrow() {
return listIndices();
}

/**
* Checks if the specified index is closed.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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).</p>
*
* <p><strong>Existence</strong> comes from each engine leaf's {@code getIndicesStats()} — one call per
* <p><strong>Existence</strong> 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 <em>cluster-stripped</em> name (Elasticsearch un-tagged, OpenSearch carrying
* {@code .os}), so each raw name is stripped of the cluster prefix and then, for the OpenSearch lookup,
Expand Down Expand Up @@ -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.</p>
*
* @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
* <p>{@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).</p>
*
* @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<MirrorStatus> statuses, Optional<String> unreadableReason) {}
public record ContentMirrors(List<MirrorStatus> statuses, Optional<String> unreadableReason,
Map<String, String> unreachableEngines) {

/** Both engines were read (or never needed to be). */
public ContentMirrors(final List<MirrorStatus> statuses,
final Optional<String> unreadableReason) {
this(statuses, unreadableReason, Map.of());
}
}

/**
* Per-index mirror status for the active working and live content indices.
Expand All @@ -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<String, String> unreachable = new LinkedHashMap<>();
final Map<String, IndexStats> esStats =
statsOrUnreachable(esImpl, MirrorStatus.ELASTICSEARCH, unreachable);
final Map<String, IndexStats> 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<String, IndexStats> statsOrUnreachable(final IndexAPI engine,
final String engineName, final Map<String, String> 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<MirrorStatus> statusesFor(final PointerLookup pointers) {
final Map<String, IndexStats> esStats = esImpl.getIndicesStats();
final Map<String, IndexStats> osStats = osImpl.getIndicesStats();
private List<MirrorStatus> statusesFor(final PointerLookup pointers,
final Map<String, IndexStats> esStats, final Map<String, IndexStats> osStats,
final Map<String, String> unreachable) {
final DatabaseCounts dbCounts = databaseCountsSupplier.get();
final List<MirrorStatus> 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;
}
Expand Down Expand Up @@ -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<MirrorStatus> out, final IndexKind kind,
final SlotPointers pointers, final Map<String, IndexStats> esStats,
final Map<String, IndexStats> osStats, final Long databaseDocCount) {
final Map<String, IndexStats> osStats, final Map<String, String> unreachable,
final Long databaseDocCount) {
if (pointers.isUnset()) {
return;
}
Expand All @@ -264,9 +312,9 @@ private void addStatus(final List<MirrorStatus> 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
Expand All @@ -275,14 +323,23 @@ private void addStatus(final List<MirrorStatus> 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));
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.dotcms.content.index.migration;

import com.fasterxml.jackson.annotation.JsonInclude;
import java.util.List;
import java.util.Map;

Expand All @@ -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<String, MirrorStatus> content,
List<MirrorStatus> siteSearch,
Verdict verdict) {
Verdict verdict,
@JsonInclude(JsonInclude.Include.NON_EMPTY) Map<String, String> unreachableEngines) {

/**
* @param current the current phase ordinal (0–3)
Expand Down
Loading
Loading