Repository navigation
Leave out removed vertices before sorting a query in memory - #5100
Merged
porunov merged 1 commit intoOct 17, 2026
Merged
Conversation
Fixes JanusGraph#5099 A graph query ordered by a key which its index doesn't sort it by, such as one which a composite index answers, or a mixed index which doesn't hold the order key, failed with IllegalStateException: Vertex with id N was removed, in a transaction which had removed one of the vertices it matched. QueryProcessor's PreSortingIterator sorted what the index returned, reading the order key of each element, before getFilterIterator left out the elements which the transaction had deleted, and the order key of a removed vertex can't be read. An unordered query, which reads no order key, and one which the index sorts, which JanusGraph doesn't sort, weren't affected, nor were the edges and properties which the transaction removed, whose ids turn into no element. The pre-sorting iterator now leaves out the elements which the transaction removed (isRemoved) before it sorts the others. getFilterIterator still leaves out the other elements which the transaction deleted, such as those which it changed so that they no longer match, as the sorted elements are reached: only the elements which are read are checked, and a change made while they are read is seen, as for an unsorted query. The check of how many elements a pre-sorting read fetched counts every element which the backend returned, as before. Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Oleksandr Porunov <alexandr.porunov@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused fix preserves existing limits and lazy filtering, includes targeted regression coverage, and has no identified blocking issues.
0 open findings
What changed in this PR
Fixes #5099 by excluding removed elements before JanusGraph sorts query results in memory, avoiding reads of removed vertices’ properties.
Changes:
- Preserves backend fetch limits and lazy deletion checks.
- Adds unit and backend regression tests for removals before sorting and during iteration.
- Documents the fix in the changelog.
| File | Description |
|---|---|
| janusgraph-core/src/test/java/org/janusgraph/graphdb/query/QueryProcessorTest.java | Tests filtering removed elements before sorting. |
| janusgraph-core/src/main/java/org/janusgraph/graphdb/query/QueryProcessor.java | Skips removed elements while counting all fetched results. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphTest.java | Covers composite indexes and mutations during iteration. |
| janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java | Covers sorting with mixed indexes. |
| docs/changelog.md | Explains the corrected query behavior. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Oct 15, 2026
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.
Fixes #5099
A graph query ordered by a key which its index doesn't sort it by, such as one which a composite index answers, or a mixed index which doesn't hold the order key, failed with
IllegalStateException: Vertex with id N was removedin a transaction which had removed one of the vertices it matched.QueryProcessor'sPreSortingIteratorsorted what the index returned, reading the order key of each element, beforegetFilterIteratorleft out the elements which the transaction had deleted. The query now leaves out the removed elements before it sorts, and the other deleted ones as before.Changes
QueryProcessor.PreSortingIteratorleaves out the elements which the transaction removed (isRemoved()), whose order values can't be read, before it sorts what the backend returned. Its check against fetchingMAX_SORT_ITERATIONelements still counts every element which the backend returned.getFilterIteratorstill leaves out the other elements which the transaction deleted, such as those which it changed so that they no longer match, as the sorted elements are reached, as for an unsorted query: only the elements which the caller reads are checked, and a change which it makes while it reads them is seen.Tests
JanusGraphTest.testQueriesSortedInMemorySkipTheElementsWhichTheTransactionRemoved(every storage backend; composite indexes of vertices and of edges bykind): in a transaction which drops a vertex, and with it its edge, and another edge,g.V().has('kind', 'p').order().by('name'), the same descending with a limit,graph.query().has('kind', 'p').orderBy('name', asc).vertices(), the same orders of edges, and an order of a vertex's edges bynameinlocal(), return the vertices and edges which are left, in order.JanusGraphIndexTest.testQueriesSortedInMemorySkipTheElementsWhichTheTransactionRemoved(every index backend; mixed indexes namedmixedandedges, which the Solr test container creates, holdingkindbut notname): the same.JanusGraphTest.testQueriesSortedInMemoryLeaveOutTheElementsRemovedWhileTheyAreRead(every storage backend; a composite index of vertices bykind): a vertex which the transaction removes, or whosekindit changes, while it readsgraph.query().has('kind', 'p').orderBy('name', asc).vertices()is left out when it is reached, and so is an edge which it removes while it readsv.query().labels('road').orderBy('name', asc).edges()of a vertex whose properties it changed, as for unordered queries.QueryProcessorTest.leavesOutTheRemovedElementsBeforeSortingWhatTheBackendReturns(no backend): of what a mocked executor returns, a removed element, whose order value can't be read, is left out before the others are sorted.On master
dd4f0f845, bothtestQueriesSortedInMemorySkipTheElementsWhichTheTransactionRemovedtests fail at their first query withIllegalStateException: Vertex with id N was removed., on inmemory, Lucene, Elasticsearch and Solr, and so does theQueryProcessorTestcase. The orders of edges, whose removed ids turn into no element, returned the edges which are left already.testQueriesSortedInMemoryLeaveOutTheElementsRemovedWhileTheyAreReadpasses on master, which checks the sorted elements as they are reached too.Mutations:
PreSortingIterator, which sorts the removed elements with the others, fails bothtestQueriesSortedInMemorySkipTheElementsWhichTheTransactionRemovedtests and theQueryProcessorTestcase, as on master.executor.isDeletedon each element which the backend returns) instead of as the elements are reached, as a first version of this change did, fails each of the three parts oftestQueriesSortedInMemoryLeaveOutTheElementsRemovedWhileTheyAreRead: the removed vertex is reached and its name can't be read, and the changed vertex and the removed edge are returned.Locally, Java 11, with checkstyle (no violations):
-Dtest.skip.tp=false,InMemoryJanusGraphProcessTestandInMemoryMultiQueryJanusGraphProcessTest, 1,860 tests, 80 skipped) pass.BerkeleyElasticsearchTest(118, 1 skipped) passes against Elasticsearch 9.5.4 in a container.JanusGraphIndexTest.testQueriesSortedInMemorySkipTheElementsWhichTheTransactionRemovedpasses on BerkeleyJE against an in-JVMMiniSolrCloudCluster(Solr 9.10.1), asBerkeleySolrTestruns it against its container, and fails there with master'sQueryProcessorwith theIllegalStateException. The Solr test container is left to CI.Notes
SubqueryIteratorturns the ids of an index query into elements, this PR merges without conflicts; it changes only whatQueryProcessordoes with the elements.isRemoved(), which reads nothing, is checked before the sort, so the query does no more work than on master. Checking every deletion there instead costs a scan of the vertex's added relations of a constrained label for each element which the backend returns, where the check as the elements are reached takes only those which the caller reads: on inmemory,v.query().direction(OUT).labels('child').orderBy('name', asc).limit(10).edges()on a vertex with 20,000 edges of aONE2MANYlabelchild, in a transaction which added 2,000 more, took 1.8 to 2.0 s that way, against 22 to 33 ms on master and 23 to 32 ms with this change (0.21 to 0.23 s, against 36 to 49 and 31 to 36 ms, with 200 added edges; the median of 7 timed runs after 3 untimed ones, in each of two runs).🤖 Generated with Claude Code