Skip to content

fix(opensearch): preserve OS index store on phase rollback during in-flight reindex (#36471) - #36472

Merged
fabrizzio-dotCMS merged 2 commits into
mainfrom
issue-36471-os-store-midreindex-rollback
Jul 8, 2026
Merged

fix(opensearch): preserve OS index store on phase rollback during in-flight reindex (#36471)#36472
fabrizzio-dotCMS merged 2 commits into
mainfrom
issue-36471-os-store-midreindex-rollback

Conversation

@swicken

@swicken swicken commented Jul 8, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Fixes #36471 — rolling FEATURE_FLAG_OPEN_SEARCH_PHASE back to 0 while a full reindex is draining the journal wiped the OS side of the indicies store and stranded a partial .os reindex pair on the cluster; a later boot catchup adopted the partial pair as active, so Phase-2 searches silently returned a fraction of the content (measured 8/24, 0/6 on the demo starter) with zero errors.

Root causeIndiciesFactory.point() (the legacy ES store) deleted rows by index_type across all index_version values, while only ever reading its own NULL-version rows. Every ES switchover therefore wiped the OS rows (index_version='os-3.x') in the shared table. In Phases 1/2 the wipe was invisible because the OS mirror immediately re-wrote the rows — and only found the values to re-write via the stale VersionedIndicesCache, which point() doesn't flush. In Phase 0 the mirror is skipped, so the rows stayed gone.

  • Scope point()'s delete with index_version IS NULL so the legacy store only manages the rows it owns. This also removes the Phase-1/2 dependence on the stale-cache accident.
  • Treat OS reindex slots encountered during a Phase-0 switchover or abort as a stranded mid-reindex rollback (abortStrandedOsReindex): clear the slots first (a leftover slot flips isInFullReindex() back to true on a later move to Phase 2, which would switch ES over onto null pointers), then delete the partial physical .os indices so no boot catchup can ever adopt them. Logged at WARN with the deleted index names — no silent path remains.
  • New ContentletIndexAPIImplMidReindexRollbackIT in OpenSearchUpgradeSuite covering the point() row-preservation contract plus the Phase-0 switchover and abort paths (3 tests, green).
  • OPENSEARCH_MIGRATION.md: documented the mid-reindex rollback behavior and operational rule under "Rollback risk during dual-write phases".

Verification — reproduced live on the single-node-os-migration example (released image: 4→2 rows, partial orphans 201/200 docs, catchup adoption after restart, Phase-2 partial results). Same sequence on the fixed image: .os working/live rows survive, the partial pair is deleted at switchover with a WARN, restart adopts nothing, and Phase-2 counts equal Phase-0 counts exactly (24/24, 6/6, 26/26).

Follow-ups deliberately out of scope: a catchup adoption sanity-check (doc-count vs ES twin) and operator-facing phase-change validation while isInFullReindex() — both defense-in-depth beyond the damage-site fix.

Checklist

  • Tests
  • Translations (n/a)
  • Security implications contemplated (change respects store ownership boundaries; no new input surface)

This PR fixes: #36471

…flight reindex (#36471)

Rolling FEATURE_FLAG_OPEN_SEARCH_PHASE back to 0 while a full reindex was
draining the journal wiped the active OS working/live rows from the
indicies store and stranded a partial .os reindex pair on the cluster,
which a later boot catchup adopted as active — phase-2 searches then
silently returned a fraction of the content.

- Scope IndiciesFactory.point()'s delete to its own NULL-version rows so
  the legacy ES store update can never remove the OS versioned rows it
  does not manage (root cause of the store wipe; in phases 1/2 the wipe
  was masked only because the OS mirror re-wrote the rows from a stale
  VersionedIndicesCache).
- Treat OS reindex slots found during a Phase-0 switchover or abort as a
  stranded mid-reindex rollback: clear the slots (a leftover slot makes
  isInFullReindex() true again on a later flip to Phase 2 and would
  switch ES over onto null pointers) and delete the partial physical
  .os indices so no boot catchup can ever adopt them.
- Cover the rollback in OpenSearchUpgradeSuite
  (ContentletIndexAPIImplMidReindexRollbackIT) and document the
  operational rule in OPENSEARCH_MIGRATION.md.
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Claude finished @swicken's task in 9m 12s —— View job


Rollback-Safety Analysis

  • Read docs/core/ROLLBACK_UNSAFE_CATEGORIES.md
  • Get full PR diff (eb15ac31...5049c6e9)
  • Analyze diff against every unsafe category
  • Apply label

Verdict: ✅ Safe to Rollback

This PR is pure application-code logic — no runonce DB migration task, no ES/OS mapping change, no schema/DDL, no REST/GraphQL contract change, no OSGi interface change.

  • IndiciesFactory.java — scopes the existing DELETE from indicies where index_type=? or index_name=? to ... and index_version is null. This changes only the runtime query predicate, not the indicies table schema. No columns/tables added, renamed, or dropped.
  • ContentletIndexAPIImpl.java — adds abortStrandedOsReindex(), called from the existing Phase-0 switchover/abort paths. It only touches rows in indicies/OS versioned-index metadata via existing VersionedIndicesAPI methods (saveIndices, removeVersion) and deletes physical OS cluster indices — no data model, PK, or storage-provider change.
  • OPENSEARCH_MIGRATION.md, test additions, suite registration — docs/tests only.

None of C-1…C-4, H-1…H-8, or M-1…M-4 apply: there's no structural storage change, no ES mapping change, no contentlet_as_json version bump, no DROP/RENAME, no PK restructuring, no new field type, no storage-provider switch, no stored-procedure drop, no NOT NULL-without-default column, no VTL/REST/GraphQL/OSGi contract change.

Rolling back to N-1 simply restores the prior (buggy) unscoped-delete/no-abort behavior described in the issue — it does not leave N-1 unable to start, read data, or serve requests. Label AI: Safe To Rollback applied.

#36471)

When the OS store record holds reindex slots but no active working/live
(reachable when the active OS pair is deleted through the index-management
flow mid-reindex), rebuilding the record from working/live alone produced
an empty record that saveIndices rejects — the throw skipped the physical
deletes and left exactly the stranded slots + partial indices the abort
exists to remove. Remove the version row instead (same treatment as
clearOsStorePointer, #35640) so the deletes always run, and preserve a
siteSearch pointer if one exists. Covered by a fourth IT case.
@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Jul 8, 2026
@mergify

mergify Bot commented Jul 8, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Merged via the queue into main with commit f574a9d Jul 8, 2026
60 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the issue-36471-os-store-midreindex-rollback branch July 8, 2026 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Documentation PR changes documentation files

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

OpenSearch: phase rollback during in-flight reindex wipes OS index store; partial orphan indices later adopted, causing silent incomplete search results

2 participants