Skip to content

Commit 390a561

Browse files
fix(opensearch): make index bootstrap idempotent for orphaned cluster indices (#36237) (#36238)
## Proposed Changes Fixes #36237. Makes the OpenSearch/Elasticsearch index **bootstrap idempotent** against an *orphaned cluster index* — one that physically exists in the cluster but is missing from the dotCMS index store. ### Root cause `ContentletIndexAPIImpl.createContentIndex(name, shards, tag)` (used only by the ES/OS bootstrap paths) always issues a create. Readiness is decided by `indexReadyOS()`/`indexReadyES()`, which key off the **store** (`VersionedIndices` / `indicies`), not the physical cluster state. When a prior startup created the physical index but never committed its store pointer (partial/interrupted init, failed `pointOS`, etc.), the two diverge: on restart the catchup path re-derives the same logical name and the create fails with `resource_already_exists`. That exception bubbles to `checkAndInitializeIndex()` and is swallowed, leaving the instance **half-initialised** — and the read provider then queries an unfinished index, surfacing as `search_phase_execution_exception / all shards failed`. ### The fix In `createContentIndex(name, shards, tag)`, before creating, check `providerApi.indexExists(physicalName)`: - **Already exists** → reuse it, re-assert the custom mapping (`putMapping` is additive/idempotent, so a previously unmapped orphan is repaired), and return `true`. The caller's `point()` re-registers it in the store. - **Does not exist** → create as before. This is scoped to the two callers (`bootstrapAndPointES` / `bootstrapAndPointOS`); no public/interface signatures change. The `(IndexAPIImpl) indexAPI` cast mirrors the existing `indexReadyES()/indexReadyOS()` usage. ## Testing - `./mvnw compile -pl :dotcms-core` (JDK 25) — compiles clean. - No signature changes; `ContentletIndexAPIImplPhaseTest` fakes never exercise the bootstrap path, so existing unit coverage is unaffected. - A meaningful idempotency IT requires a live cluster (`opensearch-upgrade` profile) since the bootstrap path casts to `IndexAPIImpl` — can be added on request. ## Out of scope (tracked in #36237) - `checkAndInitializeIndex()` swallowing bootstrap failures instead of failing loudly. - `ContentFactoryIndexOperationsES` hiding the per-shard `ShardSearchFailure` root cause behind the generic "all shards failed" wrapper. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- re-run link-issue: Team : Scout added to #36237 --> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4706807 commit 390a561

2 files changed

Lines changed: 244 additions & 11 deletions

File tree

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java

Lines changed: 64 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -652,22 +652,75 @@ public synchronized boolean createContentIndex(final String indexName, final int
652652

653653

654654
/**
655-
* Create an index exclusively in one of the SE Providers
656-
* @param indexName
657-
* @param shards
658-
* @param tag
659-
* @return
660-
* @throws IOException
655+
* Create an index exclusively in one of the SE Providers.
656+
*
657+
* <p><b>Idempotent bootstrap.</b> If the physical index already exists in the target
658+
* cluster it is reused instead of issuing a create. This guards against an orphaned
659+
* cluster index — present in the cluster but missing from the index store — left behind
660+
* when a previous bootstrap created the index but never committed its store pointer
661+
* (e.g. the OS {@code VersionedIndices} row, or after a partial/interrupted startup).
662+
* Without this guard the restart re-derives the same logical name, the create fails with
663+
* {@code resource_already_exists}, and {@code checkAndInitializeIndex()} aborts — leaving
664+
* the instance half-initialised. The custom mapping is (re)applied either way
665+
* ({@code putMapping} is additive/idempotent), so a previously unmapped orphan is repaired,
666+
* and the caller's {@code point()} re-registers the index in the store.</p>
667+
*
668+
* @param indexName logical index name (no cluster prefix, no vendor tag)
669+
* @param shards number of shards to create with (ignored when the index already exists)
670+
* @param tag target provider ({@link IndexTag#ES} or {@link IndexTag#OS})
671+
* @return {@code true} when the index exists (reused) or was created successfully
672+
* @throws IOException on a hard creation failure
661673
*/
662674
private boolean createContentIndex(final String indexName, final int shards, IndexTag tag)
663675
throws IOException {
664-
final MappingHelper helper = MappingHelper.getInstance();
676+
final IndexAPIImpl impl = (IndexAPIImpl) indexAPI;
665677

666-
ContentletIndexOperations ops = router.esImpl();
667-
if(tag == IndexTag.OS) {
668-
ops = router.osImpl();
669-
}
678+
final ContentletIndexOperations ops = tag == IndexTag.OS ? router.osImpl() : router.esImpl();
679+
final IndexAPI providerApi = tag == IndexTag.OS ? impl.osImpl() : impl.esImpl();
680+
681+
return createContentIndex(indexName, shards, tag, ops, providerApi,
682+
MappingHelper.getInstance());
683+
}
684+
685+
/**
686+
* Idempotent-bootstrap core of {@link #createContentIndex(String, int, IndexTag)}, with the
687+
* provider collaborators injected so the orphan-reuse decision can be unit-tested without a
688+
* running cluster or the {@link MappingHelper} singleton. See the public-facing overload's
689+
* javadoc for the behaviour contract.
690+
*
691+
* @param indexName logical index name (no cluster prefix, no vendor tag)
692+
* @param shards number of shards to create with (ignored when the index already exists)
693+
* @param tag target provider ({@link IndexTag#ES} or {@link IndexTag#OS})
694+
* @param ops vendor write operations for {@code tag} (creation + physical-name mapping)
695+
* @param providerApi vendor index API for {@code tag} (existence probe)
696+
* @param helper mapping helper used to (re)assert the custom mapping
697+
* @return {@code true} when the index exists (reused) or was created successfully
698+
* @throws IOException on a hard creation failure
699+
*/
700+
boolean createContentIndex(final String indexName, final int shards, final IndexTag tag,
701+
final ContentletIndexOperations ops, final IndexAPI providerApi,
702+
final MappingHelper helper) throws IOException {
670703
final String physicalName = ops.toPhysicalName(indexName);
704+
705+
// Reuse an orphaned cluster index rather than failing the create (see method javadoc).
706+
// The existence probe is best-effort: any failure is treated as "does not exist" so we
707+
// fall through to the create path rather than aborting bootstrap on a transient error.
708+
// The failure is logged at DEBUG so a real connectivity/config problem (which would then
709+
// also surface on the create attempt) stays traceable instead of being silently swallowed.
710+
final boolean alreadyExists = Try.of(() -> providerApi.indexExists(physicalName))
711+
.onFailure(e -> Logger.debug(this,
712+
"Bootstrap existence probe failed for " + physicalName
713+
+ " — treating as 'does not exist' and attempting create: "
714+
+ e.getMessage(), e))
715+
.getOrElse(false);
716+
if (alreadyExists) {
717+
Logger.info(this, String.format(
718+
"Bootstrap: %s index already exists, reusing and re-asserting mapping: %s",
719+
tag, physicalName));
720+
helper.addCustomMapping(List.of(indexName), tag);
721+
return true;
722+
}
723+
671724
final boolean contentIndex = ops.createContentIndex(physicalName, shards);
672725
if (contentIndex) {
673726
helper.addCustomMapping(List.of(indexName), tag);
Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
package com.dotcms.content.elasticsearch.business;
2+
3+
import static org.junit.Assert.assertFalse;
4+
import static org.junit.Assert.assertTrue;
5+
import static org.mockito.ArgumentMatchers.anyInt;
6+
import static org.mockito.ArgumentMatchers.anyString;
7+
import static org.mockito.Mockito.mock;
8+
import static org.mockito.Mockito.never;
9+
import static org.mockito.Mockito.verify;
10+
import static org.mockito.Mockito.when;
11+
12+
import com.dotcms.content.elasticsearch.util.MappingHelper;
13+
import com.dotcms.content.index.ContentletIndexOperations;
14+
import com.dotcms.content.index.IndexAPI;
15+
import com.dotcms.content.index.IndexTag;
16+
import com.dotcms.content.index.VersionedIndicesAPI;
17+
import java.io.IOException;
18+
import java.util.List;
19+
import org.junit.Test;
20+
21+
/**
22+
* Unit tests for the idempotent index-bootstrap logic introduced for orphaned cluster indices
23+
* (#36237 / PR #36238).
24+
*
25+
* <p>Exercises {@link ContentletIndexAPIImpl#createContentIndex(String, int, IndexTag,
26+
* ContentletIndexOperations, IndexAPI, MappingHelper)} — the collaborator-injected seam of the
27+
* private {@code createContentIndex(indexName, shards, tag)} — so the orphan-reuse decision can be
28+
* validated without a running Elasticsearch/OpenSearch cluster or the {@code MappingHelper}
29+
* singleton. All collaborators are Mockito doubles.</p>
30+
*
31+
* <p>The behaviour under test:</p>
32+
* <ul>
33+
* <li>an index already present in the cluster is <b>reused</b> (no create) and its mapping is
34+
* re-asserted — the orphaned-index repair path;</li>
35+
* <li>a missing index is created and, on success, mapped;</li>
36+
* <li>a failed create does not apply a mapping;</li>
37+
* <li>a failing existence probe is treated as "does not exist", so bootstrap falls through to
38+
* the create path instead of aborting.</li>
39+
* </ul>
40+
*/
41+
public class ContentletIndexAPIImplBootstrapTest {
42+
43+
private static final String CLUSTER_PREFIX = "cluster_test.";
44+
private static final String LOGICAL_NAME = "working_T0";
45+
private static final String PHYSICAL_NAME = CLUSTER_PREFIX + LOGICAL_NAME;
46+
private static final int SHARDS = 1;
47+
48+
/**
49+
* Builds an instance solely so the package-private seam can be invoked. The constructor
50+
* dependencies are irrelevant to the seam, which operates exclusively on its injected
51+
* collaborators.
52+
*/
53+
private static ContentletIndexAPIImpl newApi() {
54+
return new ContentletIndexAPIImpl(
55+
mock(ContentletIndexOperations.class),
56+
mock(ContentletIndexOperations.class),
57+
mock(IndexAPI.class),
58+
mock(IndiciesAPI.class),
59+
mock(VersionedIndicesAPI.class));
60+
}
61+
62+
/**
63+
* Given : the physical index already exists in the target cluster (an orphaned cluster index
64+
* left behind by a previous bootstrap that never committed its store pointer).
65+
* When : createContentIndex() runs during bootstrap.
66+
* Then : the index is reused (no create is issued), the custom mapping is re-asserted to
67+
* repair a possibly-unmapped orphan, and the method returns true.
68+
*/
69+
@Test
70+
public void test_orphanIndexExists_reusesAndReassertsMapping_skipsCreate() throws IOException {
71+
final ContentletIndexOperations ops = mock(ContentletIndexOperations.class);
72+
final IndexAPI providerApi = mock(IndexAPI.class);
73+
final MappingHelper helper = mock(MappingHelper.class);
74+
75+
when(ops.toPhysicalName(LOGICAL_NAME)).thenReturn(PHYSICAL_NAME);
76+
when(providerApi.indexExists(PHYSICAL_NAME)).thenReturn(true);
77+
78+
final boolean result = newApi()
79+
.createContentIndex(LOGICAL_NAME, SHARDS, IndexTag.ES, ops, providerApi, helper);
80+
81+
assertTrue("Existing (orphaned) index must be reused and reported as available", result);
82+
verify(ops, never()).createContentIndex(anyString(), anyInt());
83+
verify(helper).addCustomMapping(List.of(LOGICAL_NAME), IndexTag.ES);
84+
}
85+
86+
/**
87+
* Given : the physical index does not exist in the target cluster.
88+
* When : createContentIndex() runs and the create succeeds.
89+
* Then : the index is created with the resolved physical name and shard count, the custom
90+
* mapping is applied, and the method returns true.
91+
*/
92+
@Test
93+
public void test_indexMissing_createSucceeds_appliesMapping() throws IOException {
94+
final ContentletIndexOperations ops = mock(ContentletIndexOperations.class);
95+
final IndexAPI providerApi = mock(IndexAPI.class);
96+
final MappingHelper helper = mock(MappingHelper.class);
97+
98+
when(ops.toPhysicalName(LOGICAL_NAME)).thenReturn(PHYSICAL_NAME);
99+
when(providerApi.indexExists(PHYSICAL_NAME)).thenReturn(false);
100+
when(ops.createContentIndex(PHYSICAL_NAME, SHARDS)).thenReturn(true);
101+
102+
final boolean result = newApi()
103+
.createContentIndex(LOGICAL_NAME, SHARDS, IndexTag.ES, ops, providerApi, helper);
104+
105+
assertTrue("A successful create must be reported as available", result);
106+
verify(ops).createContentIndex(PHYSICAL_NAME, SHARDS);
107+
verify(helper).addCustomMapping(List.of(LOGICAL_NAME), IndexTag.ES);
108+
}
109+
110+
/**
111+
* Given : the physical index does not exist.
112+
* When : createContentIndex() runs and the create fails (returns false).
113+
* Then : no mapping is applied and the method returns false — the failure is not masked.
114+
*/
115+
@Test
116+
public void test_indexMissing_createFails_noMapping() throws IOException {
117+
final ContentletIndexOperations ops = mock(ContentletIndexOperations.class);
118+
final IndexAPI providerApi = mock(IndexAPI.class);
119+
final MappingHelper helper = mock(MappingHelper.class);
120+
121+
when(ops.toPhysicalName(LOGICAL_NAME)).thenReturn(PHYSICAL_NAME);
122+
when(providerApi.indexExists(PHYSICAL_NAME)).thenReturn(false);
123+
when(ops.createContentIndex(PHYSICAL_NAME, SHARDS)).thenReturn(false);
124+
125+
final boolean result = newApi()
126+
.createContentIndex(LOGICAL_NAME, SHARDS, IndexTag.ES, ops, providerApi, helper);
127+
128+
assertFalse("A failed create must not be reported as available", result);
129+
verify(helper, never()).addCustomMapping(List.of(LOGICAL_NAME), IndexTag.ES);
130+
}
131+
132+
/**
133+
* Given : the existence probe itself fails (e.g. a transient cluster error).
134+
* When : createContentIndex() runs.
135+
* Then : the probe failure is swallowed and treated as "does not exist", so bootstrap falls
136+
* through to the create path rather than aborting. Here the subsequent create
137+
* succeeds, so the index is created and mapped.
138+
*/
139+
@Test
140+
public void test_existenceProbeThrows_treatedAsMissing_proceedsToCreate() throws IOException {
141+
final ContentletIndexOperations ops = mock(ContentletIndexOperations.class);
142+
final IndexAPI providerApi = mock(IndexAPI.class);
143+
final MappingHelper helper = mock(MappingHelper.class);
144+
145+
when(ops.toPhysicalName(LOGICAL_NAME)).thenReturn(PHYSICAL_NAME);
146+
when(providerApi.indexExists(PHYSICAL_NAME))
147+
.thenThrow(new RuntimeException("cluster unreachable"));
148+
when(ops.createContentIndex(PHYSICAL_NAME, SHARDS)).thenReturn(true);
149+
150+
final boolean result = newApi()
151+
.createContentIndex(LOGICAL_NAME, SHARDS, IndexTag.ES, ops, providerApi, helper);
152+
153+
assertTrue("A failed existence probe must fall through to a (successful) create", result);
154+
verify(ops).createContentIndex(PHYSICAL_NAME, SHARDS);
155+
verify(helper).addCustomMapping(List.of(LOGICAL_NAME), IndexTag.ES);
156+
}
157+
158+
/**
159+
* Given : an OS-tagged bootstrap of an already-existing index.
160+
* When : createContentIndex() runs with {@link IndexTag#OS}.
161+
* Then : the mapping is re-asserted against the OS provider — the tag is propagated unchanged
162+
* to the mapping helper so the correct vendor is targeted.
163+
*/
164+
@Test
165+
public void test_osTag_isPropagatedToMappingHelper() throws IOException {
166+
final ContentletIndexOperations ops = mock(ContentletIndexOperations.class);
167+
final IndexAPI providerApi = mock(IndexAPI.class);
168+
final MappingHelper helper = mock(MappingHelper.class);
169+
170+
final String osPhysical = PHYSICAL_NAME + ".os";
171+
when(ops.toPhysicalName(LOGICAL_NAME)).thenReturn(osPhysical);
172+
when(providerApi.indexExists(osPhysical)).thenReturn(true);
173+
174+
final boolean result = newApi()
175+
.createContentIndex(LOGICAL_NAME, SHARDS, IndexTag.OS, ops, providerApi, helper);
176+
177+
assertTrue(result);
178+
verify(helper).addCustomMapping(List.of(LOGICAL_NAME), IndexTag.OS);
179+
}
180+
}

0 commit comments

Comments
 (0)