[hrstoyanov]

External updates to a live GigaMap bypass its mutation hooks, leaving a VectorIndex's transient search state stale. Since #861, retiring only the graph also leaves a temporary vector file with old vectors: the rebuilt graph can miss newly imported entities or return IDs that were deleted.

This PR adds VectorIndex.invalidateGraph() for replication and live binary-import integrations. It retires the in-memory graph and marks a derived temporary vector file stale. The next access clears and recovers that file from current entities before rebuilding the graph. Clearing and vectorization stay off the import path, and repeated invalidations coalesce until access.

Recovery preserves the imported vector count. Zeroing it would make recovery advance structuralModCount locally, allowing the next writer update to compare equal and skip its invalidation. Persistent vector files are left unchanged, and on-disk graph mode rejects invalidation. Lock handling supports callers holding the parent-map monitor, rejects calls from inside a search, and serializes concurrent first initialization.

Regression coverage is on this PR branch in VectorIndexInvalidateGraphTest: mutate entity vectors outside the map API, invalidate twice, verify fresh vectors and search results, verify one recovery per access, and verify the persisted witness stays unchanged. This test fails before the fix. Existing lock-order, concurrent-search, and on-disk refusal tests remain.

Compatibility with upstream #863: its discardGraph() continues to serve reindex/reset, while external invalidation preserves the imported witness. invalidationAfterReindexPreservesTheNewWitness exercises reindex followed by invalidation and verifies fresh vectors without a local counter increment.

Validation against upstream 4029cb01:

• PR-branch invalidation, lock-order and reindex tests: 24 tests, zero failures.
• Combined fork main (359fb050, this PR plus GigaMap/jvector: reuse vectors during exact graph construction #864): clean Maven install, 2,082 tests, zero failures, 11 skipped; JVector accounts for 623 tests, zero failures, 10 skipped.
• The installed jars were then checked against committed PerunCS Cluster revision 4f7f058f in an isolated source snapshot: unit 1,176 tests (2 skipped), integration 230, crash matrix 27, chaos soak 1; zero failures. Ongoing uncommitted cluster changes were excluded.
• Built jars, installed Maven jars and the 13 Store/Serializer jars resolved by TourBiz match by SHA-256.

Lucene keeps using its existing reusable close() seam. No binary layout or replication protocol changes are introduced.

Fixes #847.

[hg-ms]

Hi @hrstoyanov,

thank you for this contribution and for reporting the issue in #847.

Before we can merge this PR, the legal requirements have to be fulfilled on your side. This is required by the Eclipse Foundation, and we unfortunately can't make workarounds for it any more. Once that's done, we'll be happy to continue with
the review.
Thanks for your understanding and your support!

[hrstoyanov]

Update after #848 and #855: invalidateGraph() now follows the lock-order rule from #848.

Why. #848 established that a thread holding the GigaMap monitor must never wait for builderLock, because the lock's holders (a persist, an embedded-mode search) may need the monitor. The first version of this PR took the write lock unconditionally and its Javadoc told replication/merge layers to call it while synchronized on the map. That is the forbidden order.

What changed (VectorIndex.java, ARCHITECTURE.md):

• A caller that holds the monitor only tries the index group's write locks (internalTryLockExclusive, the same all-or-nothing attempt removeAll()/reindex() make) and wait(1)s on the monitor between attempts. While it waits, new searches of the group are held back briefly, so overlapping searches cannot starve it.
• A caller without the monitor simply waits for the lock, as before.
• Calling it from within a search of the same index (the thread holds the read lock) throws IllegalStateException instead of deadlocking on itself, matching what internalTryLockExclusive does.
• The method still never acquires the monitor itself. The Javadoc and the lock-order section of ARCHITECTURE.md now describe this.

Tests. New VectorIndexInvalidateGraphLockOrderTest, modeled on VectorIndexLockOrderTest:

• invalidation under synchronized(map) while an embedded search holds the read lock
• invalidation without the monitor waits for a running search
• invalidation from within a search is rejected
• repeated invalidation under the monitor completes while searches overlap

Without the fix, the first, third and fourth fail (deadlock, self-deadlock and starvation respectively); with it all four pass. gigamap and jvector suites pass on the branch.

[hrstoyanov]

@hg-ms @fh-ms - I signed the ECA

[fh-ms]

Hi, and thanks a lot for this PR. The problem description is excellent: the replication/binary-merge scenario is clearly explained, and it’s a valid point that the only current workaround is Unsafe-level field manipulation, which is definitely not something anyone should have to rely on. The proposal is also nicely scoped. The fail-closed behaviour for incremental on-disk mode and the test coverage show you thought this through carefully.

That said, we’ll have to postpone this one for now. The core team is currently working on larger changes in GigaMap and the vector index, and those will very likely touch the same areas this PR changes (builder lifecycle, graphRebuilt handling, deferred builder ops). Merging it now would probably lead to conflicts, or we’d end up with an API whose semantics would need to change again shortly afterwards.

We definitely want to keep the use case in mind, though. Being able to safely invalidate a stale in-memory graph after the underlying data changes outside of add/update/set is a legitimate requirement, and we’ll take it into account in the ongoing work. Once those changes have landed, we’ll get back to this and check whether the PR can be rebased, whether it’s still needed, or whether the new design already covers it.

Thanks again for the effort

[hrstoyanov]

Update after #863, in case it helps the ongoing GigaMap/vector work.

#863 lists a race this PR already fixes. #863's "Known, not addressed" says: "After a failed initialization, a search re-initializes the index without the map's monitor, so two threads can both initialize it." On main, ensureIndexInitialized() creates the builder without the monitor. This PR (d98884d) double-checks it under the parent monitor, like ensureGraphRebuilt(); the regression test is VectorIndexInvalidateGraphTest.concurrentFirstSearchesAfterInvalidationAllSeeTheFullGraph. Without it, a second initializer can replace the graph the first just rebuilt while graphRebuilt stays true, so an empty or partial graph is served for good. After #863 the same window opens when discardGraph() fails and tears the index down. If useful, I can split this fix into a small standalone PR, independent of the postponed invalidateGraph() API.

For the redesign: an invalidation must not move structuralModCount. #863's discardGraph() bumps it on purpose, so re-initialization rejects the torn-down on-disk files. invalidateGraph() does a similar teardown but must not bump it: replicas compare that counter with the writer's to decide whether an imported batch changed the index. When an earlier version of this PR let it move on the reader (zeroing vectorCount made recovery bump the counter), the reader ran one ahead and skipped every second batch. If invalidateGraph() ends up sharing discardGraph()'s teardown, the bump has to stay outside the shared part, and the reader path keeps the imported vectorCount instead of zeroing it.

Rebase. #863 touches the regions next to this PR's changes (ensureIndexInitialized/ensureGraphRebuilt, internalRemoveAll). Once it merges I'll rebase and rerun the Store reactor and our cluster suites (unit, integration, crash-matrix, soak), unless you'd rather fold the use case into the new design as you suggested on Oct 19.