Skip to content

[MOD-19327] Remove id reuse from the tiered HNSW index - #1085

Merged
nonirosenfeldredis merged 12 commits into
mainfrom
reuse-from-cache
Oct 29, 2026
Merged

nonirosenfeldredis merged 12 commits into
mainfrom
reuse-from-cache

Conversation

@nonirosenfeldredis

@nonirosenfeldredis nonirosenfeldredis commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Describe the changes in the pull request
Remove id reuse from the tiered HNSW index.

Today an overwrite reserves the replaced id, and the insert that follows clears the slot under an exclusive main index guard. That acquisition waits out every graph scan, and while it is pending it blocks the shared acquisitions of the repair jobs and of deleteLabelFromHNSW, which serialises the workers under an update load. Measured locally (100k documents, 120,000 random-key updates, 16 connections): about 1.1k ops/s with the update-path reuse against about 4.4k with it off, and the main thread spent about 70% of its samples blocked on the flat or main index guard.

After this PR the tiered index never reuses a deleted element's id: an overwrite deletes the old element and the insert appends a fresh id, and the GC compacts the deleted elements as it did before id reuse existed.

  • Removed: reserved swap jobs, the deferred insert submission, the exclusive barrier in the insert job, the id reuse in addVector, updateVectors and write-in-place, and setReuseOnUpdate. updateVectors is a delete followed by adds.
  • Kept: the HNSW-level slot helpers (clearDeletedSlot, storeNewElementInClearedSlot), which the stacked follow-up uses.
  • Follow-up: the stacked PR adds reuse back on the insert path, from a cache of slots the GC clears, off by default.

Supersedes #1084.

Which issues this PR fixes

  1. MOD-19327

Main objects this PR modified

  1. TieredHNSWIndex: the update-path reuse is removed (reuseIdOnUpdate, reserved swap jobs, insertVectorToHNSWReusingId, updateVectorInPlace, updateMultiValueInPlace, updateMultiValueAsync).
  2. Tests: the reuse-on-update tests are removed.

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

🤖 Generated with Claude Code


Note

Medium Risk
Changes core tiered HNSW update/delete/insert job ordering and compaction behavior under async load; correctness risk is mitigated by large test deletions/additions but behavior no longer matches the previous reuse semantics.

Overview
Removes the tiered HNSW “reuse deleted id on overwrite/update” path so updates no longer reserve swap jobs, defer insert jobs until repairs finish, or fill cleared slots via insertVectorToHNSWReusingId (which took an exclusive main index guard and contended with repair/delete work).

Overwrites and updateVectors now follow delete + fresh append: deleteLabelFromHNSW no longer pairs deleted ids with pending insert jobs, swap jobs always become ready for GC when repairs complete, and insert jobs always call insertVectorToHNSW. Write-in-place addVectorInPlace deletes the label first (including when it exists in both flat and HNSW) then addVectors; multi-value updateVectors is always deleteVector plus per-vector addVector. setReuseOnUpdate / reuseIdOnUpdate and related helpers (updateVectorInPlace, updateMultiValueInPlace/Async, deferred reuse submission) are dropped; HNSW slot helpers stay for a planned follow-up.

Tests covering id reuse and reserved swap jobs are removed; addVectorInPlaceOverwritesLabelInBothTiers asserts overwrite return semantics when a label spans both tiers.

Reviewed by Cursor Bugbot for commit a19f216. Bugbot is set up for automated code reviews on this repo. Configure here.

nonirosenfeldredis and others added 5 commits October 27, 2026 20:08
A reuse insert held the flat index guard shared across the exclusive main
index acquisition that clears the reserved slot. That acquisition waits out
every concurrent graph scan, so every writer needing the flat guard
exclusively (`addVectorToFlat`, on the main thread) queued behind it.

Clear the slot first, with no flat guard held, then take the guard again,
re-check that the job is still valid, and store, releasing the guard right
after the store as a fresh insert does. If the label was overwritten or
deleted meanwhile, the reservation is released and the cleared slot, a valid
empty deleted element, is compacted like any other deleted id.

- clearReservedSlot: the exclusive step, split out of
  insertVectorToHNSWReusingId
- dropInvalidInsertJob: the shared handling of a job invalidated before it ran
- test hook afterReuseBarrier and a test that overwrites the label while the
  guard is free

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Each reuse insert takes the main index guard exclusively to clear its slot.
That waits out every concurrent graph scan and, while pending, blocks the
shared acquisitions of the repair jobs and of the main thread's
deleteLabelFromHNSW. Under a sustained single-value update load this
serialises the workers: the backlog grows to about ten thousand jobs, the
flat buffer fills and the main thread inserts directly. On 100k documents,
16 connections and random keys that is about 1.1k ops/s with reuse against
4.4k without.

Default `reuseIdOnUpdate` to false until the exclusive acquisition per
insert is addressed. `setReuseOnUpdate(true)` still enables it.

- the tests that cover reuse switch it on explicitly before they start
- a test pins the new default

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ped after clearing its slot

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Remove the id reuse from the update path (async single and multi-value
overwrites, write-in-place updates) and make it a flag on the insert path
instead. With the flag on, the GC clears the slot of every ready swap job
under the exclusive main index guard and queues it in a FIFO cache, then
compacts the oldest cached slots beyond a limit. An insert takes the oldest
cached id instead of appending a fresh one, with the main index guard held
shared only.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…yond a limit

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nonirosenfeldredis
nonirosenfeldredis marked this pull request as ready for review October 27, 2026 20:36

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.37%. Comparing base (12e3c03) to head (a19f216).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1085      +/-   ##
==========================================
- Coverage   97.55%   97.37%   -0.18%     
==========================================
  Files         143      143              
  Lines        9715     9549     -166     
==========================================
- Hits         9477     9298     -179     
- Misses        238      251      +13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

… info

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nonirosenfeldredis

Copy link
Copy Markdown
Collaborator Author

Local measurements of this experiment (VecSim 281a86cb, RediSearch with OPTIMIZE_PARTIAL_UPDATE on, 100k documents, one run each, Mac M4 Max). Reuse on is the build with the reuseIdOnInsert default flipped to true.

Vector updates only (120,000 updates to random keys, 16 connections)

Reuse off Reuse on
ops/s 5,448 5,391
CPU (of one core) 981% 996%
Inserts that took a cleared slot 0 59,997
Defrag runs / time 118 / 352 ms 118 / 269 ms
Backend size at the end (100,000 labels) 100,096 100,515
After a forced GC: backend size / marked deleted 100,000 / 0 100,258 / 258

Mixed: KNN queries plus rate-limited vector updates (memtier with the arguments of search-vector-field-update-knn-mixed, 180 s)

Reuse off Reuse on
KNN ops/s (avg latency) 2,263 (28.3 ms) 2,314 (27.6 ms)
Background updates 299/s 300/s
Inserts that took a cleared slot 0 49,664
Defrag runs / time 97 / 272 ms 97 / 200 ms
After a forced GC: backend size / marked deleted 100,000 / 0 100,274 / 274

Reading:

  • Throughput and CPU match reuse off. For comparison, the update-path reuse on main gave about 1.1k ops/s against about 4.4k without reuse in the same updates-only setup ([MOD-19327] Clear the reused slot without the flat guard, and disable id reuse by default #1084).
  • About half of the inserts take a cleared slot, as expected from compacting half of the cache each GC.
  • Defrag time is about 25% lower with reuse on in both runs.
  • The cache keeps cleared slots: a forced GC leaves about 260 of them, because each GC compacts half. An idle index never shrinks to zero by itself.

Single local runs, so differences of a few percent are noise. Not run on CI or with large documents yet.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
An empty vector allocates nothing, unlike a libstdc++ deque, so the size of a
fresh index matches its estimate on every standard library.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nonirosenfeldredis nonirosenfeldredis changed the title [MOD-19327] Experiment: reuse ids on insert from a cache filled by the GC [MOD-19327] Reuse ids on insert from a cache filled by the GC Oct 28, 2026
@nonirosenfeldredis
nonirosenfeldredis changed the base branch from sharon-MOD-19327-reuse-insert-flat-guard to main October 28, 2026 15:41
Drop the cache of cleared slots and the insert-time reuse built on it, so the
tiered index never reuses a deleted element's id. Deleted elements are compacted
by the GC as before the id reuse work. The HNSW-level slot helpers
(`clearDeletedSlot`, `storeNewElementInClearedSlot`) are left in place.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nonirosenfeldredis nonirosenfeldredis changed the title [MOD-19327] Reuse ids on insert from a cache filled by the GC [MOD-19327] Remove id reuse from the tiered HNSW index Oct 29, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 070fb42. Configure here.

@dor-forer dor-forer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good

…ite count

With reuse gone the swap job lookup is only written by the main thread under the
shared main index guard, and by compaction under the exclusive one, so the extra
lock is unnecessary. An in-place overwrite of a label present in both tiers
reported -1 instead of 0.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nonirosenfeldredis
nonirosenfeldredis added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 1db74be Oct 29, 2026
15 checks passed
@nonirosenfeldredis
nonirosenfeldredis deleted the reuse-from-cache branch October 29, 2026 13:59
nonirosenfeldredis added a commit that referenced this pull request Oct 29, 2026
[MOD-19327] Remove id reuse from the tiered HNSW index (#1085)

* [MOD-19327] Clear the reused slot without the flat index guard

A reuse insert held the flat index guard shared across the exclusive main
index acquisition that clears the reserved slot. That acquisition waits out
every concurrent graph scan, so every writer needing the flat guard
exclusively (`addVectorToFlat`, on the main thread) queued behind it.

Clear the slot first, with no flat guard held, then take the guard again,
re-check that the job is still valid, and store, releasing the guard right
after the store as a fresh insert does. If the label was overwritten or
deleted meanwhile, the reservation is released and the cleared slot, a valid
empty deleted element, is compacted like any other deleted id.

- clearReservedSlot: the exclusive step, split out of
  insertVectorToHNSWReusingId
- dropInvalidInsertJob: the shared handling of a job invalidated before it ran
- test hook afterReuseBarrier and a test that overwrites the label while the
  guard is free

* [MOD-19327] Disable id reuse on update by default

Each reuse insert takes the main index guard exclusively to clear its slot.
That waits out every concurrent graph scan and, while pending, blocks the
shared acquisitions of the repair jobs and of the main thread's
deleteLabelFromHNSW. Under a sustained single-value update load this
serialises the workers: the backlog grows to about ten thousand jobs, the
flat buffer fills and the main thread inserts directly. On 100k documents,
16 connections and random keys that is about 1.1k ops/s with reuse against
4.4k without.

Default `reuseIdOnUpdate` to false until the exclusive acquisition per
insert is addressed. `setReuseOnUpdate(true)` still enables it.

- the tests that cover reuse switch it on explicitly before they start
- a test pins the new default

* [MOD-19327] Submit deferred reuse inserts when a reuse insert is dropped after clearing its slot

* Experiment: reuse ids on insert from a cache filled by the GC

Remove the id reuse from the update path (async single and multi-value
overwrites, write-in-place updates) and make it a flag on the insert path
instead. With the flag on, the GC clears the slot of every ready swap job
under the exclusive main index guard and queues it in a FIFO cache, then
compacts the oldest cached slots beyond a limit. An insert takes the oldest
cached id instead of appending a fresh one, with the main index guard held
shared only.

* Compact a fixed number of cleared slots per GC instead of the ones beyond a limit

* Cap the slots a GC clears and compact half of the cache

* Count the inserts that take a cleared slot and report it in the debug info

* Extract the insert into a cleared slot from insertVectorToHNSW

* Allocate the reusable slots cache with the index allocator

* Hold the reusable slots in a vector read from a head index

An empty vector allocates nothing, unlike a libstdc++ deque, so the size of a
fresh index matches its estimate on every standard library.

* [MOD-19327] Remove id reuse from the tiered HNSW index

Drop the cache of cleared slots and the insert-time reuse built on it, so the
tiered index never reuses a deleted element's id. Deleted elements are compacted
by the GC as before the id reuse work. The HNSW-level slot helpers
(`clearDeletedSlot`, `storeNewElementInClearedSlot`) are left in place.

* [MOD-19327] Drop the swap job lookup lock and fix the in-place overwrite count

With reuse gone the swap job lookup is only written by the main thread under the
shared main index guard, and by compaction under the exclusive one, so the extra
lock is unnecessary. An in-place overwrite of a label present in both tiers
reported -1 instead of 0.

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
aradalvand pushed a commit to aradalvand/RediSearch that referenced this pull request Nov 2, 2026
…RediSearch#12074)

* [MOD-19327] Bump VecSim to remove id reuse from the tiered HNSW index

Advance deps/VectorSimilarity to the commit that removes id reuse from the
tiered HNSW index (RedisAI/VectorSimilarity#1085).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

* [MOD-19327] Update the vector-update tests for the removed id reuse

Without id reuse an overwrite's insert is queued right away, so the job
accounting has no held inserts, and the replaced vector's slot stays marked
deleted until the GC compacts it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants