fix(search): journal content deletes so lost index removals are retried (#37276)

[fabrizzio-dotCMS]

Implements the spec approved in #37297. Fixes #37276.

Base branch note: this targets 37276-silent-index-delete-loss (the spec branch), not main, because the spec has not merged yet. Until it does, the diff here also shows the spec commit — that is the shared ancestor, not a duplicate. Once #37297 lands this collapses to just the implementation.

The problem

Content destruction deleted the database rows transactionally, then handed the index removal to an in-memory post-commit listener that recorded nothing durable. Any loss of that task orphaned the index document permanently:

• the JVM stopped between commit and execution (a rolling deploy is exactly this window),
• the shared listener pool rejected the task,
• or the bulk write returned per-item failures that were logged and treated as success.

No error surfaced, nothing retried, and the divergence never healed. The application reads from the index — search, listings, URL maps, widget counts — so the symptom reaching users was an inflated result count above a shorter list.

Adds never had this problem: they write their intent to dist_reindex_journal inside the transaction, and ReindexThread retries until the index acknowledges. The asymmetry is visible on the same IndexPolicy.DEFER check, two branches apart — add → journal (ContentletIndexAPIImpl:2308), delete → commit listener (:3055).

The equivalent delete machinery already existed end to end — ReindexAction.DELETE, addIdentifierDelete, the read-back flag, and both the ES and OS bulk consumers, all covered by integration tests — with zero production callers. This PR wires it up.

What changed

Change	Why
destroyContentlets and deleteAllVersionsandBackup enqueue a DELETE journal entry in the same transaction that deletes the rows	The durable record is what survives a restart or a rejected task. The commit listener stays as the low-latency path.
ReindexQueueFactory.findContentToReindex resolves a REINDEX/DELETE collision by row id instead of poll order	From review feedback on #37297 — see below.
ContentletIndexOperationsES and ...OS raise on a partially failed bulk	Loss point L3. The issue named only ES; OpenSearch carries the identical defect.
A removal skipped because the primary provider's index pointers will not load no longer reads as completed	Loss point L2. Shadow providers keep warn-and-continue.
Bulk failure messages no longer say Error reindexing for removals	That wording is why searching production logs for failed deletes came back empty.
IndexPolicyProvider javadoc corrected	It claimed a WAIT_FOR default; the code defaults to DEFER.

Three things worth a reviewer's attention

1. The batch collision had to be fixed first, or the fix reintroduces the bug.
findContentToReindex builds its batch as a Map keyed by identifier alone, while ReindexEntry equality includes the delete flag — so a REINDEX and a DELETE for the same identifier are not equal (the duplicate-drain loop below does not collapse them) yet collide on the key, and the later poll() silently overwrites the earlier. Unreachable today because nothing enqueues deletes; routine the moment destroyContentlets does, since content is saved and then destroyed.

Two ways the pair arrives, both broken: at equal priority (the common case — both addIdentifierReindex and addIdentifierDelete default to NORMAL) the drain query's ORDER BY priority ASC has no tiebreaker, so the order is undefined by contract; at unequal priority (a full reindex at REINDEX/300 against a removal at NORMAL/100) the stale reindex wins deterministically, every time.

Resolution is by id, not by "deletes always win": an identifier destroyed and later reused is a legitimate reindex. Priority deliberately does not break the tie — a stale ASAP reindex would beat a newer removal, which is the defect again.

2. The escalation is in the providers, not the router — that placement is what keeps ADR-0009 intact.
ContentletIndexAPIImpl#putToIndex already isolates the dual-write shadow leg: it catches the ES failure, always calls OS, logs an OS failure as shadow divergence, and rethrows only the ES one. So raising inside the providers yields ADR-0009's contract in every phase — phases 1/2 swallow the OS failure while OS is the shadow, phase 3 propagates it once OS is primary. ContentletIndexPartialFailurePhaseTest pins this so a well-meaning refactor cannot move it up.

3. One delete path is deliberately left alone.
delete(List, User, boolean, boolean) can delete a subset of an identifier's versions or languages, and a journal entry is identifier-wide — journalling there would remove index documents for languages that still exist. Same reasoning excludes unpublish/archive. Both are marked with comments explaining why, so nobody "completes" them later. ContentletDestroyIndexRemovalTest has a regression test for the unpublish case.

Behaviour change operators will notice

Partial bulk failures now raise instead of being swallowed. In an environment that has been quietly losing index writes this will look like a regression and is not — the failures were always happening, only their visibility changed. Full detail in specs/37276-silent-index-delete-loss/release-note.md, including the query for enumerating removals the index still owes.

putToIndex is on the public ContentletIndexAPI interface. Signature unchanged and source/binary compatible, but an out-of-tree plugin relying on it returning normally after a partial failure will now see an exception. Out-of-tree callers cannot be enumerated from this repo; that residual risk is covered by the release note rather than by a compatibility flag, which would make the defect a supported configuration.

Testing

278 unit tests green across the index and reindex packages, 12 of them new:

• ReindexQueueFactoryBatchKeyTest (5) — collision resolution, both arrival orders, dedup preserved
• ContentletIndexOperationsESPartialFailureTest (4) / ...OSPartialFailureTest (3) — raise on partial failure, silent on clean and null responses, message wording

8 new integration tests, registered in MainSuite1b/MainSuite2b, NOT yet run locally. The Docker daemon died partway through verification. They compile and are wired into their suites; they need to go green in CI before this merges.

• ReindexDeleteJournalTest (3) — durable entry written in-transaction, rollback leaves none, an exhausted removal stays discoverable above ERROR priority
• ContentletDestroyIndexRemovalTest (2) — removal applied once the write path recovers; unpublish leaves other languages live
• ContentletIndexPartialFailurePhaseTest (2) — ADR-0009 shadow isolation, ordinary writes unaffected
• ContentletIndexProviderSkipTest (2) — primary pointer failure surfaces, shadow skip still completes

⚠️ ContentletIndexProviderSkipTest empties the indicies table to force the primary-provider failure and restores it in a finally. It is the most fragile test here and may deserve a different injection mechanism — flagging it rather than hiding it.

Not in scope

Repairing documents already orphaned (a full reindex remains the remedy); retrying past REINDEX_MAX_FAILURE_ATTEMPTS; the unpublish/archive path; push-publish bundle handling; and the four other null → continue provider-skip sites outside the delete path.

🤖 Generated with Claude Code

This PR fixes: #37276

[fabrizzio-dotCMS]

Integration tests now verified locally — the caveat in the description is resolved.

All 9 integration tests pass, run together in one JVM: ReindexDeleteJournalTest 3/3, ContentletDestroyIndexRemovalTest 2/2, ContentletIndexPartialFailurePhaseTest 2/2, ContentletIndexProviderSkipTest 2/2. 278 unit tests still green.

Getting there produced one substantive code change, pushed in 36b77be:

A second silent path on the removal, adjacent to L2. loadProviderIndicesQuietly returns null only when loading the pointers throws. When they load but hold no active index, it returns a non-null ProviderIndices with null slots — no delete operations are added, putToIndex early-returns on the empty batch, and the caller sees success just as invisibly as in the throwing case. The guard now counts the delete operations the primary contributed rather than testing for a null ProviderIndices, so zero operations means the removal did not happen whichever way the primary came up empty.

That one is worth a reviewer's eye: it is a real widening of the check, found only because the integration test could not make the original narrow guard fire.

Two notes on how the verification went, since they affect how you'd reproduce it:

• dotcms-integration unpacks the installed dotcms-core WAR, not target/classes. Running mvn compile and then verify silently tests whatever WAR was last installed — three runs were spent chasing a phantom failure in code that was never executing. mvn install -pl :dotcms-core -DskipTests first.
• ContentletIndexProviderSkipTest's injection needed a cache flush: emptying the indicies table alone does nothing because IndiciesAPI reads through IndiciesCache. It now restores via IndiciesAPI#point instead of leaving the table empty for the rest of the suite — the fragility I flagged in the description is addressed, though it still mutates shared state and is the test I'd watch first if CI goes red.