diff --git a/docs/backend/OPENSEARCH_MIGRATION.md b/docs/backend/OPENSEARCH_MIGRATION.md index 71e8795219f6..d577fc9fa4c1 100644 --- a/docs/backend/OPENSEARCH_MIGRATION.md +++ b/docs/backend/OPENSEARCH_MIGRATION.md @@ -295,6 +295,40 @@ can preview `.os` (and the ES/OS twin as distinct rows) while normal users keep view — is the one place the two UIs should converge; it does **not** require changing the internal handle model, only the display sink. +##### Two alias views: searching vs. managing (issue #36983) + +`SiteSearchAPI` exposes the alias map twice, and picking the wrong one is a bug: + +| Method | Resolves against | Use it for | +|---|---|---| +| `getAliasToIndexMap()` | the **read provider only** (ES in Phases 0/1, OS in Phases 2/3) | **searching** — resolve an alias against the engine that will actually serve the query | +| `getAliasToIndexMapAllEngines()` | the **same provider set as `listIndices()`** (union in Phases 1/2) | **managing / displaying** — portlet columns, index selectors, choosing an index to crawl | + +The reason there are two: **`listIndices()` is a union of both engines in the dual-write phases, +while alias resolution is single-engine.** Any index that lives only on the engine the current phase +does *not* read from therefore appears in the list with a blank alias. Two mirror-image symptoms of +the same defect: + +- **Phase 2 + an index created in Phase 0** (Elasticsearch only) — reads come from OpenSearch, alias + invisible. +- **Phase 1 + an index created in Phase 3** (OpenSearch only; typical after a downgrade 3 → 2 → 1) — + reads come from Elasticsearch, alias invisible. + +`getAliasToIndexMapAllEngines()` merges over the write providers and applies the **read provider +last**, so on a mirror desync (one alias resolving to different indices per engine) the management +view agrees with what a search would hit. In the single-provider phases (0 and 3) there is nothing to +merge and the idle engine is not consulted. + +Callers on the management side: `site_search_index_stats.jsp` (Indices tab), `site_search_job_schedule.jsp` +(crawl index selector), `test_site_search.jsp` (Search tab selector) and `SiteSearchJobImpl` (the +crawl's alias resolution — an alias invisible there makes the crawl treat an existing index as new +and drop its alias). Everything on the search path keeps the single-engine method. + +**A phase change never builds counterparts retroactively.** An index created in a single-provider +phase exists on that engine only until a crawl runs in a dual-write phase. Downgrading past that +point leaves it listed but unsearchable (its content lives on the engine that no longer serves +reads) — visible in the readiness report as `MISSING_COUNTERPART`; the fix is always a re-crawl. + #### Site Search mirror reconciliation (write path) — self-heal on crawl The logical-handle model above makes *reads* correct, but a Site Search index can still end up @@ -357,6 +391,62 @@ ensure every Site Search index has been crawled at least once so its OS counterp sync. The migration-readiness endpoint below is what tells the operator *which* indices still need that crawl, before they change the phase. +##### The crawl inherits the content index — reindex first, crawl second + +A Site Search crawl does **not** read the database. It builds its bundle from a **search over the +content index**: + +```java +// FileAssetBundler:205 — same shape in HTMLPageAsContentBundler and URLMapBundler +searchResults.addAll(this.conAPI.searchIndex(luceneQuery + " +live:true", ...)); +``` + +That search is phase-routed, so in Phases 2/3 it is served by **OpenSearch**. If the OpenSearch +*content* mirror has not been rebuilt by a full reindex, the crawl simply cannot see the content that +is missing from it — and writes a Site Search index containing only what it found. Observed on a +Phase-3 crawl: the content index held 685 live documents on Elasticsearch and 21 on OpenSearch (never +reindexed), and the resulting Site Search index came out with **14 documents** instead of ~443. The +crawl answered its query correctly; the corpus it queried was 3% complete. + +This is worse than the read-time cliff above, in three ways: + +1. **The crawl reports success.** Nothing warns that the input corpus was nearly empty — the counts it + logs are of what it bundled, so they look internally consistent. +2. **The bundlers swallow search failures at `Logger.debug`** (`FileAssetBundler:206-208, 213-215`). + Even a hard search error surfaces as nothing more than a smaller bundle. +3. **The damage outlives its cause.** Reindexing the content store afterwards fixes the content mirror + but does *not* repair the Site Search index that was already built from the empty one — it keeps + its 14 documents until it is crawled again. Nor will the readiness report flag it: the index exists + on OpenSearch, which in Phase 3 is exactly the expected topology, so the row reads healthy. The + defect is *inside* the index, not in its shape. + +**Ordering rule:** a Site Search crawl is only meaningful once the content index of the phase's +**read** engine is complete. In Phases 2/3 that means **full content reindex first, Site Search crawl +second** — the reverse order silently produces a truncated index that looks fine everywhere. + +**The crawl warns when it is about to do this.** `SiteSearchJobImpl` checks how much of the content +the index it is about to read actually holds — the read engine's copy measured against the database, +the same `osIndexedPercent` / `esIndexedPercent` the readiness endpoint reports — and logs a `WARN` naming the index, the engine, the +percentage, and the fact that reindexing afterwards will not repair the result: + +``` +Site Search crawl starting against an INCOMPLETE content index: 'working_20260811191012' on +OpenSearch holds 3.06% of the 686 contentlets the database has. A crawl builds its corpus by +querying that index, so it can only index what it finds there — this crawl will produce a partial +Site Search index, and reindexing the content later will NOT repair it (it must be crawled again). +Run a full reindex first. +``` + +It is **advisory only**: it never stops the crawl, and any failure to measure is swallowed — a +diagnostic must not be able to break indexing. Only the engine the phase actually reads from is +checked, so an incomplete OpenSearch mirror stays silent in Phases 0/1 where the crawl queries a +complete Elasticsearch. Threshold: `SITE_SEARCH_CRAWL_MIN_CONTENT_INDEXED_PERCENT` (default `95`; +`0` disables the check). + +Note the readiness endpoint does catch the precondition: an unreconciled content mirror shows as +`COUNT_DRIFT` on `WORKING`/`LIVE` with `safeToAdvance: false`. It only *reports*, though — nothing +stops a promotion or a crawl from proceeding anyway. + #### Migration-readiness endpoint (pre-phase-change advisory) `GET /api/v1/index/migration/readiness` is an internal, read-only report a support technician runs @@ -380,11 +470,69 @@ through the write-path gate above. (original) — negative = behind, positive = ahead, `null` when a count is unknown — with verdict `IN_SYNC` / `MISSING_COUNTERPART` / `COUNT_DRIFT`. The top level also carries the `clusterId` embedded in every physical name. The response is the model itself (no `ResponseEntityView` envelope). -- **Stateless, from live counts.** Every field is derived at request time. Counts are **exact** — the - Site Search half uses `SiteSearchAPI.documentCount` and the content half reads each engine leaf's - `getIndicesStats()` (index `_stats` `primaries.docs.count`), never a search total (which the ES/OS +- **Site Search entries also carry the `alias`, per engine.** `es.alias` / `os.alias` hold the alias + that engine has attached to the index (omitted when there is none; never present on content rows, + which are addressed by name only). Per engine on purpose: an index can hold its alias on one side + and not the other — e.g. created before dual-write started, counterpart built later — and that + asymmetry is what the operator needs to see. It is what makes the report usable at all, since a + site-search index is known by its alias, never by its `sitesearch__` name. One + alias lookup per engine covers the whole set, not one per index. When an alias is itself shaped + like an index name, `recommendation` appends a NOTE: that is the fingerprint of the crawl overwrite + fixed in issue #36983 — the fix stops new occurrences but cannot restore an alias already lost, so + this is the only way to find the indices that still need theirs restored. It never changes + `verdict`: the verdict measures data integrity (existence + counts), while a damaged alias costs no + data and must not block a phase change. +- **Stateless, from live counts.** Every field is derived at request time. Counts are **exact and + current**: both halves issue a real count query per index — Site Search through + `SiteSearchAPI.documentCount`, the content half through + `ContentletIndexOperations.getIndexDocumentCount`. Neither uses a search hit total (which the ES/OS clients cap at 10,000 and would hide drift on large indices). Both reconcilers query the two engine leaves directly, not the phase-aware router, so the report shows both sides in every phase. +- **Why the count is a query and not `_stats` `docs.count`.** The content half still calls + `getIndicesStats()` — but only to decide **existence**, one call per engine covering the whole index + set, so both slots are settled from a single snapshot. The count itself must not come from there: + `docs.count` is a per-shard counter that only advances when the shard refreshes, so it trails a + just-written document by seconds. During that window the document is already searchable while the + report still shows the previous number — and a support technician checking whether a publish reached + OpenSearch reads that as a **lost write**. This endpoint is the source of truth for exactly that + question, so it must never report a number the engine can already contradict (issue #36983). +- **Content rows also say how much of the database's content each engine actually holds.** `databaseDocCount` is how many documents + the index should hold per `contentlet_version_info` (keyed by `identifier, lang, variant_id` — the + same unit as an index document), and `esIndexedPercent` / `osIndexedPercent` are each engine + measured against it. This is the only signal in the report that does not come from a search engine, + and that is the point: **`driftPercent` compares the two engines against each other, which stops + being an answer once one of them is the only one left.** In Phase 3 a mirror that was never rebuilt + has nothing to be diffed against and reads unremarkably, while `osIndexedPercent` still says `3.06`. When a + copy is materially incomplete the `recommendation` names it and spells out the fallout — including + that a Site Search crawl builds its corpus from a query against this index. It never changes the + `verdict`, which states a different fact (the ES↔OS relationship). + + **The denominator is an exact count.** `SELECT COUNT(*) AS working, COUNT(live_inode) AS live FROM + contentlet_version_info` — one row per `(identifier, lang, variant_id)`, the same unit as an index + document, so a complete index reads exactly `100.0` (verified against a live install: 686/685, + matching the index counts exactly). PostgreSQL runs it as a `Parallel Seq Scan` — `COUNT(live_inode)` + needs the column, so the heap is read — measured at **15 ms / 171k rows, 21 ms / 394k, 22 ms / 453k**, + roughly linear at ~50 ns per row on a warm cache. It runs on an admin-only endpoint on demand and once + per crawl, never on a write path. Kept as one statement on purpose: splitting it lets `COUNT(*)` alone + use a `Parallel Index Only Scan` (17 ms), but the live half stays a sequential scan regardless (nearly + every row has a live version, so the index buys nothing), and the two together measured 41 ms against + 28 ms combined. Not the `pg_class.reltuples` estimate on purpose: that drifts a few points + between `ANALYZE` runs and surfaced as an indexed percentage slightly over 100%, which reads as a defect. With + exact counts, above 100% means the index holds documents the database no longer has — orphans from a + delete that never propagated, worth looking at rather than rounding away. Site Search rows have no + such denominator (their corpus is crawled pages and files), so the fields are absent there. + +- **What a count still cannot tell you.** A number that does not move is not proof that nothing was + written: the document id is `identifier_languageId_variant`, so re-publishing content already present + in that index is an **update**, and the total stays put. And in a dual-write phase the OpenSearch copy + only ever receives what changes *from that point on* — a mirror sitting at 15 of 683 documents is the + expected state until a full reindex, so a `+1` there is easy to misread as "nothing happened". To + settle it for one specific write, ask for the **document**: + + ```bash + curl -s "http://:9200/.os/_doc/__DEFAULT" + # "found": true with the modDate of your edit ⇒ the dual-write landed + ``` - **`safeToRollback` needs no history.** A downgrade routes reads back to Elasticsearch, so it is unsafe when any index's ES copy is behind its OpenSearch counterpart (`esDocCount < osDocCount`, or the ES copy missing) — that delta, typically content written while OpenSearch served reads, would be @@ -402,6 +550,140 @@ Because this endpoint is the source of truth for migration/QA, the index portlet `.os` indices by role: `MigrationIndexVisibility` is now purely phase-based (hidden in Phases 0/1/2, shown in Phase 3, for everyone). The role key is retained only to gate this endpoint. +##### How to read the readiness report + +**Access — both conditions, or 403.** The caller must be a **CMS administrator** *and* hold the +migration support role. The role key comes from `OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY` (default +`os_migration_qa`); the check is `MigrationReadinessResource.isMigrationSupportUser`. A plain admin +without the role gets a 403, and so does a role holder who is not an admin — deliberate, so a regular +user never learns a migration is running. The endpoint is `@Hidden`, so it is absent from +`openapi.yaml` and from the API playground: it will not show up by browsing, only by knowing the URL. + +```bash +# Backend session or basic auth; both the admin role and the support role are required. +curl -u admin@dotcms.com:admin http://localhost:8080/api/v1/index/migration/readiness | jq +``` + +If it returns 403, grant the `os_migration_qa` role to the admin user (Roles & Permissions), or point +`OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY` at a role they already hold. There is no envelope: the JSON +**is** the report. + +**Read it top-down, in this order:** + +1. **`phase`** — `current`/`name`, plus `readEngine`, `writeEngines` and `dualWrite`. Everything below + is relative to this: which engine answers searches *right now*, and which ones receive writes. +2. **`verdict.safeToAdvance` / `verdict.safeToRollback`** — the go/no-go pair. They answer different + questions and are not opposites: *advance* is blocked when the OpenSearch mirror is behind + (promoting would lose data on the OpenSearch-only phase); *rollback* is blocked when OpenSearch is + **ahead** (downgrading would hide the delta until a reindex). Both can be `false` at once. +3. **`verdict.summary` + `verdict.blockers`** — the sentence to paste into a ticket, then the per-index + list of what to fix. An empty `blockers` with `safeToAdvance: false` cannot happen; if `blockers` is + non-empty, each entry names the index and the action. +4. **`content` (keyed `WORKING`/`LIVE`) and `siteSearch` (list)** — the evidence behind the verdict. + +**Per-index row.** `es` and `os` each carry `{exists, docCount, physicalName}` — plus `alias` on Site +Search rows. Then: + +| Field | How to read it | +|---|---| +| `verdict` | `IN_SYNC` · `MISSING_COUNTERPART` (one engine lacks the index) · `COUNT_DRIFT` (both hold it, different counts) | +| `driftPercent` | `(OS − ES) / ES × 100`, rounded to 2 decimals. `0.0` in sync · negative = mirror **behind** (blocks *advance*) · positive = mirror **ahead** (blocks *rollback*) · `-100.0` mirror empty/absent · `+100.0` the original is empty but the mirror holds data · `null` a count could not be measured | +| `databaseDocCount` + `esIndexedPercent` / `osIndexedPercent` | Content rows only. What percentage of the database's content each engine actually holds — measured **against the database**, not against the other engine — the one completeness signal that still works in Phase 3, where there is nothing left to diff. `100.0` = complete; `3.06` = the mirror was never rebuilt. Absent for Site Search and when a count could not be measured | +| `docCount: -1` | The count could **not** be measured. Never read it as "zero" — the verdict treats it as out of sync on purpose | +| `physicalName` | The exact name on that server (cluster-prefixed; `.os`-tagged on OpenSearch) — copy/paste it into `_cat/indices` to verify by hand | +| `recommendation` | The concrete action (re-crawl / reindex). A trailing `NOTE:` flags an alias that is really an index name (see above) | + +**Worked example — the downgrade case.** After going 3 → 2 → 1, a Site Search index created by a +crawl while in Phase 3 exists **only** on OpenSearch: + +```json +{ "indexName": "sitesearch_20260811155758_6c1f7101-…", + "es": { "exists": false, "docCount": 0, "physicalName": "cluster_x.sitesearch_20260811155758_6c1f7101-…" }, + "os": { "exists": true, "docCount": 412, "physicalName": "cluster_x.sitesearch_20260811155758_6c1f7101-….os", + "alias": "sitesearch-ph-3" }, + "driftPercent": 100.0, "verdict": "MISSING_COUNTERPART" } +``` + +(A Site Search row, so it carries no `databaseDocCount` / `*IndexedPercent`: those exist only for the +content indices, whose denominator is the database.) + +Read as: the index and its alias are intact on OpenSearch, but in Phase 1 reads come from +Elasticsearch, where it does not exist — so **its content is unsearchable until it is re-crawled**, +and `safeToRollback` is `false` because OpenSearch holds documents Elasticsearch does not. A phase +change never builds counterparts retroactively; only a crawl (or reindex, for content) does. + +Note this is exactly the information the *portlet* could not show before issue #36983: the index list +is a union of both engines while alias resolution was single-engine, so that row rendered with a blank +Alias. The endpoint never had that blind spot — it queries both engine leaves directly, in every +phase — which is why it stays the source of truth even when a portlet column looks empty. + +##### Worked example — activating a pre-migration backup content index + +dotCMS lets an administrator activate an **old inactive index** (Maintenance → Index → *Make Default*, +or `PUT /api/es/activateindex/…`) to roll back to a previous reindex. If that index **predates the +migration**, it never went through the OpenSearch create fan-out, so it has **no OpenSearch +counterpart** — and activation does not build one. + +**What the code actually does.** `ContentletIndexAPIImpl.activateIndex` repoints *both* stores by pure +name transformation: the OpenSearch pointer is set to `operationsOS.toPhysicalName(name)` = +`..os`, with **no `indexExists` check, no create and no guard** (delete has +`assertIndexNotActive`; activate has no equivalent). The OpenSearch store now names an index that has +never existed. In Phases 1/2 the shadow writes to it are best-effort and swallowed, so nothing +complains. + +**Why it is dangerous rather than merely wrong:** + +| Phase | What you see | +|---|---| +| 1 | Nothing. Silent divergence — writes to the OpenSearch counterpart go nowhere | +| 2 | Still works: the Phase-2 read fallback drops back to Elasticsearch, but logs an `ERROR` per read — the early-warning signal | +| 3 | No fallback exists. The OpenSearch pointer names an index that was never created → empty results or an exception, which reads to the customer as **lost content** | + +**What the readiness endpoint says — and when it can say it.** Once the backup is activated it *is* +the `WORKING`/`LIVE` pointer, so the very next call reports it: + +```json +"content": { + "WORKING": { + "indexName": "working_20251114093012", + "es": { "exists": true, "docCount": 148230, "physicalName": "cluster_x.working_20251114093012" }, + "os": { "exists": false, "docCount": 0, "physicalName": "cluster_x.working_20251114093012.os" }, + "databaseDocCount": 148230, + "esIndexedPercent": 100.0, + "osIndexedPercent": 0.0, + "driftPercent": -100.0, + "verdict": "MISSING_COUNTERPART", + "recommendation": "The OpenSearch copy of content index 'working_20251114093012' is missing. Run a full reindex to rebuild it before promoting to the OpenSearch-only phase." + } +} +``` + +In Phases 1/2 this also flips `verdict.safeToAdvance` to `false` and names the index in +`verdict.blockers` — the promotion gate does its job. **The fix is a full reindex**: that is the only +path that fans out through the router and materializes the OpenSearch copy (a phase change never +does, and neither does activation). + +**Three traps worth knowing before relying on this:** + +1. **You cannot pre-check a backup.** The content half of the report covers only the *active* + working/live pair, so a divergent backup is invisible while it sits inactive. Sequence: activate → + call readiness → reindex if it reports `MISSING_COUNTERPART` → only then change phase. +2. **In Phase 3 the verdict does not protect you.** `safeToAdvance` is forced `true` there (there is no + phase beyond 3), so a backup activated *while already in Phase 3* still reads green at the top + level. Read the per-index rows and `outOfSyncCount`, never the boolean alone — and note this is + precisely the phase where the failure is immediate and customer-visible. +3. **The endpoint reports, it never repairs.** It will not block the activation, and re-running it + changes nothing on its own. + +Note `osIndexedPercent: 0.0` next to `esIndexedPercent: 100.0`: that pair is the one part of this row +that keeps its meaning in Phase 3, where there is no Elasticsearch side left and `driftPercent` has +nothing to compare. + +The durable fix — reconcile-on-activate, rebuilding the counterpart asynchronously through the +existing reindex machinery (a synchronous copy of a large index is not viable, and a naive +point-in-time copy would lose concurrent writes) — is **not implemented**. Until it is, the operational +rule stands: after activating any pre-migration index, run a full reindex before touching the phase. + #### Tag manipulation is the sole responsibility of `IndexTag` All read/write of the vendor marker on an index name MUST go through the `IndexTag` enum. diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java index fb8e14211654..41c63d663a2c 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/ESSiteSearchAPI.java @@ -46,6 +46,7 @@ import java.util.Date; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.stream.Collectors; import io.vavr.control.Try; @@ -370,9 +371,16 @@ public SiteSearchResults search(String indexName, String query, int offset, int * @return * @throws DotDataException */ + @Override + public Optional defaultIndexName() throws DotDataException { + return Optional.ofNullable(indiciesAPI.loadIndicies().getSiteSearch()); + } + @Override public boolean isDefaultIndex(final String indexName) throws DotDataException { - return indexName.equals(indiciesAPI.loadIndicies().getSiteSearch()); + // Defined in terms of defaultIndexName so "which index is the default" has one definition + // per engine (issue #36983). + return indexName != null && defaultIndexName().filter(indexName::equals).isPresent(); } @Override 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 253d1ef17eca..e31afa724f83 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 @@ -519,9 +519,16 @@ public Map getFacets(String indexName, String query) throws // Default index activation / inspection // ========================================================================= + @Override + public Optional defaultIndexName() { + return defaultSiteSearchIndex(); + } + @Override public boolean isDefaultIndex(final String indexName) throws DotDataException { - return indexName != null && indexName.equals(defaultSiteSearchIndex().orElse(null)); + // Defined in terms of defaultIndexName so "which index is the default" has one definition + // per engine (issue #36983). + return indexName != null && defaultIndexName().filter(indexName::equals).isPresent(); } @Override diff --git a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java index 33fc35e96520..c21350d3fe0d 100644 --- a/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java +++ b/dotCMS/src/enterprise/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchAPIImpl.java @@ -25,9 +25,11 @@ import java.text.ParseException; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.quartz.SchedulerException; @@ -201,6 +203,33 @@ public Map getAliasToIndexMap() { return router.read(SiteSearchAPI::getAliasToIndexMap); } + /** + * Router: alias resolution over the SAME provider set {@link #listIndices()} uses, so every listed + * index can show its alias — the management/display view (issue #36983). + * + *

Deliberately NOT the read provider alone. The list is a union in the dual-write phases, so an + * index living only on the other engine would otherwise render with a blank alias: Phase 2 + a + * Phase-0 (ES-only) index, or Phase 1 + a Phase-3 (OS-only) index after a downgrade. Merging over + * the write providers keeps the alias view and the index list exactly in step.

+ * + *

The read provider is applied last so it wins any collision: if the two engines resolve one + * alias to different logical indices (a mirror desync), the map agrees with what a search would + * actually hit. In the single-provider phases (0 and 3) there is nothing to merge.

+ */ + @Override + public Map getAliasToIndexMapAllEngines() { + final List providers = router.writeProviders(); + if (providers.size() == 1) { + return providers.getFirst().getAliasToIndexMap(); + } + final SiteSearchAPI readProvider = router.readProvider(); + final Map merged = new LinkedHashMap<>(); + providers.stream().filter(provider -> provider != readProvider) + .forEach(provider -> merged.putAll(provider.getAliasToIndexMap())); + merged.putAll(readProvider.getAliasToIndexMap()); // last write wins → read provider + return merged; + } + // ------------------------------------------------------------------------- // Reads — read provider // ------------------------------------------------------------------------- @@ -221,6 +250,23 @@ public SiteSearchResult getFromIndex(final String index, final String id) { return router.read(impl -> impl.getFromIndex(index, id)); } + /** + * Router: the default site-search index according to the current read provider — Elasticsearch's + * legacy pointer in Phases 0/1, OpenSearch's {@code VersionedIndices} (with a legacy fallback) in + * Phases 2/3. Reading the legacy pointer directly goes stale from Phase 3 on, where + * {@code activateIndex} fans out to OpenSearch alone (issue #36983). + */ + @Override + public Optional defaultIndexName() throws DotDataException { + try { + return router.readChecked(SiteSearchAPI::defaultIndexName); + } catch (DotDataException e) { + throw e; + } catch (Exception e) { + throw new DotDataException(e.getMessage(), e); + } + } + @Override public boolean isDefaultIndex(final String indexName) throws DotDataException { try { 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 15390b1113d6..86e55b5c24da 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 @@ -1,15 +1,19 @@ package com.dotcms.content.index.migration; import com.dotcms.cdi.CDIUtils; +import com.dotcms.content.elasticsearch.business.ContentletIndexOperationsES; import com.dotcms.content.elasticsearch.business.ESIndexAPI; import com.dotcms.content.elasticsearch.business.IndiciesInfo; +import com.dotcms.content.index.ContentletIndexOperations; import com.dotcms.content.index.IndexAPI; import com.dotcms.content.index.IndexTag; import com.dotcms.content.index.domain.IndexStats; import com.dotcms.content.index.migration.MirrorStatus.IndexKind; import com.dotcms.content.index.migration.MirrorStatus.Verdict; +import com.dotcms.content.index.opensearch.ContentletIndexOperationsOS; import com.dotcms.content.index.opensearch.OSIndexAPIImpl; import com.dotmarketing.business.APILocator; +import com.dotmarketing.common.db.DotConnect; import com.dotmarketing.util.Logger; import com.dotmarketing.util.UtilMethods; import com.google.common.annotations.VisibleForTesting; @@ -27,12 +31,22 @@ *

How the counts are read (phase-independently)

*

{@code IndiciesInfo} always holds the cluster-prefixed, un-tagged Elasticsearch name for * working/live (its backing {@code indicies} table owns only the ES rows — {@code index_version IS - * NULL}); the OpenSearch counterpart is that name with the {@code .os} tag. Exact per-engine document - * counts come from each engine leaf's {@code getIndicesStats()} — the index {@code _stats} - * {@code primaries.docs.count}, an exact total not subject to the 10,000 search hit-count cap. 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, tagged — the same strip-then-tag order the maintenance JSP uses.

+ * NULL}); the OpenSearch counterpart is that name with the {@code .os} tag.

+ * + *

Existence comes from each engine leaf's {@code getIndicesStats()} — 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, + * tagged — the same strip-then-tag order the maintenance JSP uses.

+ * + *

The document count is a real count query per index + * ({@link ContentletIndexOperations#getIndexDocumentCount}), not the {@code docs.count} of + * those same stats. The stats counter is per-shard and only advances when the shard refreshes, so it + * trails a just-written document by seconds: a support technician checking whether a publish reached + * OpenSearch would read the previous number and conclude the write was lost. This endpoint is the + * source of truth for that question, so it must never report a number the engine can already + * contradict (issue #36983). A count query is also not subject to the 10,000 search hit-count cap, and + * it matches how the Site Search half has always counted — both halves now answer the same way.

* *

It queries the two engine leaves directly (never the phase-aware router) so the report shows both * sides regardless of which engine the current phase reads from. Scope is the active working/live @@ -42,21 +56,42 @@ public class ContentIndexMirrorReconciler { private final IndexAPI esImpl; private final IndexAPI osImpl; + private final ContentletIndexOperations esOps; + private final ContentletIndexOperations osOps; private final Supplier indiciesSupplier; + private final Supplier databaseCountsSupplier; public ContentIndexMirrorReconciler() { this(new ESIndexAPI(), CDIUtils.getBeanThrows(OSIndexAPIImpl.class), - ContentIndexMirrorReconciler::loadIndiciesQuietly); + new ContentletIndexOperationsES(), + CDIUtils.getBeanThrows(ContentletIndexOperationsOS.class), + ContentIndexMirrorReconciler::loadIndiciesQuietly, + ContentIndexMirrorReconciler::loadDatabaseCountsQuietly); } @VisibleForTesting ContentIndexMirrorReconciler(final IndexAPI esImpl, final IndexAPI osImpl, - final Supplier indiciesSupplier) { + final ContentletIndexOperations esOps, final ContentletIndexOperations osOps, + final Supplier indiciesSupplier, + final Supplier databaseCountsSupplier) { this.esImpl = esImpl; this.osImpl = osImpl; + this.esOps = esOps; + this.osOps = osOps; this.indiciesSupplier = indiciesSupplier; + this.databaseCountsSupplier = databaseCountsSupplier; } + /** + * How many documents each content index should hold according to the database — the denominator + * behind the indexed percentages. Counted exactly (see {@link #loadDatabaseCountsQuietly()}); + * {@code null} on either field when it could not be read. + * + * @param working one row per (identifier, language, variant): the working version always exists + * @param live the subset of those rows that also have a live version + */ + public record DatabaseCounts(Long working, Long live) {} + /** Per-index mirror status for the active working and live content indices. */ public List statuses() { final IndiciesInfo info = indiciesSupplier.get(); @@ -65,14 +100,18 @@ public List statuses() { } final Map esStats = esImpl.getIndicesStats(); final Map osStats = osImpl.getIndicesStats(); + final DatabaseCounts dbCounts = databaseCountsSupplier.get(); final List out = new ArrayList<>(2); - addStatus(out, IndexKind.CONTENT_WORKING, info.getWorking(), esStats, osStats); - addStatus(out, IndexKind.CONTENT_LIVE, info.getLive(), esStats, osStats); + addStatus(out, IndexKind.CONTENT_WORKING, info.getWorking(), esStats, osStats, + dbCounts == null ? null : dbCounts.working()); + addStatus(out, IndexKind.CONTENT_LIVE, info.getLive(), esStats, osStats, + dbCounts == null ? null : dbCounts.live()); return out; } private void addStatus(final List out, final IndexKind kind, final String rawName, - final Map esStats, final Map osStats) { + final Map esStats, final Map osStats, + final Long databaseDocCount) { if (!UtilMethods.isSet(rawName)) { return; } @@ -84,16 +123,78 @@ private void addStatus(final List out, final IndexKind kind, final final String bare = esImpl.removeClusterIdFromName(rawName); final String osKey = IndexTag.OS.tag(bare); + // Existence from the stats snapshot; the count from a live count query (see class javadoc). final boolean esExists = esStats.containsKey(bare); - final long esCount = esExists ? esStats.get(bare).documentCount() : 0L; + final long esCount = esExists ? countQuietly(esOps, bare) : 0L; final boolean osExists = osStats.containsKey(osKey); - final long osCount = osExists ? osStats.get(osKey).documentCount() : 0L; + final long osCount = osExists ? countQuietly(osOps, bare) : 0L; final Verdict verdict = MirrorStatus.verdictFor(esExists, osExists, esCount, osCount); + final String recommendation = recommend(bare, verdict, osExists) + + incompleteNote("Elasticsearch", esExists, esCount, databaseDocCount) + + incompleteNote("OpenSearch", osExists, osCount, databaseDocCount); out.add(new MirrorStatus(bare, kind, new MirrorStatus.EngineCopy(esExists, esCount, esPhysical), new MirrorStatus.EngineCopy(osExists, osCount, osPhysical), - verdict, recommend(bare, verdict, osExists))); + verdict, recommendation, databaseDocCount)); + } + + /** + * Exact document count of {@code logicalName} on one engine, or {@code -1} when the query fails. + * + *

The leaf turns the logical name into its own physical form ({@code toPhysicalName}: the ES + * leaf cluster-prefixes it, the OpenSearch leaf also applies {@code .os}), the same convention + * {@code ContentletIndexAPIImpl} uses — so this never hand-builds a physical name.

+ * + *

Failures are reported as {@code -1} rather than propagated: a readiness report that answers + * "unknown" for one engine is useful, one that returns a 500 is not. {@code -1} is the established + * unmeasurable marker — it compares unequal, so the verdict degrades to out-of-sync and + * {@code safeToRollback} to false, never to a false green.

+ */ + private static long countQuietly(final ContentletIndexOperations ops, final String logicalName) { + return Try.of(() -> ops.getIndexDocumentCount(ops.toPhysicalName(logicalName))) + .onFailure(e -> Logger.warn(ContentIndexMirrorReconciler.class, + "Could not count documents of '" + logicalName + "' on " + + ops.getClass().getSimpleName() + ": " + e.getMessage())) + .getOrElse(-1L); + } + + /** + * Indexed percentage below which an existing index is called out as incomplete in the recommendation. Not a + * tight bound on purpose: the denominator is an order-of-magnitude measure (see + * {@code MirrorStatus#indexedPercentOf}), so this is meant to catch "3% of the content", not a handful of + * documents. + */ + private static final double INCOMPLETE_INDEXED_THRESHOLD = 95.0; + + /** + * A sentence appended to the recommendation when an engine holds materially less content than the + * database says it should. + * + *

This is the half of the report that survives into Phase 3. The verdict compares the two + * engines against each other, so once one of them is the only one left it can read reassuringly + * while the surviving index is nearly empty — and everything downstream inherits that emptiness + * silently, including a Site Search crawl, whose corpus is a query over this very index + * (issue #36983). Comparing against the database keeps that visible with nothing to diff.

+ * + *

It never changes the {@code verdict}: the verdict states the ES↔OS relationship, which is a + * different fact. Reported side by side, not merged.

+ */ + private static String incompleteNote(final String engine, final boolean exists, final long count, + final Long databaseDocCount) { + if (!exists || count < 0 || databaseDocCount == null || databaseDocCount <= 0) { + return ""; + } + final double indexedPercent = count * 100.0 / databaseDocCount; + if (indexedPercent >= INCOMPLETE_INDEXED_THRESHOLD) { + return ""; + } + return String.format(" NOTE: the %s copy holds %d of the %d contentlets the database has " + + "(%.2f%%) — it was never fully rebuilt. Run a full reindex; until then, " + + "anything reading through this index sees only that fraction of the content " + + "(a Site Search crawl included, since it builds its corpus from a query " + + "against it).", + engine, count, databaseDocCount, indexedPercent); } private static String recommend(final String name, final Verdict verdict, final boolean osExists) { @@ -113,6 +214,72 @@ private static String recommend(final String name, final Verdict verdict, final } } + /** + * A fixed, fully literal statement — no interpolation, no parameters, nothing caller-supplied. Kept + * as a constant rather than assembled inline so that stays evident at a glance (and so a + * concatenation-based injection scan has nothing to flag). + */ + private static final String DATABASE_COUNTS_SQL = """ + SELECT COUNT(*) AS working_count, COUNT(live_inode) AS live_count + FROM contentlet_version_info + """; + + /** + * How many documents each content index should hold, counted exactly from + * {@code contentlet_version_info}. + * + *

That table is keyed by {@code (identifier, lang, variant_id)} — the same unit as an index + * document ({@code identifier_language_variant}) — so its row count is the denominator directly: + * {@code COUNT(*)} is every working version ({@code working_inode} is {@code NOT NULL}, so every + * row has one) and {@code COUNT(live_inode)} skips nulls and therefore counts exactly the rows that + * also have a live version. Verified against a live install: 686/685, matching the index document + * counts exactly.

+ * + *

Cost. PostgreSQL runs this as a {@code Parallel Seq Scan}: asking for + * {@code COUNT(live_inode)} needs the column, so the heap is read. Measured on local copies — + * 171k rows / 15 ms, 394k / 21 ms, 453k / 22 ms — i.e. roughly linear at ~50 ns + * per row (warm cache; a cold one pays the disk I/O). It runs on an admin-only endpoint on demand + * and once per crawl, never on a write path.

+ * + *

Kept as one statement deliberately. Splitting it lets {@code COUNT(*)} alone drop to a + * {@code Parallel Index Only Scan} (17 ms), but the live half stays a sequential scan anyway — + * nearly every row has a live version, so the index buys the planner nothing — and the two together + * measured 41 ms against 28 ms for the combined form.

+ * + *

Exact rather than the {@code pg_class.reltuples} estimate on purpose: the estimate drifts a few + * points in either direction between {@code ANALYZE} runs, which surfaces as an indexed percentage slightly over + * 100% and reads as a defect. With exact counts, 100% means complete and any excess is real — + * documents in the index that no longer exist in the database.

+ * + *

Failure is quiet: the indexed-percentage fields are omitted rather than failing the whole report.

+ */ + private static DatabaseCounts loadDatabaseCountsQuietly() { + return Try.of(() -> { + final List> rows = new DotConnect() + .setSQL(DATABASE_COUNTS_SQL) + .loadObjectResults(); + if (rows.isEmpty()) { + return null; + } + final Map row = rows.get(0); + return new DatabaseCounts(positiveOrNull(row.get("working_count")), + positiveOrNull(row.get("live_count"))); + }).onFailure(e -> Logger.warn(ContentIndexMirrorReconciler.class, + "Could not read the expected content counts for migration readiness: " + e.getMessage())) + .getOrElse((DatabaseCounts) null); + } + + /** The count as a positive Long, or {@code null} when it is absent, negative or zero. */ + private static Long positiveOrNull(final Object value) { + final Long count = asLong(value); + return count != null && count > 0 ? count : null; + } + + /** JDBC hands a {@code COUNT} back as Long, BigDecimal or BigInteger depending on the driver. */ + private static Long asLong(final Object value) { + return value instanceof Number ? ((Number) value).longValue() : null; + } + private static IndiciesInfo loadIndiciesQuietly() { return Try.of(() -> APILocator.getIndiciesAPI().loadIndicies()) .onFailure(e -> Logger.warn(ContentIndexMirrorReconciler.class, 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 b581a23fdc80..8c7b2d7f91fd 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 @@ -1,6 +1,7 @@ package com.dotcms.content.index.migration; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import io.swagger.v3.oas.annotations.media.Schema; @@ -15,12 +16,19 @@ * row belongs to. Each engine's side is a nested {@link EngineCopy} so the report reads as * {@code es:{exists,docCount}} / {@code os:{exists,docCount}}.

* - * @param indexName the logical index name (no {@code .os} tag) - * @param kind which mirrored index family this row belongs to - * @param es the Elasticsearch copy (existence + exact document count) - * @param os the OpenSearch ({@code .os}) copy (existence + exact document count) - * @param verdict the diff verdict between the two copies - * @param recommendation human-readable, action-oriented advice for a support technician + * @param indexName the logical index name (no {@code .os} tag) + * @param kind which mirrored index family this row belongs to + * @param es the Elasticsearch copy (existence + exact document count) + * @param os the OpenSearch ({@code .os}) copy (existence + exact document count) + * @param verdict the diff verdict between the two copies + * @param recommendation human-readable, action-oriented advice for a support technician + * @param databaseDocCount how many documents this index should hold according to the + * database — the engine-independent denominator behind + * {@link #esIndexedPercent()} / {@link #osIndexedPercent()}. An exact count + * of {@code contentlet_version_info}, so 100% means complete and any excess is + * real. Only the content indices have one; {@code null} for Site Search, whose + * corpus (crawled pages and files) has no such counterpart, and {@code null} + * when it could not be read. */ @JsonIgnoreProperties("kind") // internal grouping/label only — the report keys rows by it, never emits it public record MirrorStatus( @@ -29,7 +37,14 @@ public record MirrorStatus( EngineCopy es, EngineCopy os, Verdict verdict, - String recommendation) { + String recommendation, + @JsonInclude(JsonInclude.Include.NON_NULL) Long databaseDocCount) { + + /** 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) { + this(indexName, kind, es, os, verdict, recommendation, null); + } /** Which mirrored index family a status row belongs to. */ public enum IndexKind { CONTENT_WORKING, CONTENT_LIVE, SITE_SEARCH } @@ -47,14 +62,29 @@ public enum Verdict { /** * One engine's copy of the index. * + *

The alias is reported per engine on purpose: during the migration an index can carry + * its alias on one engine and not on the other (e.g. an index created before dual-write started, + * whose counterpart was built later), and that asymmetry is precisely what an operator needs to + * see. Collapsing both sides into one field would hide it.

+ * * @param exists whether this engine holds the index * @param docCount exact document count (0 when absent, -1 when the count query failed) * @param physicalName the full index name as stored on that engine's server — cluster-prefixed and, * for OpenSearch, {@code .os}-tagged (e.g. {@code cluster_08abc3.live_20260406} * on ES, {@code cluster_08abc3.live_20260406.os} on OS). Reported whether or not * the copy exists, so a missing copy shows the name to look for. + * @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}. */ - public record EngineCopy(boolean exists, long docCount, String physicalName) {} + public record EngineCopy(boolean exists, long docCount, String physicalName, + @JsonInclude(JsonInclude.Include.NON_NULL) String alias) { + + /** 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); + } + } /** Whether this index needs operator action (a re-crawl / reindex) before the phase change. */ public boolean needsAttention() { @@ -90,6 +120,61 @@ public Double driftPercent() { return Math.round(pct * 100.0) / 100.0; } + /** + * What percentage of the database's content the Elasticsearch copy holds — a percentage of + * {@link #databaseDocCount}. See {@link #indexedPercentOf(EngineCopy)}. + */ + @JsonProperty("esIndexedPercent") + @JsonInclude(JsonInclude.Include.NON_NULL) + @Schema(description = "Percentage of the documents the database says this index should hold that " + + "the Elasticsearch copy actually holds. 100.0 = complete. Absent for Site Search " + + "(no database denominator) and when a count could not be measured.") + public Double esIndexedPercent() { + return indexedPercentOf(es); + } + + /** + * What percentage of the database's content the OpenSearch copy holds — a percentage of + * {@link #databaseDocCount}. See {@link #indexedPercentOf(EngineCopy)}. + */ + @JsonProperty("osIndexedPercent") + @JsonInclude(JsonInclude.Include.NON_NULL) + @Schema(description = "Percentage of the documents the database says this index should hold that " + + "the OpenSearch copy actually holds. 100.0 = complete; a low value means the mirror was " + + "never rebuilt — and anything reading through it (including a Site Search crawl) sees " + + "only that fraction of the content. Absent for Site Search (no database denominator) " + + "and when a count could not be measured.") + public Double osIndexedPercent() { + return indexedPercentOf(os); + } + + /** + * One engine's completeness against the database: {@code docCount / databaseDocCount × 100}, + * rounded to two decimals. + * + *

Why this exists next to {@link #driftPercent()}. Drift compares the two + * engines against each other, which stops being an answer once one of them is the only + * one left: in Phase 3 there is no Elasticsearch side to compare against, so a mirror that was + * never rebuilt looks unremarkable. This compares each engine against the database — + * the source of truth, identical in every phase — so "this index holds 3% of the content" is + * still visible when there is nothing to diff (issue #36983).

+ * + *

The denominator counts one row per (identifier, language, variant) in + * {@code contentlet_version_info} — the same unit as an index document — so a complete index reads + * exactly {@code 100.0}. Above 100% means the index holds documents the database no longer has + * (orphans left by a delete that never propagated), which is worth looking at rather than + * rounding away.

+ * + * @return the percentage, or {@code null} when there is no denominator ({@code databaseDocCount} + * absent or zero) or the count was unmeasurable ({@code -1}) + */ + private Double indexedPercentOf(final EngineCopy copy) { + if (databaseDocCount == null || databaseDocCount <= 0 || copy.docCount() < 0) { + return null; + } + return Math.round(copy.docCount() * 10_000.0 / databaseDocCount) / 100.0; + } + /** * Classifies a mirror from raw existence + exact counts: a missing copy on either engine is * {@link Verdict#MISSING_COUNTERPART}; both present with unequal counts is {@link Verdict#COUNT_DRIFT} 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 c22cbe11d799..3ba522384e9b 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 @@ -10,9 +10,12 @@ import com.dotmarketing.sitesearch.business.SiteSearchAPI; import com.google.common.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.TreeSet; import java.util.function.Supplier; +import java.util.regex.Pattern; /** * Site Search half of the migration-readiness report (issue #36360): compares every logical @@ -52,14 +55,29 @@ public SiteSearchMirrorReconciler() { public List statuses() { final TreeSet names = new TreeSet<>(esImpl.listIndices()); names.addAll(osImpl.listIndices()); + // 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 List statuses = new ArrayList<>(names.size()); for (final String name : names) { - statuses.add(statusFor(name)); + statuses.add(statusFor(name, esAliases.get(name), osAliases.get(name))); } return statuses; } - private MirrorStatus statusFor(final String name) { + /** + * Reverses one engine's {@code alias -> index} map into {@code index -> alias}. Both leaves return + * logical (untagged) index names, so the keys line up with {@link SiteSearchAPI#listIndices()}. + */ + private static Map indexToAlias(final SiteSearchAPI engine) { + final Map reversed = new HashMap<>(); + engine.getAliasToIndexMap().forEach((alias, index) -> reversed.put(index, alias)); + 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; @@ -70,24 +88,59 @@ private MirrorStatus statusFor(final String 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), - new MirrorStatus.EngineCopy(osExists, osCount, osPhysical), - verdict, recommend(name, verdict)); + new MirrorStatus.EngineCopy(esExists, esCount, esPhysical, esAlias), + new MirrorStatus.EngineCopy(osExists, osCount, osPhysical, osAlias), + verdict, recommend(name, verdict, esAlias, osAlias)); + } + + /** + * A site-search index name: {@code sitesearch_[_]}. Used to spot an alias that is + * really an index name — see {@link #corruptedAlias(String, String)}. + */ + private static final Pattern INDEX_NAME_SHAPED = + Pattern.compile("^" + SiteSearchAPI.ES_SITE_SEARCH_NAME + "_\\d{8,}.*", Pattern.CASE_INSENSITIVE); + + /** + * The alias of {@code name} on either engine when it is really an INDEX NAME rather than an alias — + * the fingerprint of the defect fixed in issue #36983, where a crawl re-applied the name of the + * index it had just deleted as the new index's alias. The fix stops it from happening again but + * cannot restore an alias already overwritten, so the report surfaces it: this is the only way an + * operator can tell which indices still need their alias restored. + * + * @return the offending alias, or {@code null} when neither engine's alias looks like an index name + */ + private static String corruptedAlias(final String esAlias, final String osAlias) { + if (esAlias != null && INDEX_NAME_SHAPED.matcher(esAlias).matches()) { + return esAlias; + } + if (osAlias != null && INDEX_NAME_SHAPED.matcher(osAlias).matches()) { + return osAlias; + } + return null; } - private static String recommend(final String name, final Verdict verdict) { + private static String recommend(final String name, final Verdict verdict, final String esAlias, + final String osAlias) { + // Reported alongside the sync verdict, never as part of it: the verdict measures data + // integrity (existence + counts), while a damaged alias is an identification problem. Folding + // it into the verdict would block a phase change over something that costs no data. + final String corrupted = corruptedAlias(esAlias, osAlias); + final String aliasNote = corrupted == null ? "" : String.format( + " NOTE: the alias '%s' is an index name, not a real alias — a crawl overwrote the " + + "original alias of '%s' (issue #36983). Re-crawl this index with the intended " + + "alias to restore it.", corrupted, name); switch (verdict) { case IN_SYNC: - return "In sync — no action needed."; + return "In sync — no action needed." + aliasNote; case MISSING_COUNTERPART: return String.format("A copy of site-search index '%s' is missing on one engine. " + "Re-crawl it (Site Search → Run now) to rebuild the counterpart before " - + "promoting to the OpenSearch-only phase.", name); + + "promoting to the OpenSearch-only phase.", name) + aliasNote; case COUNT_DRIFT: default: return String.format("The two copies of site-search index '%s' hold a different " + "number of documents. Re-crawl it (Site Search → Run now) to rebuild " - + "the counterpart before promoting the phase.", name); + + "the counterpart before promoting the phase.", name) + aliasNote; } } } diff --git a/dotCMS/src/main/java/com/dotcms/publishing/job/SiteSearchJobImpl.java b/dotCMS/src/main/java/com/dotcms/publishing/job/SiteSearchJobImpl.java index 5e01d1d0396f..2e4c706e9a6e 100644 --- a/dotCMS/src/main/java/com/dotcms/publishing/job/SiteSearchJobImpl.java +++ b/dotCMS/src/main/java/com/dotcms/publishing/job/SiteSearchJobImpl.java @@ -2,6 +2,9 @@ import com.dotcms.content.elasticsearch.business.ESMappingAPIImpl; import com.dotcms.content.elasticsearch.business.IndiciesAPI; +import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; +import com.dotcms.content.index.migration.ContentIndexMirrorReconciler; +import com.dotcms.content.index.migration.MirrorStatus; import com.dotcms.enterprise.LicenseUtil; import com.dotcms.enterprise.license.LicenseLevel; import com.dotcms.enterprise.publishing.bundlers.FileAssetBundler; @@ -24,6 +27,7 @@ import com.dotmarketing.sitesearch.business.SiteSearchAuditAPI; import com.dotmarketing.sitesearch.model.SiteSearchAudit; import com.dotmarketing.util.ActivityLogger; +import com.dotmarketing.util.Config; import com.dotmarketing.util.AdminLogger; import com.dotmarketing.util.DateUtil; import com.dotmarketing.util.Logger; @@ -35,6 +39,7 @@ import com.google.common.collect.ImmutableList.Builder; import com.liferay.portal.model.User; import com.liferay.util.StringPool; +import io.vavr.control.Try; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -44,6 +49,8 @@ import java.util.ListIterator; import java.util.Map; import java.util.Objects; +import java.util.Optional; +import java.util.function.Supplier; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -83,6 +90,7 @@ public class SiteSearchJobImpl { private final UserAPI userAPI; private final SiteSearchAuditAPI siteSearchAuditAPI; private final PublisherAPI publisherAPI; + private final Supplier> contentMirrorStatuses; private String bundleId; @@ -95,12 +103,27 @@ public class SiteSearchJobImpl { final SiteSearchAuditAPI siteSearchAuditAPI, final PublisherAPI publisherAPI ) { + this(indicesAPI, siteSearchAPI, hostAPI, userAPI, siteSearchAuditAPI, publisherAPI, + () -> new ContentIndexMirrorReconciler().statuses()); + } + + @VisibleForTesting + SiteSearchJobImpl( + final IndiciesAPI indicesAPI, + final SiteSearchAPI siteSearchAPI, + final HostAPI hostAPI, + final UserAPI userAPI, + final SiteSearchAuditAPI siteSearchAuditAPI, + final PublisherAPI publisherAPI, + final Supplier> contentMirrorStatuses + ) { this.indicesAPI = indicesAPI; this.siteSearchAPI = siteSearchAPI; this.hostAPI = hostAPI; this.userAPI = userAPI; this.siteSearchAuditAPI = siteSearchAuditAPI; this.publisherAPI = publisherAPI; + this.contentMirrorStatuses = contentMirrorStatuses; } public SiteSearchJobImpl() { @@ -198,7 +221,8 @@ public void run(final JobExecutionContext jobContext) + "; Job Identifier: " + SiteSearchAPI.ES_SITE_SEARCH_NAME); } - private PreparedJobContext prepareJob(final JobExecutionContext jobContext) + @VisibleForTesting + PreparedJobContext prepareJob(final JobExecutionContext jobContext) throws DotDataException, IOException, DotSecurityException { synchronized (SiteSearchJobImpl.class) { final JobDataMap dataMap = jobContext.getJobDetail().getJobDataMap(); @@ -239,6 +263,12 @@ private PreparedJobContext prepareJob(final JobExecutionContext jobContext) // Run now jobs can not get the incremental treatment. final String indexAlias = getAliasName(dataMap.getString(INDEX_ALIAS)); final IndexMetaData indexMetaData = getIndexMetaData(indexAlias); + // The alias the crawl must end up with. It is NOT necessarily the string stored in the + // job detail: that one can be a raw index name, in which case getIndexMetaData resolves + // the index's real alias (or null when it has none). Everything downstream — the config + // handed to the publisher, which re-applies it after the index switch — must use this + // resolved value, never the raw stored string (issue #36983). + final String resolvedAlias = indexMetaData.getAlias(); final String newIndexName; final String indexName; @@ -298,19 +328,23 @@ private PreparedJobContext prepareJob(final JobExecutionContext jobContext) uniqueFolderName(); // We use a new index name only on non-incremental newIndexName = newIndexName(); - final String newAlias = - indexMetaData.isNewIndex() ? indexMetaData.getAlias() : null; + final String newAlias = indexMetaData.isNewIndex() ? resolvedAlias : null; siteSearchAPI.createSiteSearchIndex(newIndexName, newAlias, 1); // This is the old index we will swap from. // if it doesnt exist. It doesnt matter here since we will end up with the new one. indexName = indexMetaData.getIndexName(); } + // Advisory, never a gate: the crawl reads the content index, so an incomplete one silently + // yields a partial Site Search index (issue #36983). + incompleteContentIndexWarning() + .ifPresent(warning -> Logger.warn(SiteSearchJobImpl.class, warning)); + Logger.info(SiteSearchJobImpl.class, () -> String .format("Incremental mode [%s]. current index is `%s`. new index is `%s`. alias is `%s` bundle id is `%s` ", BooleanUtils.toStringYesNo(incremental), indexName, UtilMethods.isSet(newIndexName) ? newIndexName : "N/A", - indexAlias, + UtilMethods.isSet(resolvedAlias) ? resolvedAlias : "N/A", bundleId) ); @@ -342,7 +376,7 @@ private PreparedJobContext prepareJob(final JobExecutionContext jobContext) config.setHosts(hosts); config.setNewIndexName(newIndexName); config.setIndexName(indexName); - config.setIndexAlias(indexAlias); + config.setIndexAlias(resolvedAlias); config.setId(bundleId); config.setStartDate(startDate); config.setEndDate(endDate); @@ -375,6 +409,64 @@ private PreparedJobContext prepareJob(final JobExecutionContext jobContext) } } + /** + * Config key for the indexed percentage below which a crawl is warned about. Percentage of the content the + * database says exists; {@code 0} disables the check. + */ + static final String MIN_CONTENT_INDEXED_KEY = "SITE_SEARCH_CRAWL_MIN_CONTENT_INDEXED_PERCENT"; + + /** Default: warn when the content index serving the crawl is missing more than 5% of the content. */ + static final double DEFAULT_MIN_CONTENT_INDEXED = 95.0; + + /** + * The warning to emit before crawling when the content index this crawl will read from is + * materially incomplete, or {@link Optional#empty()} when there is nothing to say. + * + *

Why a crawl cares about the CONTENT index. A crawl does not read the + * database: the bundlers build the bundle from {@code ContentletAPI#searchIndex}, a phase-routed + * search over the content index. So the crawl can only find what that index holds — if the + * OpenSearch content mirror was never rebuilt, a Phase-3 crawl silently produces a Site Search + * index containing a fraction of the site, reports success, and that truncated index survives even + * after the content store is reindexed (issue #36983).

+ * + *

Measured against the database, not against the other engine: in Phase 3 there is no + * other engine to compare with, which is exactly when this is most needed. Advisory only — it never + * stops the crawl, and any failure to compute it is swallowed, because a diagnostic must not be + * able to break indexing.

+ */ + @VisibleForTesting + Optional incompleteContentIndexWarning() { + final double threshold = Config.getFloatProperty( + MIN_CONTENT_INDEXED_KEY, (float) DEFAULT_MIN_CONTENT_INDEXED); + if (threshold <= 0) { + return Optional.empty(); + } + final boolean readsOpenSearch = MigrationPhase.current().isReadEnabled(); + return Try.of(() -> contentMirrorStatuses.get().stream() + .map(status -> indexedShortfall(status, readsOpenSearch, threshold)) + .flatMap(Optional::stream) + .findFirst()) + .getOrElse(Optional.empty()); + } + + /** The shortfall message for one content row, or empty when that row is fine or unmeasured. */ + private static Optional indexedShortfall(final MirrorStatus status, + final boolean readsOpenSearch, final double threshold) { + final Double indexedPercent = readsOpenSearch + ? status.osIndexedPercent() : status.esIndexedPercent(); + if (indexedPercent == null || indexedPercent >= threshold) { + return Optional.empty(); + } + return Optional.of(String.format( + "Site Search crawl starting against an INCOMPLETE content index: '%s' on %s holds " + + "%.2f%% of the %d contentlets the database has. A crawl builds its corpus by " + + "querying that index, so it can only index what it finds there — this crawl " + + "will produce a partial Site Search index, and reindexing the content later " + + "will NOT repair it (it must be crawled again). Run a full reindex first.", + status.indexName(), readsOpenSearch ? "OpenSearch" : "Elasticsearch", indexedPercent, + status.databaseDocCount())); + } + /** * Unique thread safe site-search index name * @return @@ -401,7 +493,8 @@ private String uniqueFolderName(){ * @return @see IndexMetaData * @throws DotDataException */ - private IndexMetaData getIndexMetaData(String indexAlias) throws DotDataException { + @VisibleForTesting + IndexMetaData getIndexMetaData(String indexAlias) throws DotDataException { String indexName = null; boolean defaultIndex = false; long recordCount = 0; @@ -410,7 +503,12 @@ private IndexMetaData getIndexMetaData(String indexAlias) throws DotDataExceptio // Resolve via the site-search API so aliases are looked up with .os-aware physical names // in Phases 2/3; the content-index router misses site-search aliases there // and would force every crawl into full mode (issue #36360). - final Map aliasMap = siteSearchAPI.getAliasToIndexMap(); + // AllEngines: over the same provider set as `indices` above. A crawl can legitimately + // target an index that lives only on the engine the phase does not read from (e.g. an + // OpenSearch-only index created in Phase 3, seen again after a downgrade to Phase 1) — + // with a read-provider-only map its alias is invisible, so the crawl would treat it as a + // brand-new index and drop the alias instead of carrying it over (issue #36983). + final Map aliasMap = siteSearchAPI.getAliasToIndexMapAllEngines(); indexName = aliasMap.get(indexAlias); if (UtilMethods.isSet(indexName)) { if (siteSearchAPI.isDefaultIndex(indexAlias)) { @@ -421,7 +519,13 @@ private IndexMetaData getIndexMetaData(String indexAlias) throws DotDataExceptio // the alias comes with an index name that is already in use. if(indices.contains(indexAlias)){ indexName = indexAlias; - indexAlias = null; + // The job was saved with a raw index name where an alias was expected (the job + // scheduler used to fall back to raw names when alias resolution missed on + // OpenSearch — issue #36983). Recover the index's REAL alias so a full crawl + // re-applies it to the new index. Carrying the raw name forward instead would + // make `switchIndex` set the DEAD index's name as the new index's alias, + // destroying the alias the user created (issue #36983, Bug 1). + indexAlias = aliasOf(indexName, aliasMap); } } if(UtilMethods.isSet(indexName)){ @@ -432,6 +536,21 @@ private IndexMetaData getIndexMetaData(String indexAlias) throws DotDataExceptio return new IndexMetaData(indexName, defaultIndex, indexAlias, recordCount == 0); } + /** + * Reverse lookup of the alias attached to {@code indexName}, given an alias→index map. + * + * @param indexName the (logical) index name to find an alias for + * @param aliasToIndex alias → index map as returned by {@link SiteSearchAPI#getAliasToIndexMap()} + * @return the alias pointing at {@code indexName}, or {@code null} when the index has none + */ + private static String aliasOf(final String indexName, final Map aliasToIndex) { + return aliasToIndex.entrySet().stream() + .filter(entry -> indexName.equals(entry.getValue())) + .map(Map.Entry::getKey) + .findFirst() + .orElse(null); + } + private static final Pattern invalidAliasNamePattern = Pattern.compile("[^a-zA-Z0-9-_]"); /** diff --git a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java index ea9a26cecbbd..b2683202c214 100644 --- a/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java +++ b/dotCMS/src/main/java/com/dotcms/rest/api/v1/index/MigrationReadinessResource.java @@ -73,6 +73,11 @@ public MigrationReadinessResource() { + "Per index: `es`/`os` = {exists, docCount (exact; -1 = count failed), physicalName " + "(full name as stored: cluster-prefixed, .os-tagged on OpenSearch)}; `verdict` = " + "IN_SYNC | MISSING_COUNTERPART | COUNT_DRIFT; `recommendation` = what to run to fix it.\n\n" + + "Content rows also carry `databaseDocCount` (what the database says the index should " + + "hold) with `esIndexedPercent` / `osIndexedPercent` — each engine measured against the " + + "DATABASE rather than against the other engine, so an index that was never rebuilt is " + + "still visible in Phase 3, where there is no second engine to diff against. Absent for " + + "Site Search (no database denominator).\n\n" + "`driftPercent` = how far the OpenSearch mirror deviates from the Elasticsearch original, " + "as a signed % of the original: (OS − ES) / ES × 100. 0.0 = in sync; NEGATIVE = mirror " + "BEHIND (missing that % of docs); POSITIVE = mirror AHEAD (extra docs); -100.0 = mirror " diff --git a/dotCMS/src/main/java/com/dotmarketing/sitesearch/ajax/SiteSearchAjaxAction.java b/dotCMS/src/main/java/com/dotmarketing/sitesearch/ajax/SiteSearchAjaxAction.java index e1cc5337d5e2..ce0902508a7e 100644 --- a/dotCMS/src/main/java/com/dotmarketing/sitesearch/ajax/SiteSearchAjaxAction.java +++ b/dotCMS/src/main/java/com/dotmarketing/sitesearch/ajax/SiteSearchAjaxAction.java @@ -265,7 +265,11 @@ public void getIndexStatus(HttpServletRequest request, HttpServletResponse respo String indexName = ESIndexHelper.getInstance().getIndexNameOrAlias(map,"indexName", "indexAlias", APILocator.getESIndexAPI()); response.setContentType("text/plain"); - response.getWriter().println(APILocator.getIndiciesAPI().loadIndicies().getSiteSearch().equals(indexName) ? "default" : "inactive"); + // Phase-aware default (issue #36983): the legacy IndiciesInfo pointer freezes at the + // Elasticsearch-era default from Phase 3 on, where activateIndex fans out to OpenSearch + // alone — and dereferencing it NPE'd when no default had ever been set. + response.getWriter().println( + APILocator.getSiteSearchAPI().isDefaultIndex(indexName) ? "default" : "inactive"); } catch(Exception ex) { throw new RuntimeException(ex); @@ -275,7 +279,8 @@ public void getIndexStatus(HttpServletRequest request, HttpServletResponse respo @Override public void getNotActiveIndexNames(HttpServletRequest request, HttpServletResponse response) throws IOException { try { - String defaultIndex=APILocator.getIndiciesAPI().loadIndicies().getSiteSearch(); + // Phase-aware default (issue #36983) — see getIndexStatus above. + final String defaultIndex = APILocator.getSiteSearchAPI().defaultIndexName().orElse(null); List ret=new ArrayList<>(); for(String ii : APILocator.getSiteSearchAPI().listIndices()) if(defaultIndex==null || !defaultIndex.equals(ii)) 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 d314a30a8b88..f51f35005ffc 100644 --- a/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java +++ b/dotCMS/src/main/java/com/dotmarketing/sitesearch/business/SiteSearchAPI.java @@ -1,208 +1,266 @@ -package com.dotmarketing.sitesearch.business; - -import java.io.IOException; -import java.text.ParseException; -import java.util.List; -import java.util.Map; - -import org.quartz.SchedulerException; - -import com.dotcms.content.index.domain.Aggregation; -import com.dotcms.content.index.domain.DotSearchException; -import com.dotcms.enterprise.publishing.sitesearch.SiteSearchConfig; -import com.dotcms.enterprise.publishing.sitesearch.SiteSearchPublishStatus; -import com.dotcms.enterprise.publishing.sitesearch.SiteSearchResult; -import com.dotcms.enterprise.publishing.sitesearch.SiteSearchResults; -import com.dotmarketing.exception.DotDataException; -import com.dotmarketing.quartz.ScheduledTask; - - -public interface SiteSearchAPI { - public static final String ES_SITE_SEARCH_NAME = "sitesearch"; - public static final String ES_SITE_SEARCH_MAPPING = "_doc"; - public static final String ES_SITE_SEARCH_EXECUTE_JOB_NAME = "runningOnce"; - - List 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 - * only). - * - *

Why this exists — the incremental-crawl safety gate

- * A Site Search index is one logical index mirrored across both engines. An - * incremental crawl writes documents in place into an existing index - * rather than rebuilding it, so it never issues a {@code createSiteSearchIndex}. If a write engine - * is missing its copy of that index (a phase rollout that never rebuilt an old index, a Phase-0 - * index that has no OpenSearch twin yet, or a shadow-create that failed fire-and-forget), the - * in-place document write would let the engine auto-create the index with a dynamic - * mapping — {@code keyword} fields become {@code text}, breaking aggregations. The crawl planner - * gates on this method: when it returns {@code false} it must fall back to a full - * rebuild, which recreates the index (with the correct mapping) on every engine and - * re-points the alias — self-healing the missing mirror on the next crawl (issue #36360). - * - *

For a single-engine implementation this is simply whether that engine holds the index; the - * phase-aware router ({@code SiteSearchAPIImpl}) aggregates it across all current write providers.

- * - * @param indexName the logical site-search index name (no {@code .os} tag) - * @return {@code true} only if every current write engine already holds the index - */ - boolean existsOnAllWriteEngines(String indexName); - - /** - * Whether the index's copies on every current write engine are in sync — i.e. - * the index exists on all of them (see {@link #existsOnAllWriteEngines(String)}) and its - * document counts match across engines. - * - *

Why counts, not just existence

- * A missing twin is one kind of desync; the other is content drift — both engines hold - * the index but with different documents (e.g. an OpenSearch shadow write failed fire-and-forget - * during a previous incremental crawl, so ES has documents OpenSearch does not). Existence alone - * cannot see that. Because a Site Search index is written only by the crawl job (single writer, - * immediate refresh) and no crawl on the same index runs concurrently, at crawl-planning time the - * copies are quiescent, so equal document counts is a sound in-sync invariant and a mismatch is - * real drift. - * - *

The incremental-crawl gate uses this instead of bare existence: an incremental crawl writes - * only the new delta and would perpetuate any pre-existing drift, so when the mirrors are out of - * sync the crawl is demoted to a full rebuild that re-creates identical copies on every engine - * (issue #36360). For a single write engine there is nothing to compare, so this is trivially - * {@code true}; the phase-aware router aggregates it across all current write providers.

- * - * @param indexName the logical site-search index name (no {@code .os} tag) - * @return {@code true} only if the index exists on every write engine with matching document counts - */ - boolean writeMirrorsInSync(String indexName); - - /** - * Accurate document count of this index's physical copy on this engine (Elasticsearch the plain - * index, OpenSearch the {@code .os} twin) — the primitive behind the {@link #writeMirrorsInSync(String)} - * parity check. - * - *

Unlike a plain {@code search(...).getTotalResults()}, this returns an exact - * total. Default hit-count tracking on the Elasticsearch 7.x / OpenSearch clients caps reported - * totals at 10,000, so two large mirrors that have genuinely drifted (e.g. 15,000 vs 12,000) would - * both read back {@code 10000} and compare equal, hiding the drift the gate exists to catch. This - * method issues a real count (a dedicated count request / {@code track_total_hits}) so drift is - * detected above 10k docs (issue #36360).

- * - * @param indexName the logical site-search index name (no {@code .os} tag) - * @return the exact document count; {@code 0} if the index does not exist on this engine; {@code -1} - * if the count query failed — callers must treat a failed count as "not in sync" (rebuild), - * never as an empty index - */ - long documentCount(String indexName); - - /** - * Resolves site-search aliases to their backing index names — phase-aware and OpenSearch - * {@code .os}-aware. Keys (alias) and values (index) are both logical names. - * - *

Design decision — why this lives on {@code SiteSearchAPI}, not the content-index router

- * A Site Search index is one logical index mirrored across both engines, so this API's - * surface speaks in logical (untagged) names — a vendor-neutral handle — and each engine adapter - * translates that handle to its physical form at the boundary (ES uses it verbatim; OpenSearch - * appends {@code .os}). Alias resolution therefore MUST live here: the OpenSearch adapter knows to - * re-tag the lookup with {@code .os}, whereas the content-index router - * ({@code IndexAPI#getAliasToIndexMap}) builds the OS physical name without {@code .os} - * and, in Phases 2/3 (OS reads), queries a name that does not exist — silently returning - * nothing ("Index Alias not found"). Routing site-search alias resolution through the content - * router was the root cause fixed in issue #36360; callers must use this method and never the - * content router with a logical Site Search name. - * - *

The {@code .os} tag never crosses this boundary: it is applied only inside the OpenSearch - * adapter for the lookup and stripped back off the resolved value, so both the alias keys and the - * index values returned here are logical and directly comparable against {@link #listIndices()} - * output.

- * - *

Why dual-write phases cannot collide the map

- * In Phases 1/2 the ES twin ({@code xxx}) and the OpenSearch twin ({@code xxx.os}) carry the - * same alias, so a naive ES∪OS merge would map one alias key to two different index - * values and silently drop one. This method avoids that by resolving against a single - * engine — the current phase's read provider (ES in Phases 0/1, OS in - * Phases 2/3), never a union. The two twins never land in the same map: Phase 1 returns - * {@code {lol=xxx}} from ES; Phase 2 returns {@code {lol=xxx}} from OS ({@code xxx.os} with the - * tag stripped). Because both twins share the same logical base, a synchronized cluster resolves - * the alias to the same logical name in every phase. - * - *

Two residual edges, both benign here:

- *
    - *
  1. Multi-index alias within one engine (one alias pointing at two indices on - * the same provider) would lose one entry to the reverse-map — but that state is prevented - * upstream by the {@code createAlias} existence check (issue #36360). A healthy cluster has - * one index per alias per engine.
    2. - *
  2. Mirror desync (the ES and OS aliases point at different logical - * indices) makes the result diverge by phase — which is correct, since you resolve against - * the engine you read from; it is a mirror-reconciliation concern, not a collision.
    3. - *
- * - * @return map of logical alias name to logical index name; empty when nothing resolves - */ - Map getAliasToIndexMap(); - - /** - * This basically tells you if the index passed as parameter is the default site search index or not - * @param indexName - * @return - * @throws DotDataException - */ - boolean isDefaultIndex(String indexName) throws DotDataException; - - void activateIndex(String indexName) throws DotDataException; - - void deactivateIndex(String indexName) throws DotDataException, IOException; - - boolean createSiteSearchIndex(String indexName, String alias, int shards) throws DotSearchException, IOException; - - boolean setAlias(String indexName, final String alias); - - List getTasks() throws SchedulerException; - - void deleteTask(String taskName) throws SchedulerException; - - void scheduleTask(SiteSearchConfig config) throws SchedulerException, ParseException, ClassNotFoundException; - - void putToIndex(String idx, SiteSearchResult res, String resultType); - - void putToIndex(String idx, List res, String resultType); - - void deleteFromIndex(String idx, String docId); - - SiteSearchResults search(String query, int start, int rows); - - SiteSearchResults search(String indexName, String query, int start, int rows); - - ScheduledTask getTask(String taskName) throws SchedulerException; - - void pauseTask(String taskName) throws SchedulerException; - - SiteSearchPublishStatus getTaskProgress(String jobName) throws SchedulerException; - - boolean isTaskRunning(String jobName) throws SchedulerException; - - void executeTaskNow(SiteSearchConfig config) throws SchedulerException, ParseException, ClassNotFoundException; - - SiteSearchResult getFromIndex(String index, String id); - - Map getAggregations(String indexName, String query) throws DotDataException; - - /*** - * @deprecated use getAggregations instead - */ - @Deprecated - Map getFacets(String indexName, String query) throws DotDataException; - - List listClosedIndices(); - - public void deleteOldSiteSearchIndices(); - - /** - * Deletes a single site-search index by name from every engine that holds it (ES and, during a - * migration, its OpenSearch counterpart), mirroring the operator's single-index view. The - * active (default) site-search index cannot be deleted — deactivate it first. - * - * @param indexName the site-search index name (must be a {@code sitesearch_*} name) - * @throws DotDataException if the name is not a site-search index or the delete fails - * @throws IOException on an index-engine error - */ - void deleteIndex(String indexName) throws DotDataException, IOException; -} +package com.dotmarketing.sitesearch.business; + +import java.io.IOException; +import java.text.ParseException; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import org.quartz.SchedulerException; + +import com.dotcms.content.index.domain.Aggregation; +import com.dotcms.content.index.domain.DotSearchException; +import com.dotcms.enterprise.publishing.sitesearch.SiteSearchConfig; +import com.dotcms.enterprise.publishing.sitesearch.SiteSearchPublishStatus; +import com.dotcms.enterprise.publishing.sitesearch.SiteSearchResult; +import com.dotcms.enterprise.publishing.sitesearch.SiteSearchResults; +import com.dotmarketing.exception.DotDataException; +import com.dotmarketing.quartz.ScheduledTask; + + +public interface SiteSearchAPI { + public static final String ES_SITE_SEARCH_NAME = "sitesearch"; + public static final String ES_SITE_SEARCH_MAPPING = "_doc"; + public static final String ES_SITE_SEARCH_EXECUTE_JOB_NAME = "runningOnce"; + + List 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 + * only). + * + *

Why this exists — the incremental-crawl safety gate

+ * A Site Search index is one logical index mirrored across both engines. An + * incremental crawl writes documents in place into an existing index + * rather than rebuilding it, so it never issues a {@code createSiteSearchIndex}. If a write engine + * is missing its copy of that index (a phase rollout that never rebuilt an old index, a Phase-0 + * index that has no OpenSearch twin yet, or a shadow-create that failed fire-and-forget), the + * in-place document write would let the engine auto-create the index with a dynamic + * mapping — {@code keyword} fields become {@code text}, breaking aggregations. The crawl planner + * gates on this method: when it returns {@code false} it must fall back to a full + * rebuild, which recreates the index (with the correct mapping) on every engine and + * re-points the alias — self-healing the missing mirror on the next crawl (issue #36360). + * + *

For a single-engine implementation this is simply whether that engine holds the index; the + * phase-aware router ({@code SiteSearchAPIImpl}) aggregates it across all current write providers.

+ * + * @param indexName the logical site-search index name (no {@code .os} tag) + * @return {@code true} only if every current write engine already holds the index + */ + boolean existsOnAllWriteEngines(String indexName); + + /** + * Whether the index's copies on every current write engine are in sync — i.e. + * the index exists on all of them (see {@link #existsOnAllWriteEngines(String)}) and its + * document counts match across engines. + * + *

Why counts, not just existence

+ * A missing twin is one kind of desync; the other is content drift — both engines hold + * the index but with different documents (e.g. an OpenSearch shadow write failed fire-and-forget + * during a previous incremental crawl, so ES has documents OpenSearch does not). Existence alone + * cannot see that. Because a Site Search index is written only by the crawl job (single writer, + * immediate refresh) and no crawl on the same index runs concurrently, at crawl-planning time the + * copies are quiescent, so equal document counts is a sound in-sync invariant and a mismatch is + * real drift. + * + *

The incremental-crawl gate uses this instead of bare existence: an incremental crawl writes + * only the new delta and would perpetuate any pre-existing drift, so when the mirrors are out of + * sync the crawl is demoted to a full rebuild that re-creates identical copies on every engine + * (issue #36360). For a single write engine there is nothing to compare, so this is trivially + * {@code true}; the phase-aware router aggregates it across all current write providers.

+ * + * @param indexName the logical site-search index name (no {@code .os} tag) + * @return {@code true} only if the index exists on every write engine with matching document counts + */ + boolean writeMirrorsInSync(String indexName); + + /** + * Accurate document count of this index's physical copy on this engine (Elasticsearch the plain + * index, OpenSearch the {@code .os} twin) — the primitive behind the {@link #writeMirrorsInSync(String)} + * parity check. + * + *

Unlike a plain {@code search(...).getTotalResults()}, this returns an exact + * total. Default hit-count tracking on the Elasticsearch 7.x / OpenSearch clients caps reported + * totals at 10,000, so two large mirrors that have genuinely drifted (e.g. 15,000 vs 12,000) would + * both read back {@code 10000} and compare equal, hiding the drift the gate exists to catch. This + * method issues a real count (a dedicated count request / {@code track_total_hits}) so drift is + * detected above 10k docs (issue #36360).

+ * + * @param indexName the logical site-search index name (no {@code .os} tag) + * @return the exact document count; {@code 0} if the index does not exist on this engine; {@code -1} + * if the count query failed — callers must treat a failed count as "not in sync" (rebuild), + * never as an empty index + */ + long documentCount(String indexName); + + /** + * Resolves site-search aliases to their backing index names — phase-aware and OpenSearch + * {@code .os}-aware. Keys (alias) and values (index) are both logical names. + * + *

Design decision — why this lives on {@code SiteSearchAPI}, not the content-index router

+ * A Site Search index is one logical index mirrored across both engines, so this API's + * surface speaks in logical (untagged) names — a vendor-neutral handle — and each engine adapter + * translates that handle to its physical form at the boundary (ES uses it verbatim; OpenSearch + * appends {@code .os}). Alias resolution therefore MUST live here: the OpenSearch adapter knows to + * re-tag the lookup with {@code .os}, whereas the content-index router + * ({@code IndexAPI#getAliasToIndexMap}) builds the OS physical name without {@code .os} + * and, in Phases 2/3 (OS reads), queries a name that does not exist — silently returning + * nothing ("Index Alias not found"). Routing site-search alias resolution through the content + * router was the root cause fixed in issue #36360; callers must use this method and never the + * content router with a logical Site Search name. + * + *

The {@code .os} tag never crosses this boundary: it is applied only inside the OpenSearch + * adapter for the lookup and stripped back off the resolved value, so both the alias keys and the + * index values returned here are logical and directly comparable against {@link #listIndices()} + * output.

+ * + *

Why dual-write phases cannot collide the map

+ * In Phases 1/2 the ES twin ({@code xxx}) and the OpenSearch twin ({@code xxx.os}) carry the + * same alias, so a naive ES∪OS merge would map one alias key to two different index + * values and silently drop one. This method avoids that by resolving against a single + * engine — the current phase's read provider (ES in Phases 0/1, OS in + * Phases 2/3), never a union. The two twins never land in the same map: Phase 1 returns + * {@code {lol=xxx}} from ES; Phase 2 returns {@code {lol=xxx}} from OS ({@code xxx.os} with the + * tag stripped). Because both twins share the same logical base, a synchronized cluster resolves + * the alias to the same logical name in every phase. + * + *

Two residual edges, both benign here:

+ *
    + *
  1. Multi-index alias within one engine (one alias pointing at two indices on + * the same provider) would lose one entry to the reverse-map — but that state is prevented + * upstream by the {@code createAlias} existence check (issue #36360). A healthy cluster has + * one index per alias per engine.
    2. + *
  2. Mirror desync (the ES and OS aliases point at different logical + * indices) makes the result diverge by phase — which is correct, since you resolve against + * the engine you read from; it is a mirror-reconciliation concern, not a collision.
    3. + *
+ * + * @return map of logical alias name to logical index name; empty when nothing resolves + */ + Map getAliasToIndexMap(); + + /** + * Alias resolution for management and display — the same map as + * {@link #getAliasToIndexMap()} but covering every index the current phase lists, not only + * those on the read provider. + * + *

Why a second method instead of changing the first

+ * {@link #listIndices()} is a union of both engines in the dual-write phases, while + * {@link #getAliasToIndexMap()} resolves against a single engine (the read provider). Any + * index that lives only on the other engine therefore appears in the list with a blank alias: + * + *
    + *
  • Phase 2 + an index created in Phase 0 (Elasticsearch only) — reads come from + * OpenSearch, so its alias is invisible.
    • + *
  • Phase 1 + an index created in Phase 3 (OpenSearch only, e.g. after a downgrade) — + * reads come from Elasticsearch, so its alias is invisible.
    • + *
+ * + * The two are mirror images of one defect (issue #36983). This method closes it by resolving over + * the same provider set {@code listIndices()} uses, so every listed index can show its alias. + * + *

The distinction is deliberate and must be kept: searching resolves an alias + * against the engine that will actually serve the query — that is {@link #getAliasToIndexMap()} and + * it stays single-engine. Managing (listing indices, choosing one to crawl, + * labelling a row in the portlet) needs to identify everything on screen, which is this method.

+ * + *

When both engines resolve the same alias to different logical indices — a mirror desync — the + * read provider's answer wins, so the map never disagrees with what a search would do.

+ * + * @return map of logical alias name to logical index name across the phase's provider set; empty + * when nothing resolves + */ + default Map getAliasToIndexMapAllEngines() { + // A single-engine implementation (either leaf) has nothing to merge — only the router overrides. + return getAliasToIndexMap(); + } + + /** + * The site-search index currently marked as the default, resolved phase-aware. + * + *

Which store holds that pointer changes with the phase: Elasticsearch owns it in Phases 0/1 + * (the legacy {@code indicies} row), OpenSearch in Phases 2/3 ({@code VersionedIndices}, falling + * back to the legacy row when its slot was never populated). Reading the legacy row directly — + * {@code IndiciesInfo#getSiteSearch()} — is therefore correct only up to Phase 1: from Phase 2 on, + * {@code activateIndex} fans out to OpenSearch alone in Phase 3, so the legacy pointer freezes at + * whatever was default in the Elasticsearch era and every screen reading it shows a stale default + * (issue #36983).

+ * + *

Callers that only need to test one name should use {@link #isDefaultIndex(String)}, which is + * defined in terms of this. Callers that need the name itself — preselecting it in a dropdown, + * listing the non-default indices — use this one, and get an empty {@link Optional} instead of a + * {@code null} to dereference when no default has ever been set.

+ * + * @return the logical name of the default index, or empty when there is none + * @throws DotDataException if the pointer store cannot be read + */ + Optional defaultIndexName() throws DotDataException; + + /** + * This basically tells you if the index passed as parameter is the default site search index or not + * @param indexName + * @return + * @throws DotDataException + */ + boolean isDefaultIndex(String indexName) throws DotDataException; + + void activateIndex(String indexName) throws DotDataException; + + void deactivateIndex(String indexName) throws DotDataException, IOException; + + boolean createSiteSearchIndex(String indexName, String alias, int shards) throws DotSearchException, IOException; + + boolean setAlias(String indexName, final String alias); + + List getTasks() throws SchedulerException; + + void deleteTask(String taskName) throws SchedulerException; + + void scheduleTask(SiteSearchConfig config) throws SchedulerException, ParseException, ClassNotFoundException; + + void putToIndex(String idx, SiteSearchResult res, String resultType); + + void putToIndex(String idx, List res, String resultType); + + void deleteFromIndex(String idx, String docId); + + SiteSearchResults search(String query, int start, int rows); + + SiteSearchResults search(String indexName, String query, int start, int rows); + + ScheduledTask getTask(String taskName) throws SchedulerException; + + void pauseTask(String taskName) throws SchedulerException; + + SiteSearchPublishStatus getTaskProgress(String jobName) throws SchedulerException; + + boolean isTaskRunning(String jobName) throws SchedulerException; + + void executeTaskNow(SiteSearchConfig config) throws SchedulerException, ParseException, ClassNotFoundException; + + SiteSearchResult getFromIndex(String index, String id); + + Map getAggregations(String indexName, String query) throws DotDataException; + + /*** + * @deprecated use getAggregations instead + */ + @Deprecated + Map getFacets(String indexName, String query) throws DotDataException; + + List listClosedIndices(); + + public void deleteOldSiteSearchIndices(); + + /** + * Deletes a single site-search index by name from every engine that holds it (ES and, during a + * migration, its OpenSearch counterpart), mirroring the operator's single-index view. The + * active (default) site-search index cannot be deleted — deactivate it first. + * + * @param indexName the site-search index name (must be a {@code sitesearch_*} name) + * @throws DotDataException if the name is not a site-search index or the delete fails + * @throws IOException on an index-engine error + */ + void deleteIndex(String indexName) throws DotDataException, IOException; +} diff --git a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search.jsp b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search.jsp index 2f7ac0a6c1b8..2d4d8ea17744 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search.jsp @@ -570,9 +570,13 @@ function submitSchedule() { //Based on the error invalid_alias_name_exception returned by the ES //Alias must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?]"}] + //The upper bound is 255 (the engine's index/alias name limit), not 60: this field also accepts a + //raw index name for indices that carry no alias, and a crawl-built name + //(sitesearch__) is 62 chars — a 60-char cap made those indices impossible to + //schedule at all (issue #36983). let indexAlias = dojo.byId("indexAlias").value.trim(); indexAlias = indexAlias.replace(/\s/g, ''); - let aliasTestResult = /^(?=.{3,60}$)^(?![-_])[a-zA-Z0-9_()-]+$/.test(indexAlias); + let aliasTestResult = /^(?=.{3,255}$)^(?![-_])[a-zA-Z0-9_()-]+$/.test(indexAlias); if(!aliasTestResult) { showDotCMSErrorMessage("<%= UtilMethods.escapeSingleQuotes(LanguageUtil.get(pageContext, "Invalid-Index-Alias")) %>"); diff --git a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search_index_stats.jsp b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search_index_stats.jsp index 88453690bd6f..3d406aca7e28 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search_index_stats.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/site_search_index_stats.jsp @@ -49,8 +49,11 @@ Map indexInfo = esapi.getIndicesStats(); // Site-search .os-aware alias resolution (issue #36360): resolve through the site-search API and // reverse (alias->index) into index->alias for per-row display. The content-index router (esapi) // misses site-search aliases in Phases 2/3 because it queries OpenSearch without the .os tag. +// AllEngines (issue #36983): the rows below come from listIndices(), a union of both engines in the +// dual-write phases, so a read-provider-only alias map blanks the Alias column for every index that +// lives on the other engine (a Phase-0 index seen in Phase 2, a Phase-3 index seen in Phase 1). Map alias = new java.util.HashMap<>(); -for (Map.Entry aliasEntry : ssapi.getAliasToIndexMap().entrySet()) { +for (Map.Entry aliasEntry : ssapi.getAliasToIndexMapAllEngines().entrySet()) { alias.put(aliasEntry.getValue(), aliasEntry.getKey()); } SimpleDateFormat dater = APILocator.getContentletIndexAPI().timestampFormatter; @@ -58,6 +61,11 @@ SimpleDateFormat dater = APILocator.getContentletIndexAPI().timestampFormatter; Map map = esapi.getClusterHealth(); +// Phase-aware default (issue #36983): the legacy IndiciesInfo pointer freezes at the Elasticsearch-era +// default from Phase 3 on, where activateIndex fans out to OpenSearch alone. Resolved once for every row. +String defaultSiteSearchIndex = null; +try { defaultSiteSearchIndex = ssapi.defaultIndexName().orElse(null); } catch (Exception e) { Logger.warn(this.getClass(), "Could not resolve the default site-search index: " + e.getMessage()); } + %> @@ -128,7 +136,7 @@ Map map = esapi.getClusterHealth(); <% ClusterIndexHealth health = map.get(x); if (health == null) { health = map.get(IndexTag.OS.tag(x)); } %> <% IndexStats status = indexInfo.get(x); if (status == null) { status = indexInfo.get(IndexTag.OS.tag(x)); } %> - <%boolean active =x.equals(info.getSiteSearch());%> + <%boolean active = x.equals(defaultSiteSearchIndex);%> <% Date d = null; String myDate = null; try{ @@ -197,7 +205,7 @@ Map map = esapi.getClusterHealth(); <%-- RIGHT CLICK MENUS --%> <%for(String x : indices){%> - <%boolean active =x.equals(info.getSiteSearch());%> + <%boolean active = x.equals(defaultSiteSearchIndex);%> <%ClusterIndexHealth health = map.get(x); %>
<%@page import="com.dotmarketing.beans.Host"%> <%@page import="com.dotmarketing.business.APILocator"%> <%@page import="com.dotmarketing.portlets.languagesmanager.model.Language"%> @@ -16,9 +15,18 @@ if(request.getParameter("jobName") != null){ } } -ESIndexAPI iapi=new ESIndexAPI(); List indexes = ssapi.listIndices(); -Map alias = iapi.getIndexAlias(indexes); +// Site-search .os-aware alias resolution (issue #36983): resolve through the site-search API and +// reverse (alias->index) into index->alias for the selector. The content-index router (ESIndexAPI) +// misses site-search aliases in Phases 2/3 because the physical OpenSearch index is .os-tagged, so +// the selector fell back to the raw internal index name — which is then saved as the job's +// `indexAlias` and later re-applied as the new index's alias by the crawl, destroying the real one. +// AllEngines: this list is a union of both engines, so the alias view must cover the same set or an +// index living only on the non-read engine renders (and gets saved) as a raw name again. +Map alias = new HashMap(); +for (Map.Entry aliasEntry : ssapi.getAliasToIndexMapAllEngines().entrySet()) { + alias.put(aliasEntry.getValue(), aliasEntry.getKey()); +} SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd"); SimpleDateFormat tdf = new SimpleDateFormat("HH:mm:ss"); @@ -66,7 +74,10 @@ for(String x : indexHosts){ catch(Exception e){} } -boolean hasDefaultIndex = APILocator.getIndiciesAPI().loadIndicies().getSiteSearch() != null; +// Phase-aware default (issue #36983) — see the note in site_search_index_stats.jsp. +String defaultSiteSearchIndex = null; +try { defaultSiteSearchIndex = ssapi.defaultIndexName().orElse(null); } catch (Exception e) { Logger.warn(this.getClass(), "Could not resolve the default site-search index: " + e.getMessage()); } +boolean hasDefaultIndex = defaultSiteSearchIndex != null; List langs=APILocator.getLanguageAPI().getLanguages(); @@ -77,7 +88,7 @@ String includeExclude = (String) props.get("includeExclude") ==null ? "all": (St boolean hasPath = false; -final String siteSearch = APILocator.getIndiciesAPI().loadIndicies().getSiteSearch(); +final String siteSearch = defaultSiteSearchIndex; %> diff --git a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/test_site_search.jsp b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/test_site_search.jsp index 3282079e853f..312316b43cff 100644 --- a/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/test_site_search.jsp +++ b/dotCMS/src/main/webapp/html/portlet/ext/sitesearch/test_site_search.jsp @@ -19,7 +19,12 @@ IndiciesInfo info=APILocator.getIndiciesAPI().loadIndicies(); -String testIndex = (request.getParameter("testIndex") == null) ? info.getSiteSearch() : request.getParameter("testIndex"); +// Phase-aware default (issue #36983): from Phase 3 on, activateIndex fans out to OpenSearch alone, so +// the legacy IndiciesInfo pointer freezes at the Elasticsearch-era default and would preselect a stale +// index here. Resolved once and reused below for the "(Default)" marker. +String defaultSiteSearchIndex = null; +try { defaultSiteSearchIndex = ssapi.defaultIndexName().orElse(null); } catch (Exception e) { Logger.warn(this.getClass(), "Could not resolve the default site-search index: " + e.getMessage()); } +String testIndex = (request.getParameter("testIndex") == null) ? defaultSiteSearchIndex : request.getParameter("testIndex"); String testQuery = request.getParameter("testQuery"); @@ -56,7 +61,16 @@ try { List indices=ssapi.listIndices(); -Map alias=esapi.getIndexAlias(indices); +// Site-search alias resolution for the index selector (issue #36983): the content-index router +// (esapi) queries OpenSearch without the .os tag, so it resolves nothing in Phases 2/3 and the +// dropdown showed raw internal index names instead of the aliases operators know. Resolve through +// the site-search API, over the same provider set listIndices() uses — the list is a union of both +// engines, so a read-provider-only map would still blank the label of any index living on the other +// engine. Only the LABEL uses the alias; the option value stays the index name the search needs. +Map alias=new HashMap(); +for (Map.Entry aliasEntry : ssapi.getAliasToIndexMapAllEngines().entrySet()) { + alias.put(aliasEntry.getValue(), aliasEntry.getKey()); +} Map indexInfo = esapi.getIndicesStats(); SimpleDateFormat dater = APILocator.getContentletIndexAPI().timestampFormatter; @@ -108,7 +122,7 @@ dojo.connect(dijit.byId("testQuery"), 'onkeypress', function (evt) { 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 f0dfebf818a1..970a7c06f6d8 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 @@ -2,14 +2,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import com.dotcms.UnitTestBase; import com.dotcms.content.elasticsearch.business.IndiciesInfo; +import com.dotcms.content.index.ContentletIndexOperations; +import com.dotcms.content.index.migration.ContentIndexMirrorReconciler.DatabaseCounts; import com.dotcms.content.index.IndexAPI; +import com.dotmarketing.exception.DotRuntimeException; import com.dotcms.content.index.domain.IndexStats; import com.dotcms.content.index.migration.MirrorStatus.IndexKind; import com.dotcms.content.index.migration.MirrorStatus.Verdict; @@ -30,29 +35,54 @@ public class ContentIndexMirrorReconcilerTest extends UnitTestBase { private IndexAPI es; private IndexAPI os; + private ContentletIndexOperations esOps; + private ContentletIndexOperations osOps; @Before public void setUp() { es = mock(IndexAPI.class); os = mock(IndexAPI.class); + esOps = mock(ContentletIndexOperations.class); + osOps = mock(ContentletIndexOperations.class); when(es.removeClusterIdFromName(anyString())).thenAnswer(inv -> { final String n = inv.getArgument(0); return n.startsWith(PREFIX) ? n.substring(PREFIX.length()) : n; }); + // Mirror each leaf's physical-name convention: ES cluster-prefixes, OS also tags with .os. + when(esOps.toPhysicalName(anyString())).thenAnswer(inv -> PREFIX + inv.getArgument(0)); + when(osOps.toPhysicalName(anyString())).thenAnswer(inv -> PREFIX + inv.getArgument(0) + ".os"); } - private static IndexStats stats(final long count) { + /** + * A stats entry that marks an index as PRESENT. Its {@code documentCount} is deliberately a poison + * value: existence comes from the stats snapshot but the reported count must come from a live count + * query, because the stats counter trails a just-written document by seconds (issue #36983). If the + * implementation ever reads the count from here again, every assertion below fails loudly instead of + * silently reintroducing the lag. + */ + private static IndexStats present() { final IndexStats s = mock(IndexStats.class); - when(s.documentCount()).thenReturn(count); + when(s.documentCount()).thenReturn(-999L); return s; } + /** Stubs the live count query of one engine leaf for a logical index name. */ + private void count(final ContentletIndexOperations ops, final String logicalName, final long n) { + when(ops.getIndexDocumentCount(ops.toPhysicalName(logicalName))).thenReturn(n); + } + private static IndiciesInfo indicies(final String working, final String live) { return new IndiciesInfo.Builder().setWorking(working).setLive(live).build(); } private ContentIndexMirrorReconciler reconciler(final IndiciesInfo info) { - return new ContentIndexMirrorReconciler(es, os, () -> info); + return reconciler(info, null); + } + + /** @param expected the database denominator behind the coverage percentages, or null when absent */ + private ContentIndexMirrorReconciler reconciler(final IndiciesInfo info, + final DatabaseCounts expected) { + return new ContentIndexMirrorReconciler(es, os, esOps, osOps, () -> info, () -> expected); } /** Both content indices present on both engines with equal counts → two IN_SYNC rows. */ @@ -60,10 +90,12 @@ private ContentIndexMirrorReconciler reconciler(final IndiciesInfo info) { public void workingAndLive_inSync() { // Build the stats maps first: nesting stats() (a when()) inside a when().thenReturn(...) would // trip Mockito's UnfinishedStubbingException. - final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); - final Map osStats = Map.of("working_1.os", stats(100), "live_1.os", stats(50)); + 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); + count(esOps, "working_1", 100); count(osOps, "working_1", 100); + count(esOps, "live_1", 50); count(osOps, "live_1", 50); final List statuses = reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); @@ -85,10 +117,12 @@ public void workingAndLive_inSync() { /** The OpenSearch counterpart of the working index is missing → MISSING_COUNTERPART. */ @Test public void missingOsCounterpart_onWorking() { - final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); - final Map osStats = Map.of("live_1.os", stats(50)); // working_1.os absent + 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); + count(esOps, "working_1", 100); + count(esOps, "live_1", 50); count(osOps, "live_1", 50); final List statuses = reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); @@ -102,13 +136,15 @@ public void missingOsCounterpart_onWorking() { assertTrue(working.recommendation().contains("OpenSearch")); } - /** Counts diverge on the live index (exact stats, no cap) → COUNT_DRIFT. */ + /** Counts diverge on the live index (exact count query, no cap) → COUNT_DRIFT. */ @Test public void countDrift_onLive() { - final Map esStats = Map.of("working_1", stats(100), "live_1", stats(50)); - final Map osStats = Map.of("working_1.os", stats(100), "live_1.os", stats(40)); + 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); + count(esOps, "working_1", 100); count(osOps, "working_1", 100); + count(esOps, "live_1", 50); count(osOps, "live_1", 40); final List statuses = reconciler(indicies(PREFIX + "working_1", PREFIX + "live_1")).statuses(); @@ -131,14 +167,124 @@ public void nullIndicies_emptyList() { /** An unset working/live slot is skipped (no row, no NPE). */ @Test public void unsetSlot_skipped() { - final Map esStats = Map.of("live_1", stats(50)); - final Map osStats = Map.of("live_1.os", stats(50)); + 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); + count(esOps, "live_1", 50); count(osOps, "live_1", 50); final List statuses = reconciler(indicies(null, PREFIX + "live_1")).statuses(); assertEquals(1, statuses.size()); assertEquals(IndexKind.CONTENT_LIVE, statuses.get(0).kind()); } + + /** + * The count is read live, not from the stats snapshot: the stats counter only advances on shard + * refresh, so reading it would report a just-published document as missing for seconds — which a + * support technician reads as a lost write (issue #36983). The stats entries here carry a poison + * count, so this passes only if the reported numbers came from the count query. + */ + @Test + 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); + count(esOps, "working_1", 683); count(osOps, "working_1", 15); + + final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null)).statuses().get(0); + + assertEquals(683, working.es().docCount()); + assertEquals(15, working.os().docCount()); + verify(esOps).getIndexDocumentCount("cluster_x.working_1"); + verify(osOps).getIndexDocumentCount("cluster_x.working_1.os"); + } + + /** + * Coverage is each engine measured against the DATABASE, not against the other engine — the only + * completeness signal that survives into Phase 3, where there is no second engine to diff against + * (issue #36983). The scenario is the one observed live: the content mirror was never rebuilt. + */ + @Test + 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); + count(esOps, "working_1", 686); count(osOps, "working_1", 21); + + final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null), + new DatabaseCounts(686L, 685L)).statuses().get(0); + + assertEquals(Long.valueOf(686), working.databaseDocCount()); + assertEquals(100.0, working.esIndexedPercent(), 0.001); + assertEquals(3.06, working.osIndexedPercent(), 0.001); + // The incomplete copy is named in the recommendation, with the fallout spelled out. + assertTrue(working.recommendation().contains("OpenSearch copy holds 21 of the 686")); + assertTrue(working.recommendation().contains("Site Search crawl")); + assertFalse("the complete copy must not be flagged", + working.recommendation().contains("Elasticsearch copy holds")); + } + + /** No denominator (the query failed, or this is a Site Search row) → the fields are simply absent. */ + @Test + 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); + count(esOps, "working_1", 686); count(osOps, "working_1", 21); + + final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null)).statuses().get(0); + + assertNull(working.databaseDocCount()); + assertNull(working.esIndexedPercent()); + assertNull(working.osIndexedPercent()); + assertFalse(working.recommendation().contains("NOTE")); + } + + /** + * A complete mirror is not annotated, and coverage does not touch the verdict: the verdict states + * the ES↔OS relationship, coverage states completeness against the database. Two separate facts. + */ + @Test + 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); + count(esOps, "working_1", 686); count(osOps, "working_1", 686); + + final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null), + new DatabaseCounts(686L, 685L)).statuses().get(0); + + assertEquals(100.0, working.osIndexedPercent(), 0.001); + assertEquals(Verdict.IN_SYNC, working.verdict()); + assertFalse(working.recommendation().contains("NOTE")); + } + + /** + * A failing count query is reported as {@code -1} (the unmeasurable marker) instead of propagating: + * an "unknown" answer for one engine still leaves a usable report, and -1 compares unequal so the + * verdict degrades to out-of-sync rather than to a false green. + */ + @Test + 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); + count(esOps, "working_1", 683); + when(osOps.getIndexDocumentCount("cluster_x.working_1.os")) + .thenThrow(new DotRuntimeException("OS unreachable")); + + final MirrorStatus working = reconciler(indicies(PREFIX + "working_1", null)).statuses().get(0); + + assertEquals(683, working.es().docCount()); + assertEquals(-1, working.os().docCount()); + assertEquals(Verdict.COUNT_DRIFT, working.verdict()); + assertNull("an unmeasurable count has no drift percentage", working.driftPercent()); + } } 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 new file mode 100644 index 000000000000..76418d7d37fe --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/content/index/migration/SiteSearchMirrorReconcilerTest.java @@ -0,0 +1,141 @@ +package com.dotcms.content.index.migration; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +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.dotmarketing.sitesearch.business.SiteSearchAPI; +import java.util.List; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; + +/** + * Unit tests for the alias half of {@link SiteSearchMirrorReconciler} — how the migration-readiness + * report identifies a site-search index (issue #36983). Both engine leaves are mocked, so no live + * cluster is needed. + * + *

Operators know a site-search index by its alias, never by its {@code sitesearch__} + * name, so the report carries the alias each engine has attached — per engine, because during the + * migration the two sides can legitimately disagree.

+ */ +public class SiteSearchMirrorReconcilerTest extends UnitTestBase { + + private static final String PREFIX = "cluster_x."; + private static final String INDEX = "sitesearch_20260810160529"; + + private SiteSearchAPI es; + private SiteSearchAPI os; + + @Before + 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.existsOnAllWriteEngines(anyString())).thenReturn(true); + when(os.existsOnAllWriteEngines(anyString())).thenReturn(true); + when(es.documentCount(anyString())).thenReturn(10L); + when(os.documentCount(anyString())).thenReturn(10L); + when(es.getAliasToIndexMap()).thenReturn(Map.of()); + when(os.getAliasToIndexMap()).thenReturn(Map.of()); + } + + private SiteSearchMirrorReconciler reconciler() { + return new SiteSearchMirrorReconciler(es, os, () -> PREFIX); + } + + private MirrorStatus onlyStatus() { + final List statuses = reconciler().statuses(); + assertEquals(1, statuses.size()); + return statuses.get(0); + } + + /** The alias each engine holds is reported on that engine's copy. */ + @Test + public void alias_isReportedPerEngine() { + when(es.getAliasToIndexMap()).thenReturn(Map.of("sitesearch-ph-3", INDEX)); + when(os.getAliasToIndexMap()).thenReturn(Map.of("sitesearch-ph-3", INDEX)); + + final MirrorStatus status = onlyStatus(); + + assertEquals("sitesearch-ph-3", status.es().alias()); + assertEquals("sitesearch-ph-3", status.os().alias()); + assertEquals(Verdict.IN_SYNC, status.verdict()); + } + + /** + * An alias present on one engine and absent on the other is exactly the asymmetry an operator + * needs to see before promoting a phase, so it must survive as two distinct values. + */ + @Test + public void alias_missingOnOneEngine_staysVisibleOnTheOther() { + when(es.getAliasToIndexMap()).thenReturn(Map.of("sitesearch-ph-3", INDEX)); + when(os.getAliasToIndexMap()).thenReturn(Map.of()); + + final MirrorStatus status = onlyStatus(); + + assertEquals("sitesearch-ph-3", status.es().alias()); + assertNull(status.os().alias()); + } + + /** An index with no alias anywhere reports none — the field is simply absent from the payload. */ + @Test + public void alias_absentOnBothEngines_isNull() { + final MirrorStatus status = onlyStatus(); + + assertNull(status.es().alias()); + assertNull(status.os().alias()); + assertFalse(status.recommendation().contains("NOTE")); + } + + /** + * An alias that is really an index name is the fingerprint of the overwrite fixed in issue #36983. + * The fix cannot restore an alias already lost, so the report must call it out — while leaving the + * sync verdict alone, since no data is at risk. + */ + @Test + public void aliasShapedLikeAnIndexName_isFlaggedWithoutChangingTheVerdict() { + final String corrupted = "sitesearch_20260806203309"; + when(es.getAliasToIndexMap()).thenReturn(Map.of(corrupted, INDEX)); + when(os.getAliasToIndexMap()).thenReturn(Map.of(corrupted, INDEX)); + + final MirrorStatus status = onlyStatus(); + + assertEquals(corrupted, status.es().alias()); + assertTrue(status.recommendation().contains("is an index name, not a real alias")); + assertTrue(status.recommendation().contains(corrupted)); + assertEquals("A damaged alias must not affect the data-integrity verdict", Verdict.IN_SYNC, + status.verdict()); + assertFalse(status.needsAttention()); + } + + /** A real alias that merely starts with the site-search prefix is NOT mistaken for an index name. */ + @Test + public void aliasStartingWithThePrefix_isNotFlagged() { + when(es.getAliasToIndexMap()).thenReturn(Map.of("sitesearch-ph-3", INDEX)); + when(os.getAliasToIndexMap()).thenReturn(Map.of("sitesearch_prod", INDEX)); + + assertFalse(onlyStatus().recommendation().contains("NOTE")); + } + + /** 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")); + + assertEquals(2, reconciler().statuses().size()); + + verify(es, times(1)).getAliasToIndexMap(); + verify(os, times(1)).getAliasToIndexMap(); + } +} \ No newline at end of file diff --git a/dotCMS/src/test/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchRouterReconciliationTest.java b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchRouterReconciliationTest.java index 7ac7b06e3fed..4dd70697eb6b 100644 --- a/dotCMS/src/test/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchRouterReconciliationTest.java +++ b/dotCMS/src/test/java/com/dotcms/enterprise/publishing/sitesearch/SiteSearchRouterReconciliationTest.java @@ -1,5 +1,6 @@ package com.dotcms.enterprise.publishing.sitesearch; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; @@ -15,6 +16,8 @@ import com.dotmarketing.exception.DotDataException; import com.dotmarketing.sitesearch.business.SiteSearchAPI; import com.dotmarketing.util.Config; +import java.util.Map; +import java.util.Optional; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -233,4 +236,103 @@ public void deleteIndex_activeIndex_isRejectedBeforeAnyDelete() throws Exception verify(esImpl, never()).deleteIndex(IDX); verify(osImpl, never()).deleteIndex(IDX); } + + // ======================================================================= + // getAliasToIndexMapAllEngines — the management/display alias view (#36983) + // ======================================================================= + + private static final String OS_ONLY_IDX = "sitesearch_20260811155758"; + + /** + * The defect: in Phase 1 reads come from Elasticsearch, so an index that lives only on OpenSearch + * (created by a crawl in Phase 3, still listed after a downgrade) had no resolvable alias and the + * portlet rendered it blank. The management view must see both engines. + */ + @Test + public void aliasMapAllEngines_dualWrite_includesTheEngineThePhaseDoesNotReadFrom() { + setPhase(PHASE_1_DUAL_WRITE_ES_READS); + when(esImpl.getAliasToIndexMap()).thenReturn(Map.of("es-alias", IDX)); + when(osImpl.getAliasToIndexMap()).thenReturn(Map.of("os-alias", OS_ONLY_IDX)); + + final Map merged = router.getAliasToIndexMapAllEngines(); + + assertEquals(2, merged.size()); + assertEquals(IDX, merged.get("es-alias")); + assertEquals(OS_ONLY_IDX, merged.get("os-alias")); + } + + /** + * Mirror desync — one alias resolving to different indices on each engine. The read provider wins, + * so the management view never contradicts what a search would actually hit. + */ + @Test + public void aliasMapAllEngines_conflictingAlias_readProviderWins() { + setPhase(PHASE_1_DUAL_WRITE_ES_READS); // reads = ES + when(esImpl.getAliasToIndexMap()).thenReturn(Map.of("shared", IDX)); + when(osImpl.getAliasToIndexMap()).thenReturn(Map.of("shared", OS_ONLY_IDX)); + + assertEquals(IDX, router.getAliasToIndexMapAllEngines().get("shared")); + } + + /** Single-provider phase: nothing to merge, and the idle engine must not be consulted. */ + @Test + public void aliasMapAllEngines_phase0_onlyConsultsEs() { + setPhase(PHASE_0_ES_ONLY); + when(esImpl.getAliasToIndexMap()).thenReturn(Map.of("es-alias", IDX)); + + assertEquals(Map.of("es-alias", IDX), router.getAliasToIndexMapAllEngines()); + verify(osImpl, never()).getAliasToIndexMap(); + } + + /** + * The single-engine view stays single-engine: searches must resolve an alias against the engine + * that will serve the query, so widening this one would be wrong. + */ + @Test + public void aliasMap_singleEngine_staysOnTheReadProvider() { + setPhase(PHASE_1_DUAL_WRITE_ES_READS); // reads = ES + when(esImpl.getAliasToIndexMap()).thenReturn(Map.of("es-alias", IDX)); + + assertEquals(Map.of("es-alias", IDX), router.getAliasToIndexMap()); + verify(osImpl, never()).getAliasToIndexMap(); + } + + // ======================================================================= + // defaultIndexName — which store owns the "default" pointer (#36983) + // ======================================================================= + + private static final int PHASE_3_OS_ONLY = 3; + + /** Phases 0/1: Elasticsearch owns the pointer, so the OpenSearch store is not consulted. */ + @Test + public void defaultIndexName_phase1_comesFromElasticsearch() throws Exception { + setPhase(PHASE_1_DUAL_WRITE_ES_READS); + when(esImpl.defaultIndexName()).thenReturn(Optional.of(IDX)); + + assertEquals(Optional.of(IDX), router.defaultIndexName()); + verify(osImpl, never()).defaultIndexName(); + } + + /** + * Phase 3: OpenSearch owns it. This is the case the fix exists for — {@code activateIndex} fans out + * to OpenSearch alone there, so the legacy Elasticsearch pointer freezes at whatever was default in + * the Elasticsearch era and every screen reading it shows a stale default. + */ + @Test + public void defaultIndexName_phase3_comesFromOpenSearch() throws Exception { + setPhase(PHASE_3_OS_ONLY); + when(osImpl.defaultIndexName()).thenReturn(Optional.of(OS_ONLY_IDX)); + + assertEquals(Optional.of(OS_ONLY_IDX), router.defaultIndexName()); + verify(esImpl, never()).defaultIndexName(); + } + + /** No default set anywhere → empty, never a null to dereference. */ + @Test + public void defaultIndexName_noneSet_isEmpty() throws Exception { + setPhase(PHASE_3_OS_ONLY); + when(osImpl.defaultIndexName()).thenReturn(Optional.empty()); + + assertTrue(router.defaultIndexName().isEmpty()); + } } diff --git a/dotCMS/src/test/java/com/dotcms/publishing/job/SiteSearchJobAliasResolutionTest.java b/dotCMS/src/test/java/com/dotcms/publishing/job/SiteSearchJobAliasResolutionTest.java new file mode 100644 index 000000000000..34c344f4487f --- /dev/null +++ b/dotCMS/src/test/java/com/dotcms/publishing/job/SiteSearchJobAliasResolutionTest.java @@ -0,0 +1,233 @@ +package com.dotcms.publishing.job; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.dotcms.content.elasticsearch.business.IndiciesAPI; +import com.dotcms.content.index.IndexConfigHelper.MigrationPhase; +import com.dotcms.content.index.migration.MirrorStatus; +import com.dotcms.enterprise.publishing.sitesearch.SiteSearchResults; +import com.dotcms.publishing.PublisherAPI; +import com.dotcms.publishing.job.SiteSearchJobImpl.IndexMetaData; +import com.dotmarketing.business.UserAPI; +import com.dotmarketing.portlets.contentlet.business.HostAPI; +import com.dotmarketing.sitesearch.business.SiteSearchAPI; +import com.dotmarketing.sitesearch.business.SiteSearchAuditAPI; +import com.dotmarketing.util.Config; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.Before; +import org.junit.Test; + +/** + * Container-free coverage for how {@link SiteSearchJobImpl} resolves the alias a crawl must end up + * with (issue #36983, Bug 1). + * + *

The job detail stores whatever the Site Search scheduler put in its {@code indexAlias} field, + * and that value is not guaranteed to be an alias: when alias resolution missed on + * OpenSearch (Phases 2/3) the index selector fell back to the raw internal index name and saved it + * there. The alias derived here is the one handed to the publisher, which re-applies it to the newly + * built index after the switch — so carrying a raw index name forward made a dead index's NAME + * become the new index's alias, wiping the alias the user created. These tests pin the resolution + * rules so that cannot come back.

+ */ +public class SiteSearchJobAliasResolutionTest { + + private static final String EXISTING_INDEX = "sitesearch_20260810160529"; + private static final String CUSTOM_ALIAS = "sitesearch-ph-3"; + + private SiteSearchAPI siteSearchAPI; + private SiteSearchJobImpl job; + + @Before + public void setup() { + siteSearchAPI = mock(SiteSearchAPI.class); + when(siteSearchAPI.listIndices()).thenReturn(Collections.singletonList(EXISTING_INDEX)); + when(siteSearchAPI.search(anyString(), anyString(), anyInt(), anyInt())) + .thenReturn(new SiteSearchResults()); + + job = new SiteSearchJobImpl(mock(IndiciesAPI.class), siteSearchAPI, mock(HostAPI.class), + mock(UserAPI.class), mock(SiteSearchAuditAPI.class), mock(PublisherAPI.class)); + } + + /** + * A job saved with the index's real alias keeps behaving exactly as before: the alias travels + * through untouched and resolves to the index it points at. + */ + @Test + public void test_aliasStoredInJobDetail_isCarriedThrough() throws Exception { + when(siteSearchAPI.getAliasToIndexMapAllEngines()).thenReturn(Map.of(CUSTOM_ALIAS, EXISTING_INDEX)); + + final IndexMetaData metaData = job.getIndexMetaData(CUSTOM_ALIAS); + + assertEquals(CUSTOM_ALIAS, metaData.getAlias()); + assertEquals(EXISTING_INDEX, metaData.getIndexName()); + assertFalse(metaData.isNewIndex()); + } + + /** + * A job saved with a RAW INDEX NAME (the Phase 2/3 scheduler fallback) must resolve that index's + * real alias — not hand the raw name over as if it were one. Handing it over is what replaced the + * user's alias with a timestamped index name after a crawl (issue #36983). + */ + @Test + public void test_rawIndexNameStoredInJobDetail_resolvesTheIndexRealAlias() throws Exception { + when(siteSearchAPI.getAliasToIndexMapAllEngines()).thenReturn(Map.of(CUSTOM_ALIAS, EXISTING_INDEX)); + + final IndexMetaData metaData = job.getIndexMetaData(EXISTING_INDEX); + + assertEquals(CUSTOM_ALIAS, metaData.getAlias()); + assertNotEquals("The raw index name must never be re-applied as an alias", EXISTING_INDEX, + metaData.getAlias()); + assertEquals(EXISTING_INDEX, metaData.getIndexName()); + assertFalse(metaData.isNewIndex()); + } + + /** + * Same fallback, but the index genuinely has no alias: the crawl must end up with NO alias rather + * than one invented from the old index's name. + */ + @Test + public void test_rawIndexNameWithoutAlias_resolvesToNoAlias() throws Exception { + when(siteSearchAPI.getAliasToIndexMapAllEngines()).thenReturn(Collections.emptyMap()); + + final IndexMetaData metaData = job.getIndexMetaData(EXISTING_INDEX); + + assertNull(metaData.getAlias()); + assertEquals(EXISTING_INDEX, metaData.getIndexName()); + } + + /** + * A name that matches neither an alias nor an existing index describes a brand-new index: it is + * kept as the alias to apply at creation time. + */ + @Test + public void test_unknownName_isKeptAsTheAliasOfANewIndex() throws Exception { + when(siteSearchAPI.getAliasToIndexMapAllEngines()).thenReturn(Collections.emptyMap()); + when(siteSearchAPI.listIndices()).thenReturn(Collections.emptyList()); + + final IndexMetaData metaData = job.getIndexMetaData("brand-new-alias"); + + assertEquals("brand-new-alias", metaData.getAlias()); + assertTrue(metaData.isNewIndex()); + } + + // ======================================================================= + // Incomplete-content-index warning (issue #36983) + // ======================================================================= + + /** A content row with the given coverage on each engine. */ + private static MirrorStatus contentRow(final Long expected, final long esCount, + final long osCount) { + return new MirrorStatus("working_1", MirrorStatus.IndexKind.CONTENT_WORKING, + new MirrorStatus.EngineCopy(true, esCount, "cluster_x.working_1"), + new MirrorStatus.EngineCopy(true, osCount, "cluster_x.working_1.os"), + MirrorStatus.Verdict.IN_SYNC, "", expected); + } + + private SiteSearchJobImpl jobSeeing(final MirrorStatus... rows) { + return new SiteSearchJobImpl(mock(IndiciesAPI.class), siteSearchAPI, mock(HostAPI.class), + mock(UserAPI.class), mock(SiteSearchAuditAPI.class), mock(PublisherAPI.class), + () -> List.of(rows)); + } + + /** + * The case this exists for: a Phase-3 crawl reading an OpenSearch content index that was never + * rebuilt. The crawl queries that index to build its corpus, so it can only produce a partial Site + * Search index — and reindexing the content afterwards does not repair it. + */ + @Test + public void test_incompleteContentIndexOnTheReadEngine_isWarnedAbout() { + Config.setProperty(MigrationPhase.FLAG_KEY, "3"); // reads = OpenSearch + try { + final Optional warning = jobSeeing(contentRow(686L, 686, 21)) + .incompleteContentIndexWarning(); + + assertTrue(warning.isPresent()); + assertTrue(warning.get().contains("INCOMPLETE content index")); + assertTrue(warning.get().contains("OpenSearch")); + assertTrue(warning.get().contains("3.06%")); + } finally { + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } + + /** + * The same incomplete OpenSearch copy is NOT warned about in a phase that reads Elasticsearch: the + * crawl will query the complete ES index, so its corpus is fine. Warning there would train + * operators to ignore the message. + */ + @Test + public void test_incompleteCopyOnTheEngineNotBeingRead_isNotWarnedAbout() { + Config.setProperty(MigrationPhase.FLAG_KEY, "1"); // reads = Elasticsearch + try { + assertFalse(jobSeeing(contentRow(686L, 686, 21)) + .incompleteContentIndexWarning().isPresent()); + } finally { + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } + + /** A complete index says nothing. */ + @Test + public void test_completeContentIndex_isNotWarnedAbout() { + Config.setProperty(MigrationPhase.FLAG_KEY, "3"); + try { + assertFalse(jobSeeing(contentRow(686L, 686, 686)) + .incompleteContentIndexWarning().isPresent()); + } finally { + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } + + /** Without a database denominator there is no coverage to judge — silence, not a false alarm. */ + @Test + public void test_noDatabaseDenominator_isNotWarnedAbout() { + Config.setProperty(MigrationPhase.FLAG_KEY, "3"); + try { + assertFalse(jobSeeing(contentRow(null, 686, 21)) + .incompleteContentIndexWarning().isPresent()); + } finally { + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } + + /** The check is advisory: if it cannot be computed, the crawl proceeds silently. */ + @Test + public void test_failureToMeasure_isSwallowed() { + Config.setProperty(MigrationPhase.FLAG_KEY, "3"); + try { + final SiteSearchJobImpl failing = new SiteSearchJobImpl(mock(IndiciesAPI.class), + siteSearchAPI, mock(HostAPI.class), mock(UserAPI.class), + mock(SiteSearchAuditAPI.class), mock(PublisherAPI.class), + () -> { throw new IllegalStateException("cluster down"); }); + + assertFalse(failing.incompleteContentIndexWarning().isPresent()); + } finally { + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } + + /** The threshold is configurable, and 0 disables the check outright. */ + @Test + public void test_thresholdZero_disablesTheCheck() { + Config.setProperty(MigrationPhase.FLAG_KEY, "3"); + Config.setProperty(SiteSearchJobImpl.MIN_CONTENT_INDEXED_KEY, "0"); + try { + assertFalse(jobSeeing(contentRow(686L, 686, 21)) + .incompleteContentIndexWarning().isPresent()); + } finally { + Config.setProperty(SiteSearchJobImpl.MIN_CONTENT_INDEXED_KEY, null); + Config.setProperty(MigrationPhase.FLAG_KEY, null); + } + } +} \ No newline at end of file diff --git a/dotcms-integration/src/test/java/com/dotcms/publishing/job/SiteSearchJobImplTest.java b/dotcms-integration/src/test/java/com/dotcms/publishing/job/SiteSearchJobImplTest.java index 7e588a79fe22..9903022cc043 100644 --- a/dotcms-integration/src/test/java/com/dotcms/publishing/job/SiteSearchJobImplTest.java +++ b/dotcms-integration/src/test/java/com/dotcms/publishing/job/SiteSearchJobImplTest.java @@ -62,6 +62,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; import java.util.concurrent.TimeUnit; @@ -211,6 +212,53 @@ public void Test_Non_Incremental_Create_Default_Index_Run_Non_Incrementally_Expe } + /** + * Given a job whose stored {@code indexAlias} is a RAW INDEX NAME instead of an alias — what the + * Site Search scheduler saved whenever its alias lookup missed on OpenSearch (issue #36983) — + * when a full crawl runs, then the custom alias of that index must survive on the newly built + * index, and the dead index's NAME must never become an alias. + */ + @Test + public void Test_Non_Incremental_Job_Stored_With_Raw_Index_Name_Expect_Custom_Alias_Preserved() + throws DotPublishingException, JobExecutionException, DotDataException, IOException, DotSecurityException { + + deleteAllSiteSearchIndices(); + + final long timeMillis = System.currentTimeMillis(); + final String customAlias = IndexType.SITE_SEARCH.getPrefix() + "-alias-" + timeMillis; + final String originalIndexName = IndexType.SITE_SEARCH.getPrefix() + "_" + timeMillis; + siteSearchAPI.createSiteSearchIndex(originalIndexName, customAlias, 1); + + final String jobId = UUIDUtil.uuid(); + final JobDataMap jobDataMap = new JobDataMap(); + jobDataMap.put(SiteSearchJobImpl.RUN_NOW, Boolean.TRUE.toString()); + jobDataMap.put(SiteSearchJobImpl.INCREMENTAL, Boolean.FALSE.toString()); + // The defect: the index NAME where an alias is expected. + jobDataMap.put(SiteSearchJobImpl.INDEX_ALIAS, originalIndexName); + jobDataMap.put(SiteSearchJobImpl.JOB_ID, jobId); + jobDataMap.put(SiteSearchJobImpl.QUARTZ_JOB_NAME, SiteSearchJobImpl.RUNNING_ONCE_JOB_NAME); + jobDataMap.put(SiteSearchJobImpl.INCLUDE_EXCLUDE, "all"); + jobDataMap.put(SiteSearchJobImpl.LANG_TO_INDEX, new String[]{Long.toString(defaultLang)}); + jobDataMap.put(SiteSearchJobImpl.INDEX_HOST, site.getIdentifier()); + + final JobDetail jobDetail = Mockito.mock(JobDetail.class); + Mockito.when(jobDetail.getJobDataMap()).thenReturn(jobDataMap); + final JobExecutionContext context = Mockito.mock(JobExecutionContext.class); + Mockito.when(context.getJobDetail()).thenReturn(jobDetail); + Mockito.when(context.getFireTime()).thenReturn(new Date()); + new SiteSearchJobImpl().run(context); + + final List recentAudits = siteSearchAuditAPI.findRecentAudits(jobId, 0, 1); + Assert.assertFalse(recentAudits.isEmpty()); + final String newIndexName = recentAudits.get(0).getIndexName(); + + final Map aliasToIndex = siteSearchAPI.getAliasToIndexMap(); + Assert.assertEquals("The custom alias must follow the crawl onto the new index", + newIndexName, aliasToIndex.get(customAlias)); + Assert.assertFalse("The name of the replaced index must never become an alias", + aliasToIndex.containsKey(originalIndexName)); + } + @Test public void Test_Non_Incremental_Create_Default_Index_Create_Second_Index_Run_Non_Incrementally_Expect_Non_Default_New_Index() throws DotPublishingException, JobExecutionException, DotDataException, IOException, DotSecurityException {