Dynamic Vamana `save()` always consolidates and compacts: full rewrite on every save, and failure when no entries are live

Aberta
  • Algo errado?
#421 2 comentários 0 reações 0 responsáveis Ver no GitHub

Mantenedores costumam responder em até 5 dias

Ninguém assumiu esta issue ainda.

Avaliação

Dificuldade
4/5
Tempo estimado
3-5 dias
Facilidade para iniciantes
24/100
Tipo de issue
Bug
Clareza
Razoavelmente clara
Status de atividade
Ativa
Stack de tecnologia
cpp
Domínio
search

Direção de pesquisa

Start in include/svs/index/vamana/dynamic_index.h: save() calls consolidate() and compact() at about lines 1128 and 1161, and compact() at about line 904 does old_to_new_id_map.at(ep) at about line 1012, which throws when no points are live. Reproduce with the 10-point, delete-all, svs_index_save_stream sequence from the issue. Done means a save with zero live entries either succeeds and loads as an empty index, or fails with a clear error before any state changes. Coordinate the approach with maintainers first, since the issue also proposes a larger redesign of save.

Escrita pelo modelo de indexação a partir do texto da issue.

Descrição

Problem

MutableVamanaIndex::save begins with consolidate(); compact(); in both overloads: the directory one (include/svs/index/vamana/dynamic_index.h:1128) and the stream one (:1161). Both svs_index_save and svs_index_save_stream reach it, and since #406 the C API takes the index's write lock around the stream save (bindings/c/src/index_vamana.hpp:97-99). So a save is an exclusive mutation of the live index:

  • compact() (:904) rewrites every graph node (replace_node, :969, :981), compacts the data, resizes graph, data and status (:990), and remaps the translator (:1005). It does this even when no slot is deleted.
  • Searches on the index wait for the whole save. valkey-search reports about 1.2 s per 100k vectors for a foreground SAVE / DEBUG RELOAD (valkey-search fork PR #19).
  • Embedders that snapshot through fork() (e.g. valkey-search, where BGSAVE writes the RDB from a forked child) pay a copy-on-write fault on essentially every page of the index on every save. Peak memory during a save approaches twice the index size, and that peak arrives just as the process forks.
  • The cleanup also happens only in the child's copy. The parent keeps every deleted slot until it consolidates on its own.

Request

A save that doesn't change the index. Two options:

  1. Preferred: serialize only live entries through a temporary old-to-new id map, leaving graph, data and translator untouched. Callers could then save under shared/read access, and fork-based snapshots would only read memory.
  2. Minimal: in save(), skip consolidate() when is_consolidated() (:893) is true, and skip compact() when ids are already dense. DynamicIndexVamana::compact already uses is_consolidated() this way. Callers could then compact on their own schedule (svs_index_dynamic_consolidate / svs_index_dynamic_compact), and saves of a clean index wouldn't write to it.

Related bug: save fails when every entry is deleted

If every point has been deleted (with no consolidation in between), save() throws, and the C API returns Couldn't find key.:

  • consolidate() (:1074) finds the entry point deleted and calls compute_entry_point(data_, threadpool_, valid) (:1087). With no valid points this falls back to a deleted slot; assert(!is_deleted(new_entry_point)) would abort in a debug build.
  • compact() then builds an empty old_to_new_id_map, and ep = old_to_new_id_map.at(ep) (:1012) throws std::out_of_range.

Repro, as reproduced from valkey-search's unit tests at 90c2aa0, which make exactly these C API calls: build a dynamic index with 10 points, svs_index_dynamic_delete_points all 10, then svs_index_save_stream. The call returns false with "Couldn't find key." The code path is unchanged at 8285340. valkey-search now works around this by not calling SVS save for an index with no live entries.

The expected behaviour is that saving an index with zero live entries either succeeds (producing an empty index on load) or fails with a clear error before any state is modified.

Linguagem predominante
C++
Estrelas
241
Forks
48
Merge médio
4d 21h
PRs com merge (30d)
11

Preparar o ambiente

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Mais de intel/ScalableVectorSearch

Todas as issues de intel/ScalableVectorSearch

Issues semelhantes

Mais issues de C++

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.