fix(sitesearch): preserve the custom index alias across a crawl (#36983)

[fabrizzio-dotCMS]

Proposed Changes

Fixes Bug 1 of #36983 (QA-G17) — the custom alias of a Site Search index destroyed by a crawl in the OpenSearch read phases — plus the two defects that surfaced while diagnosing it: alias resolution that goes blind in the phase it does not read from, and a readiness report that could not answer the questions support was actually asking it.

---

1 · Bug 1 — the alias is overwritten by a crawl

Root cause. site_search_job_schedule.jsp resolved index aliases through the content-index router (new ESIndexAPI()), which is not site-search .os-aware. In Phases 2/3 the physical index lives in OpenSearch tagged with .os, so the lookup misses and the index selector falls back to the raw internal index name — which is then saved as the job's indexAlias. From there the crawl does the damage: it deletes the old index (taking the real alias with it) and re-applies the job's stored string as the new index's alias, so a dead index's NAME becomes the alias. That is the sitesearch-ph-3 → sitesearch_20260810160529 swap QA reported. #36797 rerouted the Indices tab and 6 other callers to the phase-aware API but missed this JSP.

• site_search_job_schedule.jsp — resolve aliases via the phase-aware, .os-aware Site Search API, as the Indices tab already does. Dropped the now-unused ESIndexAPI import.
• SiteSearchJobImpl — when the stored indexAlias is actually a raw index name, recover that index's real alias (or none) instead of carrying the raw name forward to the publisher. Server-side guard, so jobs already saved with a raw name are repaired on their next run rather than needing to be recreated by hand.
• site_search.jsp — the scheduler's alias field now accepts up to 255 chars (the engine's name limit) instead of 60. A crawl-built name (sitesearch_<timestamp>_<uuid>) is 62 chars, so the old cap rejected it with "Invalid Index alias" and left the index permanently un-schedulable.

2 · The aggregated alias view — Bug 3, and the downgrade case

Testing a downgrade (3 → 2 → 1) surfaced the structural half of the same defect: listIndices() is a union of both engines in the dual-write phases, while getAliasToIndexMap() resolves against a single engine (the read provider). Any index living only on the other engine appears in the list with a blank alias — which is Bug 3, in both of its reported forms:

• Phase 2 + an index created in Phase 0 (Elasticsearch only) → alias invisible.

• Phase 1 + an index created in Phase 3 (OpenSearch only, after a downgrade) → alias invisible.

• SiteSearchAPI#getAliasToIndexMapAllEngines() — resolves over the same provider set listIndices() uses, with the read provider applied last so it wins a mirror desync and the management view never contradicts what a search would hit. Single-provider phases (0 and 3) do not consult the idle engine.

• The single-engine getAliasToIndexMap() stays as is — searching must resolve an alias against the engine that will serve the query. The split is documented so the two do not get merged later.

• Switched to the new view: the Indices tab, the crawl index selector, the Search tab selector (so Bug 2's symptom — raw index IDs in the dropdown — is gone too) and SiteSearchJobImpl, where an invisible alias made the crawl treat an existing index as new and drop its alias.

3 · Making the readiness endpoint answer support's questions

The endpoint is the source of truth support consults during a migration. Three gaps, each found by using it against a live stack:

• Site Search rows now carry the alias, per engine. An operator knows these indices by alias, never by sitesearch_<timestamp>_<uuid>. Per engine on purpose: an index can hold its alias on one side and not the other, and that asymmetry is what must be visible before promoting a phase. An alias that is itself shaped like an index name gets a NOTE — the fingerprint of the defect fixed above, which cannot be repaired retroactively, so this is the only way to find the indices that still need theirs restored.

• Content counts come from a count query, not _stats docs.count. That counter is per-shard and only advances on shard refresh, so it trailed a just-written document by seconds: a technician checking whether a publish reached OpenSearch read the previous number and concluded the write was lost. A source of truth must never report a number the engine can already contradict. getIndicesStats() stays, but only to decide existence — one call per engine over the whole set, so an absent index is never confused with an unreachable engine. Both halves of the report now count the same way.

• New: content completeness against the DATABASE (databaseDocCount, esIndexedPercent, osIndexedPercent). Every completeness signal until now compared one engine against the other, which stops being an answer exactly where it matters most: in Phase 3 there is no second engine, so a mirror that was never rebuilt reads unremarkably. The case that prompted this reads 3.06 instead of looking normal.

  The denominator is an exact COUNT, one statement returning both the working and live figures from contentlet_version_info. That is a Parallel Seq Scan — measured at 15 ms for 171k rows, 21 ms for 394k, 22 ms for 453k, so roughly 10 ms of fixed parallel-scan setup plus ~25 ns per row. An earlier revision used the pg_class.reltuples estimate instead, which is O(1) but drifts a few points between ANALYZE runs; that surfaced as an indexed percentage slightly over 100%, which reads as a defect in a report whose whole job is to be trusted. With exact counts, 100% means complete and any excess is real — documents in the index that no longer exist in the database. Splitting the statement in two was also measured and is slower (41 ms vs 28 ms): the live half stays a sequential scan regardless, since nearly every row has a live version.

  Completeness never changes the verdict: the verdict states the ES↔OS relationship, this states completeness against the database. Two facts, reported side by side.

4 · Review follow-ups

• The crawl's incomplete-content-index warning is now opt-in and off the global lock (@ihoffmann-dot's finding). It measured on every crawl from inside prepareJob, which holds a monitor on SiteSearchJobImpl.class — a JVM-wide lock every concurrent crawl on the node contends for — and each measurement costs one sequential scan of contentlet_version_info plus six engine round-trips (two getIndicesStats, four document counts). Two changes:
  • Moved to run(), between prepareJob and the publish loop. Nothing in the measurement depends on what prepareJob computes — it reads the content indices, not this job's — so it had no reason to hold that lock. Still before the crawl it warns about.
  • SITE_SEARCH_CRAWL_MIN_CONTENT_INDEXED_PERCENT now defaults to 0 (off); set it to 95 to get the previous behaviour while investigating. It buys a diagnostic, not behaviour — the crawl runs identically either way — so a recurring cost on every install's crawl schedule was the wrong trade for a message only someone diagnosing a migration reads. A second guard skips Phase 0 even when enabled: with no second engine there is no mirror that could have been left behind. Both guards are plain config reads, before anything touches the database or an engine.
• site_search_job_schedule.jsp was missing its com.dotmarketing.util.Logger import (@claude's review). The scriptlet added in section 1 calls Logger.warn, and neither the JSP nor init.jsp imported it — JSPs translate on first request, so this would have failed to compile and broken the Schedule Site Search Job dialog, the very screen this PR fixes. Its two sibling JSPs already carried the import.
• Semgrep CUSTOM_INJECTION-2 on ContentIndexMirrorReconciler — false positive, and now structurally evident: the statement is a fully literal constant with no interpolation, no parameters and nothing caller-supplied.

5 · Second review pass, before merge

A full re-review of the branch turned up six more, all fixed here. The first one is the reason this did not merge on the previous round.

• The Bug-1 repair only survived one run — and the second run recreated the damage. getIndexMetaData recovered the index's real alias when the job detail held a raw index name, but nothing wrote the recovery back, and the job detail is the job's only memory between runs. So the crawl that used the recovered alias also deleted the index whose name was stored. On the next run that string resolved to neither an alias nor an existing index, the job concluded it was brand new, and: (a) stamped the dead index's name onto the new index as its alias — exactly the defect this PR exists to fix, recreated by the guard meant to prevent it; and (b) abandoned the index built on the previous run, which keeps the user's real alias and silently stops being crawled, because switchIndex skips its whole body when there is no old index to swap from. Net effect: the custom alias survives, pointing at an index that quietly goes stale, while the job crawls a different one under a machine-generated name. Fixed in two halves so neither can return alone — persistResolvedAlias writes the recovery into the job detail (SiteSearchJobProxy is a Quartz StatefulJob over a JDBC store, so the data map is re-persisted after execution), and aliasForNewIndex refuses to adopt a sitesearch_<timestamp>_<uuid> string as a new index's alias, which also covers a job nobody ever repaired. The shape check is the one the readiness report already used to flag these, now shared as SiteSearchMirrorReconciler.isIndexNameShaped rather than restated, so detector and preventer cannot drift.
• An unmeasurable count could read as IN_SYNC. A failed count query is reported as -1, and verdictFor relied on that comparing unequal. It does not when both engines fail: -1 == -1 looked like a perfect match, so the report answered "in sync / safe to advance" about a mirror it never measured. No outage needed — a search user granted indices:monitor/stats but not indices:data/read/count resolves existence and fails counting. Now checked explicitly, so unknown degrades to drift. Kept as COUNT_DRIFT rather than a new Verdict constant: the enum is serialized in the endpoint's JSON and documented, and verdictFor's javadoc already promised this behaviour — only the both-failed case escaped it.
• An absent copy reported esIndexedPercent: 0.0. Zero says "this engine lost all its content"; the truth is "there is no such index here", which exists and MISSING_COUNTERPART already state. It would have lit up every content row in Phase 3, where the Elasticsearch copies are legitimately gone — in the report this PR frames as support's source of truth. Now omitted.
• The opt-in diagnostic failed silently. No onFailure on its Try, so an operator who switched the check on specifically to diagnose a migration could not tell "the index is complete" from "the measurement blew up". Every other quiet-failure path added here already logged.
• SiteSearchAPI.java had been rewritten CRLF → LF end to end, turning a 58-line addition into a 474-line diff and taking the whole interface's git blame with it. Original endings restored; the diff is now 58 insertions, 0 deletions.
• Left as-is: with the check enabled, ContentIndexMirrorReconciler.statuses() still calls getIndicesStats() on both engines, so a Phase-3 crawl pays an Elasticsearch round-trip. Bounded by the opt-in default and by the client's own timeouts, and not worth an engine-conditional branch in a diagnostic.

Checklist

• Tests
• Translations
• Security Implications Contemplated (none — alias resolution and reporting only; the readiness endpoint keeps its existing @Hidden + admin-plus-migration-role gate and is absent from the OpenAPI schema, so no regeneration applies; the new SQL is a fixed literal COUNT over contentlet_version_info with no parameters and no user input)

Additional Info

Tests — 97/97 green

Every guard added in sections 4 and 5 was verified by removing it and watching the matching test fail, then restoring it. A test that cannot fail is not evidence, and two of these are the kind that pass by accident: the opt-in guards sit behind a vavr Try that swallows Throwable, so an assertion thrown from the supplier would be absorbed and the test would go green against the very regression it guards — they assert the supplier was never consulted instead.

• SiteSearchJobAliasResolutionTest (new, unit) — 18 cases. Four pin the alias-resolution rules (real alias carried through, raw index name → real alias, raw index name with no alias → no alias, unknown name → new-index alias). Six cover the two halves of the section-5 fix: the recovered alias is written back to the job detail, an already-correct alias is not rewritten, an index with no alias leaves the job detail alone, a stale sitesearch_<timestamp> is refused as a new index's alias, a real alias is still adopted, and an existing index gets none at creation time. The rest cover the incomplete-content-index warning and its two guards. Note test_unknownName_isKeptAsTheAliasOfANewIndex passes unchanged — a name a person chose is unaffected; only the machine-generated shape is refused.
• ContentIndexMirrorReconcilerTest (existing) — 7 new cases: the count comes from the query and not from stats (the stats entries carry a poison count, so a regression fails loudly), a failed count reported as -1, both counts failing never reading as IN_SYNC, an absent copy reporting no indexed percentage, completeness against the database, completeness absent without a denominator, and a complete mirror not flagged.
• SiteSearchRouterReconciliationTest (existing) — 4 new cases on the aggregated view: it includes the engine the phase does not read from, the read provider wins a conflicting alias, Phase 0 never consults OpenSearch, and the single-engine method stays on the read provider.
• SiteSearchMirrorReconcilerTest (new, unit) — 6 cases: alias per engine, alias missing on one engine only, no alias at all, index-name-shaped alias flagged without changing the verdict, a legitimate sitesearch-prefixed alias NOT flagged, one alias lookup per engine.
• MigrationReadinessServiceTest, MigrationReadinessResourceTest, OSSiteSearchAliasMapTest, ESIndexHelperTest — unchanged and green.
• SiteSearchJobImplTest (existing IT, MainSuite1a) — new end-to-end case: after a full crawl the custom alias must follow onto the new index, and the replaced index's name must never become an alias.

Validation run

• dotcms-core install: BUILD SUCCESS · dotcms-integration test-compile: BUILD SUCCESS
• Verified live against a Phase-1 stack: dual-write confirmed document by document on both engines, and the endpoint's numbers reconciled against _count on each cluster.
• The new IT was not executed locally (needs the integration stack); it runs in CI via MainSuite1a.
• Unit runs use -Dmaven.build.cache.enabled=false and are counted from the Tests run: line — the build cache will otherwise skip surefire entirely and still report BUILD SUCCESS.

Docs (OPENSEARCH_MIGRATION.md)

• The two alias views and when to use each; a phase change never builds counterparts retroactively.
• How to read the readiness report: the access gate (CMS admin and the OS_MIGRATION_INDEX_VISIBILITY_ROLE_KEY role, default os_migration_qa — 403 otherwise), the order to read the fields in, a per-field table, and what a count cannot tell you (re-publishing is an update, and a dual-write mirror only receives what changes from that point on).
• Two worked examples: the downgrade case, and activating a pre-migration backup content index — whose OpenSearch counterpart is never built, is silent in Phase 1, masked by the read fallback in Phase 2 and customer-visible as lost content in Phase 3, and which only a full reindex repairs.
• A consolidated configuration table under the Executive Summary: every setting that changes migration behaviour was documented inline in whichever section happened to use it — the phase flag near the top, DOTCMS_SHADOW_WRITE_LOG_LEVEL in a gotcha 1200 lines down, the readiness role key in the endpoint section — so nothing answered "what can I configure for this migration?". All four, with their shipped defaults verified against the code and a link to the section explaining each.
• The Site Search crawl inherits the content index. Found during this work: a Phase-3 crawl produced an index with 14 documents instead of ~443, because the bundlers build their corpus from conAPI.searchIndex — a phase-routed search over the content index, which held 21 of 685 live documents on OpenSearch. The crawl reports success, the bundlers swallow search failures at debug level, and the damage outlives its cause: reindexing afterwards does not repair the Site Search index already built from the empty one. States the ordering rule this implies — in Phases 2/3, full content reindex first, Site Search crawl second.

Scope
Bug 1 in full. Bugs 2 and 3 turned out to be the same root cause seen from other screens, and the aggregated alias view fixes their symptoms as well. They are left open in the issue for QA to re-verify against a build rather than closed from here.

Follow-ups not included here

• A guard on ESSiteSearchPublisher.switchIndex: it currently activates the new index and deletes the old one unconditionally, so a truncated crawl destroys the good copy — with no ES copy to recover from in Phase 3. With the completeness metric now available, that guard has a defensible criterion instead of an invented threshold. Worth its own PR.
• The whole crawl runs inside one open transaction. Noticed while measuring the above: SiteSearchJobImpl.run() calls HibernateUtil.startTransaction() and there is no matching commitTransaction() anywhere — the session is closed in the finally, so the transaction spans the entire publish loop, which is minutes. A long-lived transaction holds back xmin and delays vacuuming database-wide, and pins a pool connection for the duration. Entirely pre-existing and untouched here, but it dwarfs any query this PR adds, and it is the thing actually worth looking at if crawl DB impact is a concern.

🤖 Generated with Claude Code

This PR fixes: #36983

This PR fixes: #36983

[claude]

Claude finished @fabrizzio-dotCMS's task in 4m 42s —— View job

---

PR Review — fix(sitesearch): preserve the custom index alias across a crawl (#36983)

Reviewed the full diff against origin/main (base 966e9d5c): the two alias-resolution methods, SiteSearchJobImpl, both mirror reconcilers, MirrorStatus, the four JSPs, and the readiness endpoint.

New Issues

No blocking issues found. The core fixes hold up:

• SiteSearchJobImpl.getIndexMetaData (L541–566) — the raw-index-name → real-alias recovery (aliasOf) is correct, and resolvedAlias is threaded consistently into newAlias, the log line, and config.setIndexAlias(...). The new-index case still carries the requested alias forward.
• SiteSearchAPIImpl.getAliasToIndexMapAllEngines (L216–226) — union over writeProviders() with readProvider() applied last is sound; the single-provider short-circuit for phases 0/3 is right. getAliasToIndexMap() correctly left single-engine for search.
• incompleteContentIndexWarning (L459–476) — readsOpenSearch = phase.isReadEnabled() matches the enum (true for Phase 2/3), threshold/phase guards are correct, and construction + measurement are fully wrapped in Try, so a bean-lookup or DB failure can never break a crawl. Advisory-only, as documented.
• ContentIndexMirrorReconciler — countQuietly returns -1 on failure (degrades verdict, never a false green); incompleteNote/indexedPercentOf guard null/≤0/-1 denominators. Existence still from one stats snapshot per engine, count from a live query — the intended split.
• JSPs — getAliasToIndexMapAllEngines() + phase-aware defaultIndexName().orElse(null) are used consistently; each resolution is wrapped in try/catch with a Logger.warn, degrading to "no default marker" rather than failing the page. HashMap/Map.Entry are already in scope in each file.

Resolved

• ✅ ContentIndexMirrorReconciler.java:247 — Semgrep CUSTOM_INJECTION-2 (6 findings). The count query is now a fully literal constant (DATABASE_COUNTS_SQL), no interpolation or caller input. Correctly addressed in 57b10b8.
• ✅ SiteSearchJobImpl.java — @ihoffmann-dot's concern that the content-index check ran on every crawl inside the prepareJob SiteSearchJobImpl.class monitor. Now moved off the lock (runs after prepareJob returns, before the publish loop) and made opt-in via SITE_SEARCH_CRAWL_MIN_CONTENT_INDEXED_PERCENT (default 0 = off), so no install pays the seq-scan + per-engine count cost by default.

Notes (non-blocking, no action required)

• The coverage denominator (COUNT(*) / COUNT(live_inode) on contentlet_version_info) assumes that table's row grain matches the index document grain. The author reports it validated exactly (686/685) on a live stack, and the metric is explicitly order-of-magnitude (distinguishing 3% from 97%), so any small drift is within its stated tolerance.
• test_site_search.jsp still declares IndiciesInfo info at the top; after this change its getSiteSearch() use is gone. Harmless if now unused — worth a glance but not a bug.

Overall: a well-scoped, well-tested change. Error paths are handled defensively (everything on the diagnostic paths is swallowed and logged, never propagated to indexing), the alias-resolution split is clearly documented, and the two prior review concerns are addressed. LGTM.

· branch issue-36983-sitesearch-alias-phase-aware

[semgrep-dotcms]

Semgrep found 6 CUSTOM_INJECTION-2 findings:

• dotCMS/src/main/java/com/dotcms/content/index/migration/ContentIndexMirrorReconciler.java
  • L247-253 - Triage
  • L247-252 - Triage
  • L247-251 - Triage
  • L247-250 - Triage
  • L247-249 - Triage
  • L247-248 - Triage

The method identified is susceptible to injection. The input should be validated and properly
escaped.

If this is a critical or high severity finding, please also link this issue in the #security channel in Slack.