fix(os-migration): harden startup validation, add emergency halt path, fix thread-safe formatter

[fabrizzio-dotCMS]

Closes #35631

Summary

• IndexStartupValidator: connectivity failures are now hard errors (throws DotRuntimeException); validateIndexingConfig() returns boolean so callers can react without catching exceptions.
• IndexConfigHelper: added MigrationPhase.reset() and haltMigration() — runtime-only emergency rollback that writes ordinal 0 back to FEATURE_FLAG_OPEN_SEARCH_PHASE without a restart.
• ContentletIndexAPIImpl: calls haltMigration() when startup validation fails; hasEmptyIndices() is now phase-aware (empty OS index in phases 1/2 no longer triggers unintended full ES reindex).
• ESIndexResource: replaces non-thread-safe SimpleDateFormat with immutable DateTimeFormatter to prevent corrupted index names under concurrent requests.
• docs: adds OPENSEARCH_MIGRATION_TEST_PLAN.md — 530-line test plan covering all migration phases, rollback scenarios, and dual-write validation.

Test plan

• Phase 0 (ES-only): startup unchanged, checkAndInitializeIndex() skips OS validator.
• Phase 1/2 with OS unreachable: validateIndexingConfig() returns false → haltMigration() resets phase to 0 → fatal log emitted → dotCMS falls back to ES-only.
• Phase 1/2 with empty OS index + populated ES: no full reindex triggered; ES index preserved.
• Phase 3 with empty OS index: full reindex is triggered (expected).
• Concurrent calls to /api/v1/esindex/create: index names are well-formed (no SimpleDateFormat race).
• See docs/backend/OPENSEARCH_MIGRATION_TEST_PLAN.md for full test matrix.

🤖 Generated with Claude Code

[claude]

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

---

Review

• Read changed Java files
• Analyze IndexStartupValidator changes
• Analyze IndexConfigHelper.haltMigration / reset
• Analyze ContentletIndexAPIImpl (haltMigration call, hasEmptyIndices phase-awareness)
• Analyze ESIndexResource DateTimeFormatter swap
• Post review

---

The shape is good — version + endpoint checks gate boot, halt path is runtime-only so testers can recover, and the formatter swap is correct. A few things look wrong or worth a second look:

1. Phase 3 "abort loudly" doesn't actually abort — the surrounding try/catch swallows it

ContentletIndexAPIImpl.java:528 throws a DotRuntimeException when validation fails in Phase 3, with a comment saying it's safer to abort than serve stale ES data. But checkAndInitializeIndex() is wrapped in try { … } catch (Exception e) { Logger.fatal(…); } (line 551), which catches the throw and lets startup proceed.

End state in Phase 3 + OS unreachable today:

• Logger.error in validateIndexingConfig() ← from the validator
• DotRuntimeException thrown ← intended abort
• Caught by outer handler → Logger.fatal("Failed to create new indexes: …")
• Method returns, boot continues, no indexes get created

That's effectively the same outcome as the non-Phase-3 path, just with a different log line. If the intent is genuinely "do not boot with stale ES," either let the exception escape, narrow the outer catch, or set a flag that disables traffic. Otherwise the Phase 3 branch is documentation, not behavior.

2. The thread-safe formatter swap is incomplete

PR says SimpleDateFormat was replaced "to prevent corrupted index names under concurrent requests." Only ESIndexResource.java was updated. The same shared SimpleDateFormat timestampFormatter (declared as an interface constant on ContentletIndexAPI:33, still annotated @Deprecated(forRemoval = true)) is still used by:

• IndiciesInfo.java:78 (declaration), :143 (parse), :160 (format)
• dotmarketing/sitesearch/ajax/SiteSearchAjaxAction.java:117 — formatting a brand-new index name in a request handler (same race the PR was fixing)
• dotmarketing/portlets/cmsmaintenance/ajax/IndexAjaxAction.java:126 — same pattern
• ContentletIndexAPIImpl.java:1316 — IndiciesInfo.timestampFormatter.parse(ts)

The race in SiteSearchAjaxAction and IndexAjaxAction is the exact same shape as the one in ESIndexResource. Either finish migrating callers and delete the deprecated field, or scope the PR description to "fixed the one in ESIndexResource."

3. Possible behavior regression in hasEmptyIndices() for the initial Phase 0 → Phase 1 transition

hasEmptyIndices() now returns isEsWorkingIndexEmpty() for phases 0–2. The comment explains why this avoids spurious reindexes during dual-write catchup, which is correct for steady-state. But the first time an existing installation flips from phase 0 to phase 1, ES is populated and OS is empty — the old code would have triggered an automatic backfill reindex; the new code won't. If backfill is expected to happen through an explicit operator action / endpoint, fine, but worth a one-liner in the test plan / phase-1 cutover docs so nobody finds out the hard way.

4. haltMigration() may silently no-op depending on the config source

MigrationPhase.reset()'s javadoc already calls this out: if FEATURE_FLAG_OPEN_SEARCH_PHASE lives in ConfigSystemTable (DB-backed config), Config.setProperty writes to in-memory props but the DB value still wins on the next current() read. That means in a real prod scenario where the flag was toggled via the dotCMS config UI, haltMigration() does nothing — and the fatal log gives a false sense that fallback occurred. Two suggestions:

• Read current() again after Config.setProperty and log loudly if it didn't actually flip, so operators know they need to clear the system-table entry.
• Add a test plan row for "halt path with flag set via system-table" — the current matrix only covers the happy in-memory case.

5. Minor

• validateIndexingConfig() already logs Logger.error on failure; the caller in checkAndInitializeIndex() logs another Logger.error before haltMigration() (line 534). One of the two is redundant.
• PR description says "530-line test plan"; the file is 788 lines. Cosmetic.
• normalizeEndpoint uses default port 9200 when no port is given — fine for the common case, but two endpoints foo and foo:9200 will compare equal even if one was actually intended to be foo:443 behind a proxy. Probably acceptable given the "best-effort" framing already in the javadoc.

Not blocking

The MigrationPhase.reset() design (runtime-only flag flip + warn log) is a reasonable tester-friendly choice and matches the author's stated rationale in the review thread.

---

• Branch: fix/os-migration-startup-hardening-35631