Skip to content

fix(opensearch): readiness summary states the drift that is actually present (#37638) - #37735

Merged
fabrizzio-dotCMS merged 1 commit into
mainfrom
37638-readiness-prose-contradicts-data
Sep 25, 2026
Merged

fabrizzio-dotCMS merged 1 commit into
mainfrom
37638-readiness-prose-contradicts-data

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Closes #37638

Problem

The readiness report's counts were right, but the summary around them gave the wrong advice in two cases:

  • Phase 0 after a rollback from dual-write. Content saved at Phase 0 reaches Elasticsearch only, so the current OpenSearch copies fall behind. The summary called them "indices from an earlier migration attempt… left over" and said "dual-write will overwrite them". Both are wrong: the indices are current, and dual-write never backfills.
  • Phase 3 with OpenSearch behind Elasticsearch. The summary ended with "No index shows Elasticsearch behind OpenSearch". The drift it described runs the other way, and it said nothing about OpenSearch, which serves live search, being the engine that was behind.

Change

All changes are to the summary text; the verdict, counts, blockers, safeToAdvance and safeToRollback are unchanged.

  • MigrationReadinessService.initialPhaseNotes: at Phase 0 the summary now separates copies whose counts drifted from copies that exist only on OpenSearch. For drifted copies it names a reindex (or a Site Search re-crawl) as the fix and says dual-write never backfills.
  • MigrationReadinessService.finalPhaseDriftSentence: at Phase 3 the summary states every direction of drift that is present. When OpenSearch is behind, it adds its own sentence, which says deletes since the cutover look the same and the database comparison tells the two apart. The "no drift" sentence now appears only when the counts are equal.
  • ContentIndexMirrorReconciler.incompleteNote: the note no longer says "it was never fully rebuilt". It names both causes, since a copy that missed writes after a rollback reads the same as one that was never finished.

Verification

  • MigrationReadinessServiceTest: 6 new tests. The 4 that cover the reported cases failed before the change; the other 2 check that in-sync copies at Phase 0 and equal counts at Phase 3 keep their current summary. ContentIndexMirrorReconcilerTest also has new assertions on the note text. 67/67 pass across the three readiness test classes.
  • Checked live with the compiled classes on a local Docker stack (ES 7.10.2, OS 3.4.0):
    • At Phase 1 in sync, then rolled back to Phase 0 with nothing changed: the summary stays the same, with no "left over" text.
    • At Phase 0, after publishing one contentlet (ES 7/5, OS 6/4): the new drift note appears.
    • At Phase 3 in the same state: the reindex sentence appears, followed by the OpenSearch-behind sentence.
    • At Phase 1: the report blocks on both indices, as the Phase 0 note says it will.

🤖 Generated with Claude Code

This PR fixes: #37638

@claude

claude Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

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


Code Review

Reviewed the diff against origin/main. All changes are summary/note prose plus two small helpers (initialPhaseNotes, finalPhaseDriftSentence); verdict, counts, blockers, safeToAdvance/safeToRollback are untouched. The helper logic is sound and consistent with the existing blocksRollback/needsAttentionInFinalPhase predicates:

  • openSearchBehind (MigrationReadinessService.java:251-255) guards exists() on both sides and docCount() >= 0 before comparing, so unmeasurable counts (-1) don't produce a false "behind" — matching the fail-safe stance in blocksRollback.
  • The ES-behind and OS-behind directions are mutually exclusive per index, so the two appended sentences never contradict each other for the same index.
  • Collectors is already imported (line 16); no compile concern.
  • Test coverage is strong: both drift directions at Phase 3, the deleted-content case that stays uncounted but still described, equal-counts keeping the no-drift sentence, and the Phase 0 rollback/in-sync/orphan cases.

New Issues

  • 🟡 Medium: dotCMS/src/main/java/com/dotcms/content/index/migration/MigrationReadinessService.java:135-136 — When the only out-of-sync rows are OpenSearch-only orphans (ES side absent), the lead sentence still reads "OpenSearch already holds copies that do not match." An orphan index isn't a copy of anything to "not match" — the follow-up note correctly calls it "exists only on OpenSearch … safe to remove if unused." Given this PR's whole aim is prose that matches the data, consider gating the "copies that do not match" wording on drifted being non-empty (or softening it) so the orphan-only case doesn't reintroduce a small version of the same mismatch. Non-blocking.

Everything else looks correct and well-tested.
· 37638-readiness-prose-contradicts-data

Base automatically changed from 37636-readiness-unreachable-engine to main September 24, 2026 22:44
…present (#37638)

Phase 0 after a rollback called the current, merely stale OpenSearch copies
"left over from an earlier migration attempt" and promised dual-write would
overwrite them. Dual-write never backfills; the summary now says the copies
are behind and that a reindex is the repair, and describes OpenSearch-only
indices separately.

Phase 3 volunteered "No index shows Elasticsearch behind OpenSearch" while
OpenSearch, the engine serving reads, was the one behind. The summary now
states each direction present, and notes that deletes since the cutover
produce the same shape, with the database comparison deciding.

The incomplete-copy note no longer claims the copy "was never fully rebuilt";
a copy that missed writes after a rollback reads the same.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@fabrizzio-dotCMS
fabrizzio-dotCMS force-pushed the 37638-readiness-prose-contradicts-data branch from b75697e to 9873753 Compare September 25, 2026 02:06
@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code and removed AI: Safe To Rollback labels Sep 25, 2026
@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Sep 25, 2026
Merged via the queue into main with commit 43267ce Sep 25, 2026
75 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the 37638-readiness-prose-contradicts-data branch September 25, 2026 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Readiness report prose contradicts its own data: healthy indexes called leftovers at phase 0, wrong drift direction at phase 3

2 participants