Skip to content

fix(storage): recover edge vector indices from a single vector source - #4959

Merged
as51340 merged 6 commits into
masterfrom
fix/vector-edge-index-recovery
Oct 3, 2026
Merged

as51340 merged 6 commits into
masterfrom
fix/vector-edge-index-recovery

Conversation

@as51340

@as51340 as51340 commented Sep 29, 2026

Copy link
Copy Markdown
Contributor

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:

  1. Silent embedding loss. The snapshot writes the edge section first, resolving each tag's floats from the live index, and exports the usearch contents later. A vector removed in a transaction that later aborted is present in the edge section but absent from the side table. RecoverIndex then fell back to AddEdgeToIndex, which asked the index being built for the vector, got an empty vector, and UpdateVectorIndex treated that as an abort. The edge ended up tagged as a member with no usearch entry and no floats anywhere, with nothing logged.
  2. Dangling id after drop. UpdateOnIndexDrop only 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 through IndexedPropertyDecoder, asked a nonexistent index, and threw. If a second index on the same property was recovered afterwards, AddEdgeToIndex read the dropped id first and threw during recovery, aborting every restart.
  3. Rebuild order. The edge-type+property and global edge property rebuilds ran before the edge vector rebuild, so they indexed the plain-list form of a property that the vector rebuild then converted to a tag.

Changes

  • vector_edge_index.hpp/.cpp: VectorEdgeIndexRecoveryInfo holds only the spec. VectorEdgeIndexRecovery::EdgeVectors is one PropertyId -> (Gid -> vector) map. RecoverIndex is replaced by RecoverAllVectorEdgeIndices, 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. UpdateOnSetEdgeProperty captures 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. UpdateOnIndexDrop walks all edges to demote tags only when the last spec on a property goes.
  • snapshot.cpp: LoadPartialEdges captures each tag's vector before InitProperties discards it, merging into the shared map once per batch under a mutex. The vector section of the snapshot only fills gaps with try_emplace. All loaders from v22 onward pass the sink through LightEdgeLoader::RecoverEdges.
  • wal.cpp: the edge SET_PROPERTY hook 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: adds edge_vectors to 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* and UpdateOnIndexDrop*: capture, erase, orphan demotion, last-spec demotion with the null path, surviving-spec preservation.
  • The three existing populate tests are ported to the new entry point.

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.

@as51340 as51340 added CI -build=community -test=core Run community build and core tests on push CI -build=coverage -test=core Run coverage build and core tests on push CI -build=jepsen -test=core Run jepsen build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=debug -test=integration Run debug build and integration tests on push CI -build=release -test=core Run release build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push CI -build=release -test=stress Run release build and stress tests on push CI -build=coverage -test=clang_tidy labels Sep 29, 2026
@as51340 as51340 closed this Sep 29, 2026
@as51340 as51340 reopened this Sep 29, 2026
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

This PR has potential conflicts with the following other open pull requests which modify the same files:

@andrejtonev

andrejtonev commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Reviewed e0e257f. The single-source design looks right, and [] now survives both WAL and snapshot restarts. One regression and one pre-existing gap are worth fixing here.

Should fix

  • R1 (regression, debug builds), src/storage/v2/indices/vector_edge_index.cpp:492: [] is now a plain list, so emb: [] → SET r.emb = [1.0, 2.0] → ROLLBACK reaches DMG_ASSERT(old_value.IsNull()) in AbortEntries. The before-image is the plain []. Release builds take the right path, which removes the edge from the index. Fix: drop the assert, since any non-tag before-image just leaves the index.
  • R2 (pre-existing), src/storage/v2/indices/vector_edge_index.cpp:106 (AddEdgeToIndex): CREATE VECTOR EDGE INDEX over edges that already hold [] tags them with no usearch entry. After DROP VECTOR INDEX, reading r.emb throws Vector edge index ve does not exist. I reproduced this on a local build. It's the last path that still creates an empty tag. Fix: return early when !property.IsVectorIndexId() && property.IsAnyList() && property.ListSize() == 0, as AddVertexToIndex does in fix(storage): recover vertex vector indices from a single vector source; keep [] a plain list #4953.

Low

  • vector_edge_index.cpp:247-251: if one DropIndex in the recovery catch throws, the remaining indexes are skipped and the original error is lost. Wrap each call in its own try/catch and rethrow the original, as the vertex side in fix(storage): recover vertex vector indices from a single vector source; keep [] a plain list #4953 does.
  • tests/e2e/durability/durability_with_vector_edge_index.py:448-456: the [] case only covers WAL replay. Worth adding a snapshot variant, the R1 rollback and the R2 create-then-drop cases.

Nits

  • :194: vec = std::move(*maybe_vec);
  • :200-208: take edge_endpoints_mutex_ once after the index loop (when !member_ids.empty()), not once per matching index.
  • :723: log the property name, as :186 does.
  • :705: UpdateOnIndexDrop walks deleted vertices/edges. It's harmless, but it's inconsistent with the final build.

I checked these and they're fine: the is_permutation skip (ids are stored by name), DROP racing a snapshot (READ vs UNIQUE), and the string_view in UpdateOnIndexDrop (owning copy at :689). On a local RelWithDebInfo build of e0e257f: unit vector_edge_index 40/40, e2e durability_with_vector_edge_index 16/16, and the [] set/create/index-over/overwrite cases all recover correctly from both WAL and snapshot.

@andrejtonev andrejtonev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline notes for the summary comment above (R1, R2, the catch loop, test gaps and nits).

Comment thread src/storage/v2/edge_accessor.cpp
Comment thread src/storage/v2/indices/vector_edge_index.cpp
Comment thread src/storage/v2/indices/vector_edge_index.cpp Outdated
Comment thread src/storage/v2/indices/vector_edge_index.cpp Outdated
Comment thread src/storage/v2/indices/vector_edge_index.cpp Outdated
Comment thread src/storage/v2/indices/vector_edge_index.cpp
Comment thread src/storage/v2/indices/vector_edge_index.cpp Outdated
Comment thread tests/e2e/durability/durability_with_vector_edge_index.py

@andrejtonev andrejtonev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. CI: Release / End to end tests is red because of the new rollback_overwrite_of_empty_list case (inline). It's a test bug, not a product bug.
  2. 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 bug and Docs - changelog only.
  • Rebase: #4953 (vertex) and this PR conflict in durability.cpp, metadata.hpp, snapshot.cpp and wal.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 from tempfile.TemporaryDirectory() to the test_name / get_data_path / get_logs_path fixtures that #4973 introduced for this file.

Comment thread tests/e2e/durability/durability_with_vector_edge_index.py Outdated
Comment thread src/storage/v2/edge_accessor.cpp
@as51340 as51340 added bug bug Docs - changelog only Docs - changelog only labels Oct 1, 2026
@as51340
as51340 force-pushed the fix/vector-edge-index-recovery branch from 8a91247 to 21e81e0 Compare October 1, 2026 12:56
@as51340
as51340 requested a review from andrejtonev October 1, 2026 12:58
@as51340
as51340 added this pull request to the merge queue Oct 2, 2026
@as51340

as51340 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Tracking

  • [Link to Epic/Issue]

Standard development

CI Testing Labels

  • Select the appropriate CI test labels (CI -build=build-name -test=test-suite)

Documentation checklist

  • Add the documentation label
  • Add the bug / feature label
  • Add the milestone for which this feature is intended
    • If not known, set for a later milestone
  • Write a release note, including added/changed clauses
    • What has changed? What does it mean for a user? What should a user do with it?
    • Vector edge indexes now recover each edge's embedding from a single source, so a crash during a snapshot can no longer leave an edge marked as an index member while its embedding is lost or its index reference points at a dropped index, which previously made the property unreadable and could abort recovery on every restart; setting an indexed edge property to an empty list now stores a plain empty list and removes the edge from the index instead of producing a broken reference, including when the write arrives from an older main during a rolling upgrade. Existing snapshots and WAL files recover without any migration and no action is needed. #4959
  • [ Documentation PR link memgraph/documentation#XXXX ]
    • Is back linked to this development PR

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 2, 2026
@as51340
as51340 added this pull request to the merge queue Oct 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 2, 2026
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.
@as51340
as51340 force-pushed the fix/vector-edge-index-recovery branch from 21e81e0 to 746ed0c Compare October 2, 2026 19:41
@as51340
as51340 enabled auto-merge October 2, 2026 19:44
@as51340 as51340 removed CI -build=community -test=core Run community build and core tests on push CI -build=coverage -test=core Run coverage build and core tests on push CI -build=jepsen -test=core Run jepsen build and core tests on push CI -build=debug -test=core Run debug build and core tests on push CI -build=debug -test=integration Run debug build and integration tests on push CI -build=release -test=core Run release build and core tests on push CI -build=release -test=e2e Run release build and e2e tests on push CI -build=release -test=stress Run release build and stress tests on push CI -build=coverage -test=clang_tidy labels Oct 2, 2026
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@as51340
as51340 added this pull request to the merge queue Oct 2, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 2, 2026
@as51340
as51340 added this pull request to the merge queue Oct 3, 2026
@as51340
as51340 removed this pull request from the merge queue due to a manual request Oct 3, 2026
@as51340
as51340 added this pull request to the merge queue Oct 3, 2026
Merged via the queue into master with commit 547932f Oct 3, 2026
25 checks passed
@as51340
as51340 deleted the fix/vector-edge-index-recovery branch October 3, 2026 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug bug Docs - changelog only Docs - changelog only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants