Basic HNSW class (phase 2 - support for persistence and loading) [par… - #6131
Open
dlenev wants to merge 4 commits into
Open
Basic HNSW class (phase 2 - support for persistence and loading) [par…#6131dlenev wants to merge 4 commits into
dlenev wants to merge 4 commits into
Conversation
…t 1]. https://perconadev.atlassian.net/browse/PS-11267 Added persistence and on-demand loading APIs to HNSW implementation. Extended unit tests.
dlenev
force-pushed
the
vector-mvp-11267
branch
from
August 24, 2026 19:22
28404bf to
6bec2b8
Compare
satya-bodapati
added a commit
to satya-bodapati/percona-server
that referenced
this pull request
Aug 26, 2026
…t 1]. https://perconadev.atlassian.net/browse/PS-11267 Added persistence and on-demand loading APIs to HNSW implementation. Extended unit tests. (cherry picked from commit 6bec2b8, dlenev/vector-mvp-11267, PR percona#6131) Carried here because the aux-storage write path has nothing to hook into without it: the class on our base takes a single template parameter and its insert() has no context argument. This commit adds the second parameter and the four callbacks the design targets. It applies cleanly because it sits directly on 65261fc, the same phase-1 commit our base builds on. To be dropped from this branch once PS-11267 lands in vector-mvp. gunit hnsw-t 34/34, percona vector 17/17.
satya-bodapati
added a commit
to satya-bodapati/percona-server
that referenced
this pull request
Aug 27, 2026
…t 1]. https://perconadev.atlassian.net/browse/PS-11267 Added persistence and on-demand loading APIs to HNSW implementation. Extended unit tests. (cherry picked from commit 6bec2b8, dlenev/vector-mvp-11267, PR percona#6131) Carried here because the aux-storage write path has nothing to hook into without it: the class on our base takes a single template parameter and its insert() has no context argument. This commit adds the second parameter and the four callbacks the design targets. It applies cleanly because it sits directly on 65261fc, the same phase-1 commit our base builds on. To be dropped from this branch once PS-11267 lands in vector-mvp. gunit hnsw-t 34/34, percona vector 17/17.
satya-bodapati
added a commit
to satya-bodapati/percona-server
that referenced
this pull request
Aug 28, 2026
…t 1]. https://perconadev.atlassian.net/browse/PS-11267 Added persistence and on-demand loading APIs to HNSW implementation. Extended unit tests. (cherry picked from commit 6bec2b8, dlenev/vector-mvp-11267, PR percona#6131) Carried here because the aux-storage write path has nothing to hook into without it: the class on our base takes a single template parameter and its insert() has no context argument. This commit adds the second parameter and the four callbacks the design targets. It applies cleanly because it sits directly on 65261fc, the same phase-1 commit our base builds on. To be dropped from this branch once PS-11267 lands in vector-mvp. gunit hnsw-t 34/34, percona vector 17/17.
satya-bodapati
added a commit
to satya-bodapati/percona-server
that referenced
this pull request
Aug 28, 2026
Cherry-picked from Dmitry's vector-mvp-11267 (percona#6131) so that the persistence work could be built before it landed. It sits on the same phase-1 commit this branch is based on and applies cleanly. DROP THIS COMMIT once PS-11267 is merged into vector-mvp, or the branch will carry a duplicate of it. Original message follows. --- Basic HNSW class (phase 2 - support for persistence and loading) [part 1].
satya-bodapati
added a commit
to satya-bodapati/percona-server
that referenced
this pull request
Aug 31, 2026
Cherry-picked from Dmitry's vector-mvp-11267 (percona#6131) so that the persistence work could be built before it landed. It sits on the same phase-1 commit this branch is based on and applies cleanly. DROP THIS COMMIT once PS-11267 is merged into vector-mvp, or the branch will carry a duplicate of it. Original message follows. --- Basic HNSW class (phase 2 - support for persistence and loading) [part 1].
…t 2]. https://perconadev.atlassian.net/browse/PS-11267 Made our HNSW implementation thread-safe. Extended unit tests.
…rations support) [follow-up]. https://perconadev.atlassian.net/browse/PS-11266 Extended k_nn_search() and streamed search API to return graph node ids for entries found (in addition to primary keys of corresponding base table rows).
…t 2, post-push fix]. https://perconadev.atlassian.net/browse/PS-11267 Address scenario when select_neighbors() returns less than Mmax neighbors due to lost nodes. Added unit test triggering this scenario.
catalinbp
requested changes
Sep 7, 2026
| assert(inserted.second); | ||
|
|
||
| new_node->set_layer(target_layer); | ||
| new_node->alloc_neighbors(m_allocator, *this); |
Contributor
There was a problem hiding this comment.
m_nodes.emplace only after alloc_neighbors as you may end up with new node with uninitialized neighbors
| const size_t selected = | ||
| select_neighbors(neighbor->vec(), candidate_neighbors, Mmax, | ||
| scratch_buffer.data()); | ||
| assert(selected <= Mmax); |
Contributor
There was a problem hiding this comment.
select_neighbors() guarantees this, if I understand correclty, a useful assert would be selected == min(candidate_neighbors.size(), Mmax).
|
|
||
| m_visited = std::unordered_set<Node *>(); | ||
| m_discarded = {}; | ||
| m_scratch_buffer.clear(); |
Contributor
There was a problem hiding this comment.
also set m_persistor_ctx to nullptr;
| } | ||
| max_layer = std::max(max_layer, node->m_layer); | ||
| if (node == m_entry_point) { | ||
| max_layer = std::max(max_layer, node->layer()); |
Contributor
There was a problem hiding this comment.
Lazy loading leaves NODE_DUMMY/NODE_LOST in m_nodes, but layer() rejects those states.
Skip non-COMPLETE nodes when computing max_layer
| for (const auto &kv : m_nodes) { | ||
| const Node *node = kv.second; | ||
| for (uint8_t lc = 0; lc <= node->m_layer; ++lc) { | ||
| for (uint8_t lc = 0; lc <= node->layer(); ++lc) { |
Contributor
There was a problem hiding this comment.
// This loop also calls layer()/neighbors_begin() on every map
// entry. A lazy-load stub has neither a valid layer nor neighbor storage,
// so non-COMPLETE nodes must be skipped before this point.
| return false; | ||
| } | ||
| // Neighbor must exist on this layer (its top layer >= lc). | ||
| if (nb->m_layer < lc) { |
Contributor
There was a problem hiding this comment.
shouldn't you only apply this check to NODE_COMPLETE neighbors?
| if (nb_state == NODE_LOST) continue; | ||
|
|
||
| // Load the neighbor if it is not loaded yet, skip if it is lost. | ||
| if (nb_state == NODE_DUMMY && !load_node(persistor_ctx, nb)) |
Contributor
There was a problem hiding this comment.
this is combining a genuinely missing row with a transiet load failure. Combining these two, shortens the list but should this be the case for both situations? ..
| std::scoped_lock lock(m_entry_point_lock); | ||
| entry_point = m_entry_point.load(); | ||
| if (entry_point == nullptr) { | ||
| new_node->set_complete(); |
Contributor
There was a problem hiding this comment.
in the other place(non-entry-point path) you first insert and then set it to complete.
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.
…t 1].
https://perconadev.atlassian.net/browse/PS-11267
Added persistence and on-demand loading APIs to HNSW implementation.
Extended unit tests.