feat(ai): make the keyword search index resident and incremental - #825
Open
E2ern1ty wants to merge 3 commits into
Open
feat(ai): make the keyword search index resident and incremental#825E2ern1ty wants to merge 3 commits into
E2ern1ty wants to merge 3 commits into
Conversation
…lace The global keyword search rebuilt a throw-away Lucene index from a full graph scan on every query, so index construction cost was paid on the query path and discarded afterwards. Verbalization was also redone for the whole graph per query, and multi-round sessions ran a global search whose result was dropped. Follow the standard inverted index maintenance model instead of invalidate-and-rebuild: - Add ResidentSearchIndex, a graph scoped index built at most once and kept alive across queries, held by GraphMemoryServer per keyword index store. - Give each document a non analyzed primary key (ModelUtils.getGraphEntityKey) so writes map to Lucene update/delete by term. Cost is proportional to the change, not to graph size, and upserts are idempotent, so callers do not have to supply an exact delta. - Split the server hook into onEntitiesUpserted / onEntitiesRemoved / onSchemaChanged; only a schema change needs wholesale invalidation. - Track validity against a graph vertex version so mutations made outside the server (for example directly through MemoryMutableGraph) are detected and force a rebuild instead of serving stale results. Accessors that cannot report a version degrade to the previous per-query rebuild behaviour. - Memoize entity verbalization in a bounded, version aware LRU cache. - Replace the close()-as-flush pattern with a near real-time refresh (commit + openIfChanged), and make ensure+search atomic under one lock. - Move the discarded global search out of the multi-round session path. - Let EmbeddingOperator enumerate what the index store holds instead of scanning the graph, resolving each entity so deleted leftovers are filtered. Recall is unchanged: the document set, query string, analyzer and topN all stay the same, and equivalence against the rebuild path is asserted by tests. The rebuild path is kept as SessionOperator.searchWithGlobalGraphByRebuild for that purpose. Measured on 10000 vertices: 98.4~119.5 ms per query -> 0.41~0.46 ms steady state. On 5000 vertices with writes interleaved with queries: 44.5~47.1 ms per round -> 1.60~1.81 ms, and full builds drop from 41 to 1. MutableGraphTest goes from 0.887 s to 0.186 s. See geaflow-ai/docs/feature-resident-keyword-index.md for the design, the change list, full measurements and known limitations. The consolidate write path still rebuilds all retrieval state per insert and is tracked there.
Review of the previous commit found the version guard does not actually guard. It read the current vertex version after applying a write batch and adopted it as "everything is applied", so any mutation made outside the reporting path was silently accepted as already indexed. One later reported write was enough to swallow it, and the missing document never came back. Reproduced: build the index, add a vertex directly through MemoryMutableGraph, then upsert an unrelated vertex through the server; the first vertex becomes permanently unsearchable. Make the writer state the range instead of the reader guessing it. Add VertexVersionWindow, opened before a batch of writes and sealed after them. A batch is applied in place only when the window proves it is complete: sealed, starting exactly at the version the index last accepted, and ending at the version the graph still reports. Anything else rebuilds. The window opened by the writer between its own writes remains a blind spot, which now needs writers to serialize rather than being papered over. Also in this area: - Register an edge schema against the edge version only. It cannot change how an existing vertex is verbalized, and bumping the vertex version made consolidate invalidate the index on its first insert for nothing. - Move schema registration into MemoryGraph so bumpVersion can be private; advancing the version was a public operation any caller could trigger. Concurrency and cost: - Search under a read lock instead of one monitor covering build, write and search, so concurrent queries no longer serialize. - Memoize verbalization in a ConcurrentHashMap with no lock on either path. The memoized function is pure and its value immutable, so exclusion is not needed for correctness; racing threads at worst duplicate work. This drops a double check that only existed to protect a cache wide version field and a remove that the following put already did. Cost: the bound is now approximate and eviction is not LRU. No throughput difference was measurable at 1 to 16 threads, the critical section is a single map get. - Stamp each cache entry with its source version, vertex entries against the vertex version. Sharing one version discarded every memoized entry on any write, worst exactly where writes are frequent: consolidate issues about thirty edge writes per inserted entity. - Drop IndexWriter#commit from refresh and open the reader from the writer. A commit point buys no durability on an in memory directory. Measured 2.14~2.42 -> 1.72~1.86 ms per round on writes interleaved with queries. - Drop the graph sized Set the index kept alongside Lucene. Delete by term is idempotent so it guarded nothing, and the document count can be read from Lucene. Also removes GraphSearchStore.entityNum, which had no reader and miscounted both upserts and deletes. - Snapshot getIndexedEntities instead of returning a live key set view, which threw ConcurrentModificationException under concurrent writes. - Build a fresh IndexWriterConfig per writer; Lucene rejects reuse, so a store that advertises a long life could not be reopened after close. - Cache the schema label sets per search, guard residentIndexes with synchronizedMap, read the cache bound from Constants at use time so it is configurable, and translate the remaining Chinese Javadoc. Corrects the design doc: it blamed the 1.7 ms write round on segment growth slowing search and proposed a merge policy. 800 rounds bucketed say otherwise, search cost falls from 0.75 to 0.21 ms while write plus refresh dominates, so Lucene's default TieredMergePolicy already handles merging. The open item is refresh batching, not a hand written merge policy. Steady state read is 0.38~0.60 ms per query on 10000 vertices, writes interleaved with queries 1.36~1.40 ms per round with one full build. 20 tests pass, three of them new: the swallowed mutation regression, edge writes keeping memoized vertex verbalizations, and consistency of the lock free cache under 8 threads.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
The global keyword search in
geaflow-airebuilt a throw-away Lucene index from a full graph scan on every query, then discarded it. Index construction cost sat entirely on the query path, verbalization was redone for the whole graph per query, and multi-round sessions ran a global search whose result was thrown away.This makes the index resident per graph and maintains it incrementally, following the standard inverted index model rather than invalidate-and-rebuild.
Retrieval
ResidentSearchIndex, built at most once and kept alive across queries, held byGraphMemoryServerper keyword index store.ModelUtils.getGraphEntityKey), so writes map to Lucene update/delete by term. Cost is proportional to the change, not to graph size, and both are idempotent, so callers need not supply an exact delta.onEntitiesUpserted/onEntitiesRemoved/onSchemaChanged. Only a schema change needs wholesale invalidation.close()-as-flush pattern with a near-real-time reader opened from the writer. Nocommit(): the directory is in memory, so a commit point buys no durability.ConcurrentHashMap, each entry stamped with the source version it was computed from, so a write invalidates only the entries it affects.EmbeddingOperatorenumerate what the index store holds instead of scanning the graph, resolving each entity so deleted leftovers are filtered out.Not serving stale results
A resident structure over a mutable graph needs to know when it went stale, including when the graph is mutated outside the server (for example directly through
MemoryMutableGraph).MemoryGraphtherefore maintains a version, split into a general one and a vertex-only one so that edge writes do not invalidate a vertex-only index.Reading the current version after applying a batch is not sufficient: it accepts any unreported change as already applied, and the affected documents then go missing permanently. So the writer states the range and the index verifies it —
VertexVersionWindowis opened before a batch of writes and sealed after them, and a batch is applied in place only when it is provably complete (sealed, starting exactly at the version the index last accepted, ending at the version the graph still reports). Anything else falls back to a rebuild. Accessors that cannot report a version degrade to the previous per-query rebuild behaviour.Recall is unchanged. The document set, query string, analyzer and
topNall stay the same, and equivalence against the rebuild path is asserted by tests. The rebuild path is kept asSessionOperator.searchWithGlobalGraphByRebuildfor that purpose.Measured (macOS arm64, 10000 vertices,
topN = 30, ranges over 3 runs):MutableGraphTestLatency also stops tracking graph size: 5000 -> 20000 vertices moves the rebuild path 45 -> 177 ms per query but the resident path only 0.31 -> 0.55 ms.
geaflow-ai/docs/feature-resident-keyword-index.mdcarries the full design, the reasoning against comparable systems (Lucene/ES, Milvus, hugegraph-ai), the complete change list, measurements and limitations.Deliberately out of scope, all recorded in the doc:
KeywordRelationFunction.eval()builds a fresh index store and server per insert, making import O(V^2). It is the only path still rebuilding per O(V). Fixing it changes theConsolidateFunction.evalcontract and deserves its own change.SearcherManager), not just dropping the lock.How was this PR tested?
mvn -B -pl geaflow-ai -am clean install: all Reactor modules SUCCESS, 20 tests pass, Checkstyle 0 violations, Apache RAT Unapproved 0.New tests,
ResidentSearchIndexTest(13) andEmbeddingCandidateSetTest(1):HashMapiteration and its tie-break order was never deterministic; match counts are kept undertopNto avoid truncation sensitivity.buildCountdoes not change.VertexVersionWindow.Existing
GraphMemoryTest(LDBC data, strict content assertions),MemoryServerTest(HTTP end to end, 532-chunk import) andMutableGraphTestall pass unchanged, which is additional evidence recall did not move.Not verified in a production environment.