Harden GigaMap consistency against exceptions from user code (indexers, update logic)

[fh-ms]

Summary

GigaMap runs user code (Indexer.index, key equality, update/apply logic, custom constraints) in the middle of mutations. On several paths the internal state (storage slots, size counters, dirty flags, sibling indices) was already mutated when that user code ran, so a thrown RuntimeException left the map inconsistent — and the next store() persisted that state. Fixes an issue, which reports the add()/set() part of this class: a failed add() leaked index entries and the not-consumed entityId was reused by the next add(), binding the leaked entries to a different entity (persisted wrong query results).

Failure semantics now enforced (documented on the GigaMap interface)

add / addAll: add() now rolls back like addAll(), whose rollback additionally became exception-tolerant: the cleanup re-runs the indexers, and if the broken indexer throws again, the secondary failure is attached as a suppressed exception and the affected ids are "burned" (never reused), so stale index entries can never alias a future entity. Remainders are repaired by reindex().

removeById / remove: the removal always completes (entity gone, size correct); the remaining indices and index groups are cleaned best-effort per index/per group and the first exception is rethrown afterwards. Residue in a broken index points at an empty id, is invisible to queries, and is repaired by reindex(). Aborting instead would make an entity with a broken indexer permanently unremovable.

set / replace: the index update — the last step that runs user code — now happens before the storage slot is overwritten, so a throw leaves the map observably unchanged. As a side effect, set-path failures no longer destructively de-index the previous entity state (new removeOnFailure flag, threaded through a binary-compatible default method on IndexGroup.Internal; the lucene/jvector modules compile unchanged).

update / apply: split into a prepare phase (indexers run on the pre-mutation state; a throw aborts cleanly with the map unchanged) and an apply phase. Once the logic may have mutated the entity, the documented destructive-removal contract — previously limited to ConstraintViolationException — applies to every RuntimeException: the unreliable entity is removed, the original exception is rethrown raw, and cleanup failures are suppressed. On a logic throw the indices are first brought in line with the entity's current state so the removal's key re-derivation finds all entries; a group whose update fails de-indexes via its prepared change handlers instead.

Dirty-marking: index add/remove loops now mark state-changed even when an indexer throws mid-loop (previously a mid-loop throw could mutate children without marking, so store() silently skipped them — the same lost-update class as review finding GM-B); conversely, clean-abort paths no longer mark when provably nothing was mutated.

Accepted limitation

On maps with multiple index groups (bitmap + Lucene/vector), a failure in a later group can still leave the groups diverged: a cross-group prepare/apply restructure would break the IndexGroup.Internal API implemented by external modules. This is documented on the affected methods; reindex() repairs such states.

Tests

• indexer/edge/ThrowingIndexerConsistencyTest — 13 scenarios: throwing single- and multi-value indexers on every CRUD path, id burning vs. reclaiming, constraint interplay, suppressed cleanup failures, and lock-state health after failed mutations.
• restart/ThrowingIndexerRestartTest — restart round-trips asserting the state persisted after a failed add/remove is consistent and repairable.
• issues/AddIndexerExceptionIdReuseReproTest — the reporter's reproducer from internal#94, adopted verbatim (red on main, green here).
• Full gigamap suite passes: 1059 tests, 0 failures.

[Copilot]

Pull request overview

This PR hardens GigaMap against exceptions thrown by user-provided code (indexers, update/apply logic, constraints) so that failed mutations don’t leave persisted state inconsistent, and documents the resulting failure semantics on the public API.

Changes:

• Add rollback / best-effort-cleanup semantics across add, addAll, remove, set/replace, and update/apply, including id “burning” when cleanup itself fails.
• Reorder set so index updates run before overwriting the storage slot; split in-place apply/update into prepare vs. apply phases.
• Add regression and restart round-trip tests for throwing indexers and the reported id-reuse corruption scenario.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
gigamap/gigamap/src/main/java/org/eclipse/store/gigamap/types/GigaMap.java	Documents new failure semantics; reorders set; adds rollback + id-burning behavior for add/addAll; broadens update/apply exception handling.
gigamap/gigamap/src/main/java/org/eclipse/store/gigamap/types/GigaIndices.java	Adds exception-safe dirty marking; introduces prepare/apply update phases; implements best-effort index cleanup behaviors across index groups.
gigamap/gigamap/src/main/java/org/eclipse/store/gigamap/types/BitmapIndices.java	Makes add/remove marking exception-safe; makes removals best-effort; adds removeOnFailure support to update indices without re-running user code.
gigamap/gigamap/src/main/java/org/eclipse/store/gigamap/types/IndexGroup.java	Adds a binary-compatible default overload for internalUpdateIndices to pass removeOnFailure semantics to index groups.
gigamap/gigamap/src/test/java/org/eclipse/store/gigamap/indexer/edge/ThrowingIndexerConsistencyTest.java	New regression suite covering throwing indexers across CRUD + update/apply failure semantics.
gigamap/gigamap/src/test/java/org/eclipse/store/gigamap/restart/ThrowingIndexerRestartTest.java	New restart round-trip tests ensuring persisted state after failed add/remove stays consistent and repairable.
gigamap/gigamap/src/test/java/org/eclipse/store/gigamap/issues/AddIndexerExceptionIdReuseReproTest.java	Adds the reported reproducer test to prevent id-reuse corruption after failed add().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.