JanusGraph / JanusGraph/janusgraph
Deleting vertex followed by adding property in nested transactions leads to wrong index entry
- Dominant language
- Java
- Stars
- 5.8k
- Forks
- 1.2k
- Avg merge
- 13h 53m
- Merged PRs (30d)
- 6
Description
Look at this test case:
https://github.com/JanusGraph/janusgraph/blob/33967fdb9144be2f2013e96c684eed3a8ce86251/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java#L1695-L1698
which calls this method (initialValue = null, updatedValue != null):
https://github.com/JanusGraph/janusgraph/blob/33967fdb9144be2f2013e96c684eed3a8ce86251/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java#L1786-L1817
It creates two nested transactions. The first transaction removes the vertex and then commits, while the second transaction adds a property value and then commits. Looking at line 1816:
https://github.com/JanusGraph/janusgraph/blob/33967fdb9144be2f2013e96c684eed3a8ce86251/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java#L1816
This assertion does not check the secondary store, because the mixed index is built upon full-text search, which does not support equality check. In other words, this traversal simply checks storage backend.
However, if we make the index using string search (which means it supports equality check), as below (https://github.com/li-boxuan/janusgraph/commit/d9d79259295e):
```java
mgmt.buildIndex("mixedIndex", Vertex.class).addKey(prop, Mapping.STRING.asParameter()).buildMixedIndex(INDEX);
```
Then this test case will start to fail:
https://travis-ci.org/github/li-boxuan/janusgraph/jobs/711858999#L1397
`CQLElasticsearchTest>JanusGraphIndexTest.testDeleteVertexThenAddProperty:1708->JanusGraphIndexTest.testNestedWrites:1816 expected: but was: `
So this test case itself, if fixed as mentioned above, demonstrates the bug: the second transaction commits into secondary index store, even if this vertex does not exist anymore.
Contributor guide
Research direction
Start with JanusGraphIndexTest.testDeleteVertexThenAddProperty and testNestedWrites, especially the assertion around line 1816. Run the case through CQLElasticsearchTest with the string-mapped mixed index described in the issue, then trace the nested transaction commits and secondary-index updates. Done means the deleted vertex leaves no entry in the secondary index after the later transaction commits.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elasticsearch, java
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100