- 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
|
public void removeElement(Object elementId, MixedIndexType index, Map<String,Map<String,List<IndexEntry>>> 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<>()); |
|
} |
public void removeElement(Object elementId, MixedIndexType index, Map<String,Map<String,List<IndexEntry>>> 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:
|
Preconditions.checkArgument(index!=null,"Index with name [%s] is unknown or not configured properly",indexName); |
|
Preconditions.checkArgument(index.isMixedIndex()); |
|
return (MixedIndexType)index; |
|
} |
|
|
|
public static String element2String(JanusGraphElement element) { |
|
return element2String(element.id()); |
|
} |
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:
|
SetMultimap<String,IndexRestore> indexRestores = HashMultimap.create(); |
|
BackendOperation.execute(() -> { |
|
final StandardJanusGraphTx tx = (StandardJanusGraphTx) graph.newTransaction(); |
|
try { |
|
entry.getContentAsModifications(serializer).stream() |
|
.map(m -> ModificationDeserializer.parseRelation(m, tx)) |
|
.forEach(rel -> { |
|
//Collect affected vertex indexes |
|
for (final MixedIndexType index : getMixedIndexes(rel.getType())) { |
|
if (index.getElement()== ElementCategory.VERTEX |
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
- 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.
- Start transaction recovery (
JanusGraphFactory.startTransactionRecovery).
- Cause a transaction that deletes an indexed vertex to fail on the index backend.
- Observe: recovery throws
IllegalArgumentException: Invalid element id [...] provided for index and the stale document is never removed.
Suggested Fix
Align the precondition with element2String:
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);
master(ac0eb23)graph.allow-custom-vid-types.IndexSerializer.removeElementrejects String element ids, so recovering a deleted element throwsIllegalArgumentExceptionand aborts the restore.Details
janusgraph/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/IndexSerializer.java
Lines 334 to 338 in ac0eb23
The precondition allows only
Longfor vertices. But the very next line callselement2String, which explicitly supports String ids:janusgraph/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/util/IndexRecordUtil.java
Lines 100 to 107 in ac0eb23
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:
janusgraph/janusgraph-core/src/main/java/org/janusgraph/graphdb/log/StandardTransactionLogProcessor.java
Lines 246 to 255 in ac0eb23
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
graph.allow-custom-vid-types=trueandtx.log-tx=true, with a mixed index on vertices, using String vertex ids.JanusGraphFactory.startTransactionRecovery).IllegalArgumentException: Invalid element id [...] provided for indexand the stale document is never removed.Suggested Fix
Align the precondition with
element2String: