fix(storage): recover edge vector indices from a single vector source - #4959
Conversation
|
This PR has potential conflicts with the following other open pull requests which modify the same files: |
|
Reviewed e0e257f. The single-source design looks right, and Should fix
Low
Nits
I checked these and they're fine: the |
andrejtonev
left a comment
There was a problem hiding this comment.
Inline notes for the summary comment above (R1, R2, the catch loop, test gaps and nits).
andrejtonev
left a comment
There was a problem hiding this comment.
Round 3 @ 8a91247. R1 (rollback assert), R2 (CREATE VECTOR INDEX over an existing []), the per-index DropIndex catch and the lock-order move are fixed. I checked each one against the diff. Two things left before approval, plus housekeeping:
- CI: Release / End to end tests is red because of the new
rollback_overwrite_of_empty_listcase (inline). It's a test bug, not a product bug. - Rolling upgrade (older main → newer replica) (inline on
edge_accessor.cpp). We reproduced this on the vertex side and fixed it in #4953 (580aeff). The edge path has the same gap.
Housekeeping:
- Labels: add
bugandDocs - changelog only. - Rebase: #4953 (vertex) and this PR conflict in
durability.cpp,metadata.hpp,snapshot.cppandwal.cpp. That's one hunk each, and both sides are additive (vertex next to edge), so keep both. Suggested order: #4953 merges first, then rebase this onto master. While rebasing, move the new e2e tests fromtempfile.TemporaryDirectory()to thetest_name/get_data_path/get_logs_pathfixtures that #4973 introduced for this file.
8a91247 to
21e81e0
Compare
Tracking
Standard development
CI Testing Labels
Documentation checklist
|
Snapshot load and WAL replay tracked each edge's vector in a per-index side table that was only as complete as the snapshot's usearch export. The edge section is written first, resolving each tag's floats from the live index, and the vector section is exported later, so a vector removed in a transaction that later aborted was present in the edge section but absent from the side table. Recovery then fell back to reading the vector out of the index being built, got nothing, and left the edge tagged as a member with no entry in usearch and no floats anywhere. A drop replayed against such an edge kept the dropped id in its tag, which throws on every later read of the property and aborts recovery when a second index on the same property is rebuilt. Recovery now keeps one gid -> vector map per vector-edge-indexed property: - the snapshot edge loader captures each tag's vector before the property store drops it; the snapshot vector section is only a fallback; - replayed edge SET_PROPERTY stores the tag's vector, or demotes a tag on a property no spec covers to a plain list; - dropping the last index on a property demotes every tag on it. After replay, one pass builds every vector edge index, deciding membership from each edge's type and the indexes that still exist, and freeing each vector once inserted. It runs before the edge-type+property and global edge property rebuilds so those see final property values. On-disk formats and runtime paths are unchanged. Related to #4908, which made the same change for vertex vector indices.
…very An edge property set to [] under a vector edge index was converted to a tag with no vector at runtime, and the recovery build promoted a plain [] to the same tag. Neither form has a usearch entry, so the next recovery treated the tag as lost data and stored null. - TryConvertToVectorEdgeIndexProperty leaves an empty list as a plain list; the edge is removed from the index and reads back as []. - The build pass skips empty vectors instead of tagging them. - The snapshot loader and the WAL set-property hook turn a legacy tag with no vector into an empty list so files written before this change recover to [] rather than null. - UpdateOnIndexDrop copies the index name before erasing the spec the view may alias.
An older main replicates SET e.emb = [] on an indexed edge as a tag with no vector. The replica now stores it as the plain empty list, matching what recovery already does for the same on-disk form. The e2e rollback case used BEGIN and ROLLBACK as Cypher, which is rejected; it now rolls back through a non-autocommit Bolt connection. The replay test moves to the per-test data and log path fixtures.
21e81e0 to
746ed0c
Compare
|



Summary
Edge vector index recovery had the same structural problem that #4953 fixed for vertex vector indices: the vector for each edge lived in a per-index side table that was only as complete as the snapshot's usearch export, and every replay and rebuild step trusted that table and the tag's own ids. This PR ports the single-map design to edges.
Edge types never change, so the label-ordering class of bugs from the vertex fix has no edge analogue. Three problems remain and are addressed here:
RecoverIndexthen fell back toAddEdgeToIndex, which asked the index being built for the vector, got an empty vector, andUpdateVectorIndextreated that as an abort. The edge ended up tagged as a member with no usearch entry and no floats anywhere, with nothing logged.UpdateOnIndexDroponly visited edges present in the dropped index's side table. A tagged edge missing from it kept the dropped id. Every later read of the property went throughIndexedPropertyDecoder, asked a nonexistent index, and threw. If a second index on the same property was recovered afterwards,AddEdgeToIndexread the dropped id first and threw during recovery, aborting every restart.Changes
vector_edge_index.hpp/.cpp:VectorEdgeIndexRecoveryInfoholds only the spec.VectorEdgeIndexRecovery::EdgeVectorsis onePropertyId -> (Gid -> vector)map.RecoverIndexis replaced byRecoverAllVectorEdgeIndices, a single pass that sets up every index, reads each edge's vector once (from the map for tags, from the store for lists), decides membership from the edge type and the surviving specs, and rewrites the tag only when the stored ids differ.UpdateOnSetEdgePropertycaptures the tag's vector when a spec covers the property, erases the entry for any other value, and demotes a tag on an uncovered property to a plain list.UpdateOnIndexDropwalks all edges to demote tags only when the last spec on a property goes.snapshot.cpp:LoadPartialEdgescaptures each tag's vector beforeInitPropertiesdiscards it, merging into the shared map once per batch under a mutex. The vector section of the snapshot only fills gaps withtry_emplace. All loaders from v22 onward pass the sink throughLightEdgeLoader::RecoverEdges.wal.cpp: the edgeSET_PROPERTYhook runs before the property store consumes the value, while the decoded WAL value still carries the floats. Label-style incremental bookkeeping is gone.durability.cpp: the edge vector rebuild moves ahead of the edge-type+property and global edge property rebuilds.metadata.hpp: addsedge_vectorsto the recovered indices metadata.On-disk formats and runtime paths are unchanged.
Tests
Unit (
tests/unit/vector_edge_index.cpp):RecoverAllVectorEdgeIndicesFromEdgeVectors: every edge stored as a bare tag with vectors only in the map, which is what the edge section provides when the index section lacks the gid. On master this state indexed nothing.DropThenRecreateOnSamePropertyRecoversFromEdgeVectors: tagged edges, drop the only index, create a new one on the same property. On master the drop left the tag pointing at the dropped index and the new index's rebuild threw.RecoverAllVectorEdgeIndicesResolvesEachEdgeState: plain list with a stale map entry, tag matching no spec, tag matching one of two specs, tag with no vector.UpdateOnSetEdgeProperty*andUpdateOnIndexDrop*: capture, erase, orphan demotion, last-spec demotion with the null path, surviving-spec preservation.E2E (
tests/e2e/durability/durability_with_vector_edge_index.py): a parametrized replay test with nine scenarios covering edge-before-index, repeated updates, drop, drop-then-recreate, two indexes on one property, and snapshot-then-WAL variants. These guard the new capture and map paths through real files. The two crash and loss scenarios above are only reachable through a snapshot-time race and are reproduced at unit level.Not built or run locally. CI will verify.