Conversation
Ignition
added
CI -build=community -test=core
Contributor
Author
Tracking
Standard development
CI Testing Labels
Documentation checklist
|
Ignition
changed the title
Ignition
marked this pull request as ready for review
August 14, 2026 20:13
Ignition
enabled auto-merge
August 18, 2026 13:04
Ignition
removed
CI -build=community -test=core
…ytical mode An index entry holds a raw pointer to the vertex or edge it names, and nothing keeps that object alive on the entry's behalf. The only thing between a reader and a dangling pointer is that a collection pass removes an entry before the object it names leaves `vertices_`/`edges_`. Analytical broke that in three independent ways, each of which strands entries that then name freed memory for every later reader of the index -- the next sweep, or any scan. Reported as an ASAN use-after-free under concurrent MERGE/CREATE against an edge-type+property index. The sweeps left entries at or past the oldest active start timestamp alone, so that a transaction which started before such an entry was written could still reach the version it names. Analytical keeps no such versions: a write lands in place, and a delete removes the object in the very pass whose sweep skipped its entry. One open transaction holds that condition true indefinitely. The skip is now gated on the storage mode. The analytical arms then removed whatever was deleted at the moment they ran, having swept the indexes earlier in the same pass, so a delete landing in that window had its object removed with its entries left behind. The collectable set is now fixed before the sweeps and exactly that set is removed afterwards; anything deleted later waits for the next pass, which is how the transactional arms have always behaved. The full scan that built the set at removal time now runs once, earlier, so the pass does no more work than before. Deciding collectability also read each object's `deleted` flag and delta pointer without a lock. Both live in the same word and a concurrent delete writes them, so the read that decides whether to remove an object raced the write that made it collectable; light edges compounded it, being reachable only through vertex adjacency that analytical writes mutate in place. Both reads are now synchronised, and adjacency is copied out under the vertex lock before anything is decided about the edges it names. The tests run every case against both storage modes and both edge representations, since the ordering that has to hold is the same for all four while the paths delivering it are not. The reproducer is deterministic and single-threaded: hold one transaction open, create an object with an indexed property, delete it, collect, and the index is left holding an entry for something that no longer exists. ThreadSanitizer reported the two races under sustained concurrent churn and is clean on all four configurations now.
Which indexes a GC pass sweeps is decided by what the transactions since the last pass wrote, and that was read off their deltas as the GC unlinked them. An analytical transaction produces no deltas, so nothing armed anything: the only sweep an analytical workload ever ran was the forced all-indexes one a deletion triggers. A write that supersedes an index entry does not remove the old one -- the sweep is what collapses them -- so an analytical workload that only creates and updates grew its indexes with every write and never collected. Fifty updates to one indexed property left fifty-one entries where transactional left one, and each of them is walked and dereferenced by every scan of that index. Have the analytical write path note the label or property it touched on its transaction, and hand that to the next collection cycle at commit or abort (an analytical abort undoes nothing, so its writes still count). Sweeps in analytical are now armed by what was written rather than only by deletions. Transactional arming still comes from the deltas; the transaction-side arming stays empty and allocation-free there.
An aborting transaction removed the vertices and edges it had created from `vertices_`/`edges_` on its own thread. Nothing serialises that against a GC sweep or a query scan walking an index entry that names one of those objects: the removal tags the skip-list node with the newest accessor id, which no reader that started afterwards holds back, so the node can be freed under a reader that never had the chance to pin it. Pinning the object stores across each sweep covers the other half of that race, removals that happen after the pin was taken, and cannot cover this one. Aborts now hand their objects to `deleted_edges_`/`deleted_vertices_`, which is where committed deletes already went, so every removal happens on the GC thread and lands after that same pass's index cleanup. Light and heavy edges no longer need separate treatment at abort, and the edge-metadata removal has a single site again rather than one per path. The pins stay. They are what makes a scan safe against the GC pass removing objects underneath it, which is unchanged, and keeping them means the sweeps do not depend on remaining the only remover to stay correct. Aborted objects now stay in storage until the next collection, as deleted ones always have. They are invisible to queries throughout, and the store-size readouts the new tests use pin both halves: nothing leaves storage at abort time, and everything handed over is collected rather than leaked.
An index entry holds a raw pointer that nothing keeps alive, so an object may only leave storage once the collection pass has removed every index entry naming it. That ordering was spread over four removal loops, each with its own accessor and its own statement of what had to come first. The loops become RetireVertices, RetireEdges and RetireLightEdges. When an object may be removed is unchanged; the rule now has one place to live, so a call site can be checked by reading one function rather than the whole pass, and a removal added elsewhere stands out in a file where nothing else removes. RetireLightEdges also takes over reading the graveyard guard epoch, which has to be read as the retirement happens: it is what orders a reader that opened earlier ahead of the drain that frees the edge. The analytical vertex arm now asserts that the removal happened. An object reaching it twice would be in both of the pass's collection sets, which the light-edge arm turns into a double free, so the assert is worth more than an ignored return value.
A light edge is named only by the adjacency of the two vertices it joins, and deleting an edge erases it from both, so a deleted light edge cannot be reached from the store at all. The delete hands it to the collector directly for that reason. The analytical collection pass scanned for them anyway, copying each vertex's out-edge list out under its lock to walk it safely. The adjacency container keeps nothing inline, so that copy was a heap allocation for every vertex with an edge, on a scan that could never find anything. The scan goes, and with it the iterator built to serve it. A pass takes its objects from two sources: the list transactions hand over, and, in analytical, a scan for objects whose deltas are gone. Both name an object once it has been deleted, and retiring one twice removes it from storage twice. The scan now skips whatever the handover has already claimed, so the two sets are disjoint where they used to rely on an argument about which storage mode fills which. The soak tests that put sustained concurrent traffic through the collector go in the concurrent suite, which is invoked separately from the unit tests. They assert only that nothing has drifted once the traffic stops, so what they buy is a workload to run under a sanitizer rather than a statement about ordering. They arrive here because the light-edge handover above is what makes them pass.
Objects waiting to be collected were held one per list node, so a delete paid an allocation per object and three times the object's own size to store an eight byte identifier, and the collector then walked that chain a node at a time to retire them. They now go into batches: elements are appended into the batch at the tail, and a handover moves whole batches. The handover stays what it was, a constant time splice under the spin lock with no allocation inside it, which is the property the shared containers are a list for in the first place. A batch stops growing at the smallest size worth its own allocation, and holds its first elements inside itself, so handing over a single object still costs a single node exactly as it did before. For a large delete this cuts the time to record the objects and the time to walk them to retire them to a fifth, and what they occupy while waiting to about a third. A delete of one object per transaction is within a few percent either way and occupies the same. The benchmark measures both containers on the same handover shapes.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 18, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 19, 2026
|
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 19, 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
2 participants
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.
An index entry holds a raw pointer to the vertex or edge it names, and nothing
keeps that object alive on the entry's behalf, so a collection pass has to
remove the entry before the object leaves the store. Analytical mode broke that
rule three ways: its sweeps spared entries at or past the oldest active start
timestamp, which is sound only where an older transaction can still reach the
version an entry names; its arms removed whatever was deleted at the moment they
ran, having swept earlier in the same pass; and it decided collectability by
reading an object's deleted flag and delta pointer without holding its lock. The
collectable set is now fixed before the sweeps, exactly that set is removed
afterwards, and those reads are synchronised.
Sweeps were armed off transaction deltas, which an analytical transaction does
not produce, so its indexes grew with every write and collected only when a
deletion forced a sweep; the analytical write path now notes the label or
property it touched. Removal moves entirely to the garbage collector: an
aborting transaction used to remove its own objects on its own thread, which
holds back neither a concurrent sweep nor a reader that started afterwards, and
now hands them over as committed deletes already did. The pass no longer scans
for deleted light edges, which cannot be reached from the store at all, skips
whatever the handover has already claimed so that nothing is retired twice, and
holds waiting objects in batches rather than one list node each.