fix(opensearch): configurable shadow write log level + phase-aware index tests (#35302)

[fabrizzio-dotCMS]

Summary

• Introduces DOTCMS_SHADOW_WRITE_LOG_LEVEL (default WARN) so operators can tune shadow OS write failure verbosity at runtime without code changes
• Adds PhaseRouterTest covering all four migration phases with the mismatched-index-name (T0/T1) scenario
• Adds ContentletIndexAPIImplPhaseTest — fully isolated unit tests for deactivateIndex, createContentIndex, closeIndex, and activateIndex using a full-dependency constructor (no APILocator)
• Adds ContentletIndexAPIImplMigrationIT — integration tests requiring two real clusters, covering ghost-index and invalid-name edge cases across phases; registered in OpenSearchUpgradeSuite
• Guards addCustomMapping behind if (result) in createContentIndex — prevents applying mappings to an index the cluster rejected, keeping the soft-failure contract consistent

Test plan

• ContentletIndexAPIImplPhaseTest — ./mvnw test -pl :dotcms-core -Dtest=ContentletIndexAPIImplPhaseTest
• PhaseRouterTest — ./mvnw test -pl :dotcms-core -Dtest=PhaseRouterTest
• ContentletIndexAPIImplMigrationIT — ./mvnw verify -pl :dotcms-integration -Dcoreit.test.skip=false -Dopensearch.upgrade.test=true -Dit.test=ContentletIndexAPIImplMigrationIT
• No regressions in existing OpenSearchUpgradeSuite tests

Closes #35302

🤖 Generated with Claude Code

This PR fixes: #35302

[claude]

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

---

PR Review

• Read changed files
• Analyze production code changes
• Analyze test code
• Post review findings

---

Issues Found

1. Shadow failure logging not applied in createContentIndex — inconsistent with the PR's core contract (moderate)

ContentletIndexAPIImpl.createContentIndex(String, int) was refactored to separate primary from shadow results, but the catch block still logs all failures at ERROR regardless of provider role:

// ContentletIndexAPIImpl.java ~line 601
} catch (Exception e) {
    Logger.error(this.getClass(), "Error while creating content index " + physicalName, e);
    if (ops == primary) {
        primaryResult = false;
    }
    // shadow failures are fire-and-forget in dual-write phases  ← comment contradicts the code
}

Every other write path in this PR (PhaseRouter.writeChecked, writeBoolean, writeReturningChecked, CompositeBulkProcessor.close, BulkProcessorListener.afterBulk) was switched to logShadowWriteFailure(). This one was missed. During Phase 1/2, a shadow OS creation failure will fire at ERROR even when DOTCMS_SHADOW_WRITE_LOG_LEVEL=DEBUG. Fix this →

2. Same pattern missed in activateIndex OS mirror block (minor, pre-existing but adjacent to the PR's change):

// ContentletIndexAPIImpl.java ~line 2464
} catch (Exception e) {
    Logger.warn(this, "Could not mirror index activation to OS store for index: " + indexName, e);
}

This is a shadow write failure and should be logShadowWriteFailure() for the same reason. Technically pre-existing, but this PR is the right time to fix it since it's the canonical shadow-write path.

3. PhaseRouter.writeBoolean reads phase config twice — TOCTOU window (minor):

final List<T> providers = writeProviders();   // calls MigrationPhase.current() x2
...
final T primary = readProvider();             // calls MigrationPhase.current() again

If writeProviders() returns [ES, OS] (dual-write) but the flag is updated before readProvider() returns osImpl (Phase 2 → 3), primary ends up as osImpl but the loop iterates over both. Harmless in practice because phase changes don't race with request threads, but the pattern is a latent inconsistency. writeChecked and writeReturningChecked have the same structure.

4. PhaseRouterTest has no coverage of read() / readChecked() Phase 2 ES fallback (test gap):

The test file covers all four write paths (write, writeBoolean, writeChecked) but there are zero tests for the Phase 2 read fallback:

• OS throws → should fall back to ES and return ES result
• Both fail → exception from ES should propagate
• Phase 3: no fallback, OS failure propagates

These are the most complex paths in PhaseRouter and the ones most likely to regress silently.

5. writeBoolean default primaryResult = false is safe but needs a note (nit):

The old default was true, the new one is false. The comment says "assume failure until primary confirms success" — this is correct reasoning, but if primary throws, the exception is re-thrown anyway so the default value is never observed by callers. The change is harmless but the comment is misleading about why the default matters.

---

Design concern worth calling out

The test test_activateIndex_phase2_esName_mirroredToOsStore (and the corresponding IT test) explicitly documents that passing "working_T0" in Phase 2 writes "working_T0" to the OS pointer store — even though OS physically holds "working_T1". The comment in the test says this is "expected behavior during the catch-up window."

This means the OS pointer store can get a name that does not exist in the OS cluster, and any subsequent deactivateIndex("working_T0") in Phase 2 will silently clear the OS working slot (confirmed by test_deactivateIndex_phase2_clearsBothStoresByIndexType). The behavior is documented, but the documentation is only in test comments — not in activateIndex's javadoc, where a future maintainer is most likely to look. Consider adding a warning there.

---

Otherwise the core logic is sound: the logShadowWriteFailure extraction is clean, the if (primaryResult) guard on addCustomMapping is a correct fix, the full-dependency constructor for unit testing is the right approach, and the test scenarios comprehensively cover the four-phase routing table for write paths. The integration test's assumeFalse(esSameAsOs()) guards are correct for the two-cluster scenarios.

[claude]

PR Review in Progress

• Read changed files
• Analyze production code changes
• Analyze test code
• Post review findings

View job run

[hassandotcms]

looks good!