Repository navigation
Save concurrent dynamic index without consolidate/compact; add svs_index_load_stream_dynamic_ex - #470
Draft
yuejiaointel wants to merge 5 commits into
Draft
yuejiaointel wants to merge 5 commits into
yuejiaointel wants to merge 5 commits into
Conversation
This branch has not been deployed
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
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.
Two changes for the concurrent (FINE_GRAIN) dynamic index, both on the save/load path:
save()no longer runsconsolidate()+compact(). It writes the slots as they are (holes and soft-deleted IDs included) in a new format version0.0.1.Why: compact moves every vector. When a caller saves from a forked child (e.g. a background snapshot), every page is then copied-on-write, so peak memory roughly doubles while saving.
svs_index_load_stream_dynamic_ex. The C API could already save the concurrent index to a stream, and load it back as concurrent from a directory (svs_index_load_dynamic_ex), but loading from a stream always returned the non-concurrent index. The new function takes the samesvs_dynamic_index_params_tas the directory version, sosync_kind = FINE_GRAINrestores the concurrent index.Changes
include/svs/concurrent/dynamic_index.h): version0.0.1; no consolidate/compact; writes one status byte per slot (Empty/Valid/Deleted) and the ID translator as is, so soft-deleted IDs stay mapped to their Deleted slots (the live index keeps them until consolidate). Throws on a Pending slot.compact_gatekeeper_so save isn't starved).add_points,delete_entries,consolidate,compactandreplace_external_idtake it shared;savetakes it exclusive. Searches never take it.0.0.0(all Valid, as before) and0.0.1; stream loaders now check the version and reject unknown ones.delete_entries, so e.g.consolidate(ids)works after reload).0.0.0). Doing the same for it is a follow-up.svs_index_load_stream_dynamic_ex;IndexBuilder::load_stream_dynamicpicks the concurrent or plain dispatcher by sync kind (likeload_dynamic), with a stream source added to the concurrent dispatcher.svs_index_load_stream_dynamicis unchanged (NONE). The FINE_GRAIN wrapper's save takes the shared lock, so searches keep running.include/svs/concurrent/README.md(locks),bindings/c/docs/C_API_Design.md,svs_c.hcomments,data/serialization.toml(v0.0.1).Compatibility
0.0.0files: yes, in all loaders.0.0.1files: no — it fails with an error (no silent misread in the cases tested). Only matters for a rollback after saving, or mixed versions.consolidate()/compact().Note for callers that save from a forked child
save()takes the writers lock exclusive. In a forked child, a writer that was mid-call atfork()still holds that lock (its thread doesn't exist in the child), so the child'ssave()waits forever. Stop writers before forking. This is the same on main, where save'scompact()waits on the storage lock.Relation to
update_entries(#467)update_entriesis a writer and must also take the writers lock (one line:auto writers_lock = lock_writers_shared();beforelock_storage_shared()). It isn't on main yet, so whichever of #467/#470 merges second adds that line. Tested locally with #468/#467 merged in: after save +svs_index_load_stream_dynamic_ex(FINE_GRAIN),svs_index_dynamic_update_pointson a deleted ID reuses the same slot (no new hole, no growth), and 10 saves during a loop ofupdate_entriesall returned and reloaded.Testing
202 test cases | 201 passed | 1 failed. The one failure,tests/svs/core/io/binary.cpp:138("Directory not empty"), fails the same way on main; an intermittent SIGSEGV attests/svs/index/vamana/index.cpp:95also occurs on main and passes on rerun.100% tests passed, 0 tests failed out of 26. pre-commit clean.0.0.0files still load; unknown version and corrupt status byte rejected; save while adding/searching;svs_index_load_stream_dynamic_exwith FINE_GRAIN / NULL / old function → FINE_GRAIN / NONE / NONE.0.0.1files fail with an error in main's loaders; non-concurrent saves still load in main.Not in this PR
concurrent/multi.hhas its own save that still consolidates/compacts.add_pointsthat throws mid-way (existing behavior) makes later saves throw.🤖 Generated with Claude Code