Skip to content

deleteVertex can delete a vertex without collecting its edges when a chunk is transiently unreadable (the #5670 window, vertex side) #5680

Description

@lvca

Summary

#5670 (PR #5678) closed the window where deleting an EDGE could leave its back-reference behind: a
chunk that is momentarily unreadable - a concurrent commit publishes its pages one at a time, so a
vertex page can expose a new edge-list head RID before that head's page is visible, and an emptied
chunk is relinked out of the chain under a walker's feet - was read as "nothing to remove here"
while the edge record was deleted anyway.

GraphEngine.deleteVertex still reads its own edge lists through the best-effort
getEdgeHeadChunk, and wraps the collection loop in catch (Exception):

try {
  outEdges = getEdgeHeadChunk(vertex, Vertex.DIRECTION.OUT);
  if (outEdges != null) { ... collect edges to delete ... }
} catch (Exception e) {
  // LINKED LIST COULD BE BROKEN
  LogManager.instance().log(this, Level.WARNING, "Error on deleting outgoing edges connected to vertex %s", e, ...);
}
...
vertex.getDatabase().getSchema().getBucketById(...).deleteRecord(vertex.getIdentity(), force);

So if the head chunk is unreadable at that instant, outEdges is null, no edges are collected,
and the vertex record is deleted regardless - leaving edges pointing at a vertex that no longer
exists (ghost edges), reported by check database as invalid links.

Why it was left out of #5678

This is a deliberate, tested contract, not an oversight: deleteVertex's javadoc states that edge
disconnection is best-effort, and Issue4420TolerantDeleteTest / Issue4432CorruptVertexDeleteTest
pin that a vertex whose chain is structurally broken stays deletable. Making the collection strict
would trade repairability for strictness, and that is a decision worth taking on its own merits
rather than as a side effect of an edge-delete fix.

Note that deleteVertex does get #5670's fix transitively where it matters most: every edge it does
collect is removed via edge.delete() -> deleteEdge, which is now strict, so a NEIGHBOUR's
back-reference is no longer left dangling.

What a fix would look like

Distinguish the two cases the single getEdgeHeadChunk call currently conflates:

The open question is what an ordinary deleteVertex should do when the chain is genuinely broken
(not transiently unreadable), since the two are indistinguishable at that point: today it deletes and
warns; strict handling would require force to get the vertex out. That is the trade-off to decide.

Verification

A test in the shape of Issue5670EdgeDeleteDanglingBackRefTest - make the vertex's head chunk
unreadable, delete the vertex, and assert its edges do not outlive it - would pin whichever answer is
chosen.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions