JanusGraph / JanusGraph/janusgraph
IndexSerializer.removeElement rejects String element ids, breaking mixed-index recovery for custom vertex id types
- Dominant language
- Java
- Stars
- 5.8k
- Forks
- 1.2k
- Avg merge
- 13h 53m
- Merged PRs (30d)
- 6
Description
- Version: `master` (`ac0eb23`)
- Storage Backend: any
- Mixed Index Backend: any
- Expected Behavior: mixed-index transaction recovery should work on graphs configured with `graph.allow-custom-vid-types`.
- Current Behavior: `IndexSerializer.removeElement` rejects String element ids, so recovering a *deleted* element throws `IllegalArgumentException` and aborts the restore.
### Details
https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/IndexSerializer.java#L334-L338
```java
public void removeElement(Object elementId, MixedIndexType index, Map>> documentsPerStore) {
Preconditions.checkArgument((index.getElement()==ElementCategory.VERTEX && elementId instanceof Long) ||
(index.getElement().isRelation() && elementId instanceof RelationIdentifier),"Invalid element id [%s] provided for index: %s",elementId,index);
getDocuments(documentsPerStore,index).put(element2String(elementId),new ArrayList<>());
}
```
The precondition allows only `Long` for vertices. But the very next line calls `element2String`, which explicitly supports String ids:
https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/util/IndexRecordUtil.java#L100-L107
```java
public static String element2String(Object elementId) {
Preconditions.checkArgument(elementId instanceof Long || elementId instanceof RelationIdentifier || elementId instanceof String);
...
}
```
So the two methods disagree about what a valid element id is, and the stricter one runs first.
The only caller is the transaction-log recovery path:
https://github.com/JanusGraph/janusgraph/blob/ac0eb2392ddad3d96ab8cde2a5a9a123dbc5d839/janusgraph-core/src/main/java/org/janusgraph/graphdb/log/StandardTransactionLogProcessor.java#L246-L255
```java
JanusGraphElement element = restore.retrieve(tx);
if (element!=null) {
graph.getIndexSerializer().reindexElement(element,index,restoredDocs);
} else { //Element is deleted
graph.getIndexSerializer().removeElement(restore.elementId,index,restoredDocs);
}
```
So on a graph using custom String vertex ids, recovering a transaction whose element was deleted throws instead of removing the stale index document — meaning the recovery mechanism that exists to repair index divergence cannot complete.
### Steps to Reproduce
1. Configure a graph with `graph.allow-custom-vid-types=true` and `tx.log-tx=true`, with a mixed index on vertices, using String vertex ids.
2. Start transaction recovery (`JanusGraphFactory.startTransactionRecovery`).
3. Cause a transaction that deletes an indexed vertex to fail on the index backend.
4. Observe: recovery throws `IllegalArgumentException: Invalid element id [...] provided for index` and the stale document is never removed.
### Suggested Fix
Align the precondition with `element2String`:
```java
Preconditions.checkArgument(
(index.getElement()==ElementCategory.VERTEX && (elementId instanceof Long || elementId instanceof String)) ||
(index.getElement().isRelation() && elementId instanceof RelationIdentifier),
"Invalid element id [%s] provided for index: %s", elementId, index);
```
Contributor guide
Research direction
Start in janusgraph-core/src/main/java/org/janusgraph/graphdb/database/IndexSerializer.java and compare removeElement with element2String in IndexRecordUtil.java. Trace the deleted-element path through StandardTransactionLogProcessor.java and reproduce recovery with custom String vertex ids. Done means recovery removes the stale index document without rejecting the String id.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100