feat(hgraph): support UpdateVector with deduplicate_storage by jac0626 · Pull Request #2588 · antgroup/vsag

jac0626 · 2026-07-30T14:07:27Z

Change Type

Linked Issue

Closes: [feat](hgraph): support UpdateVector with deduplicate_storage #2587
Related: [feat](hgraph): support core operations with deduplicate_storage #2575
Related: [bug](hgraph): UpdateVector() does not update graph topology or duplicate-group membership #2322

What Changed

Support HGraph::UpdateVector when a logical ID shares storage under deduplicate_storage.
Detect sharing from current CodeSlotMap bindings rather than stale duplicate-group size.
Use copy-on-write for a shared slot: grow physical storage, initialize base, precise-reorder, and separate raw-vector codes, then atomically rebind only the updated logical ID.
Update an already private slot in place so repeated updates do not keep allocating storage.
Add CodeSlotMap::RebindSlot with expected-slot validation and release/acquire publication.
Preserve the existing HGraph behavior for graph topology and duplicate-group metadata; the broader consistency work remains tracked by [bug](hgraph): UpdateVector() does not update graph topology or duplicate-group membership #2322.
Keep the non-deduplicated UpdateVector path unchanged.
Update the English and Chinese HGraph documentation.

Test Evidence

Test details:

Debug library and unit-test builds with 48 compile jobs: passed
HGraph duplicate tests: 27 cases / 334 assertions passed
Deduplicated UpdateVector tests: 3 cases / 72 assertions passed
CodeSlotMap tests: 7 cases / 196 assertions passed
Existing HGraph Build functional test: 1 case / 29,506 assertions passed

clang-format-15 --dry-run --Werror on changed C++ files: passed
clang-tidy-15 on changed production C++ files: passed
git diff --check: passed

Compatibility Impact

API/ABI compatibility: no public API or ABI changes; CodeSlotMap::RebindSlot is internal.
Behavior changes: HGraph with deduplicate_storage=true now supports updating IDs that still share a physical code slot. Only the target ID is redirected to private storage.
Existing graph-rebuild and duplicate-group update semantics are unchanged and remain tracked by [bug](hgraph): UpdateVector() does not update graph topology or duplicate-group membership #2322.

Performance and Concurrency Impact

Performance impact: limited to UpdateVector. The first update of a shared ID allocates one private physical slot; later updates of that ID are in place. Search, Build, Add, and the non-deduplicated update path are unchanged.
Concurrency/thread-safety impact: deduplicated updates acquire the global and persistent-code locks together with deadlock avoidance. A new mapping is published only after every physical code layer is initialized, and concurrent updates of the same ID perform at most one copy-on-write allocation.

Documentation Impact

Risk and Rollback

Risk level: medium
Main risk: keeping physical code layers and logical-to-physical bindings consistent while detaching one member of a shared group.
Mitigation: initialize every code layer before atomic publication and cover representative, non-representative, repeated, concurrent, precise/raw, serialization, and last-shared-member cases.
Rollback plan: revert this change to restore explicit rejection of updates on shared deduplicated storage.

Checklist

I have linked the relevant issue (required for kind/bug and kind/feature; see "Linked Issue" above)
I have added/updated tests for new behavior or bug fixes
I have considered API compatibility impact
I have updated docs if behavior/workflow changed
My commit messages follow project conventions (Conventional Commits, optional [skip ci] prefix)

vsag-bot · 2026-07-30T14:07:34Z

/label status/waiting-for-review
/waiting-on reviewer
/request-review @jiaweizone
/request-review @wxyucs
/request-review @inabao
/request-review @Copilot

vsag-bot · 2026-07-30T14:07:41Z

Automated pull request review completed.

Review effort: medium (379 changed lines across 8 files).

No actionable inline findings were found.
Review: #2588 (review)

mergify · 2026-07-30T14:08:12Z

Merge Protections

🟢 All 3 merge protections satisfied — ready to merge.

Show 3 satisfied protections

🟢 Require kind label

label~=^kind/

🟢 Require version label

label~=^version/

🟢 Require linked issue for feature/bug PRs

body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

vsag-bot

Automated inline review completed.

Review effort: medium (379 changed lines across 8 files).
Submitted 1 inline comment.

Reviewed commit 097f34f.

vsag-bot · 2026-07-30T14:12:53Z

+    const auto next_slot = this->code_slot_map_->PhysicalCount();
+    this->ensure_physical_code_capacity_unlocked(next_slot + 1);
+    const auto new_slot = this->code_slot_map_->AllocateSlot();
+    this->insert_persistent_codes_to_slot(new_base_vec, new_slot);


[P2] Make copy-on-write slot creation transactional

If any InsertVectorToCodeSlot operation throws (for example from allocation, encoding, or an IO write), AllocateSlot() has already permanently incremented physical_count_, while only some physical flatten layers may have advanced their own counts. The logical binding remains unchanged, but a retry consumes another slot; with precise/raw layers this can immediately make their serialized physical counts disagree, and even a base-only index can reach physical_count > serialized logical slots, causing deserialization validation to reject the index. Preserve the pre-update state on insertion failure or otherwise roll back/resynchronize the allocated slot and every code layer before returning the error.

LHT129 · 2026-07-30T14:14:37Z

+            update_status =
+                update_status && high_precise_codes_->UpdateVector(new_base_vec, inner_id);
+        }
+        // A member previously split from a shared slot must keep its private raw code in sync on


[note] The comment says "Preserve the existing unique-ID behavior outside duplicate groups", but the enclosing condition not duplicate_ids.empty() means this branch is entered when the ID is part of a duplicate group (just not sharing a physical slot). The earlier comment on the same block already explains the intent well. Consider rewording to avoid confusion:

// A member previously split from a shared slot must keep its private raw // code in sync on later updates, matching the per-layer update done for // unique IDs in the non-dedup path.

LHT129 · 2026-07-30T14:14:48Z

+    const auto current_slot = this->code_slot_map_->Resolve(inner_id);
+    const auto duplicate_ids = duplicate_tracker->GetDuplicateIds(inner_id);
+    bool slot_is_shared = false;
+    for (const auto duplicate_id : duplicate_ids) {


[suggestion] The slot_is_shared loop calls Resolve for every duplicate_id, including inner_id itself (if GetDuplicateIds returns the full group). When duplicate_id == inner_id, Resolve always returns current_slot, making the comparison trivially true. Consider skipping self to avoid the redundant atomic load:

for (const auto duplicate_id : duplicate_ids) { if (duplicate_id == inner_id) { continue; } if (this->code_slot_map_->Resolve(duplicate_id) == current_slot) { slot_is_shared = true; break; } }

This is a minor efficiency concern since duplicate groups are typically small, but it makes the intent clearer.

LHT129 · 2026-07-30T14:14:57Z

@@ -69,6 +69,35 @@ CodeSlotMap::PublishSlot(InnerIdType inner_id, CodeSlotIdType code_slot_id) {
    published_logical_count_.fetch_add(1, std::memory_order_acq_rel);


[note] RebindSlot correctly does not increment published_logical_count_ — it replaces an existing binding rather than publishing a new logical ID. This is consistent with PublishSlot which does increment the counter. Worth keeping in mind for future serialization: the serialized format stores per-inner_id slot bindings, so rebinding is naturally captured without any format change.

LHT129

Overall this is a well-designed change. The copy-on-write approach for detaching a shared duplicate-group member is sound, and the concurrency handling with std::lock + std::defer_lock correctly avoids deadlock between the global and codes mutexes.

Summary of what was reviewed:

hgraph.cpp UpdateVector: The three-way split (non-dedup early-return / dedup-not-shared in-place / dedup-shared copy-on-write) is clean and each path is correct.
code_slot_map.cpp RebindSlot: The CAS-based rebind with expected-slot validation provides strong atomicity guarantees. Correctly does not touch published_logical_count_.
code_slot_map.h: Interface changes are minimal and well-documented.
Tests: Comprehensive coverage including shared-group update, precise-reorder codes, separate raw vectors, concurrent updates, serialization round-trip, and error-path validation in CodeSlotMap tests.

Inline comments posted:

[note] on hgraph.cpp:804 — comment wording clarification
[suggestion] on hgraph.cpp:791 — minor loop optimization (skip self in slot_is_shared check)
[note] on code_slot_map.cpp:69 — published_logical_count_ consistency note

No blocking issues found. The implementation correctly handles the key invariant: every code layer is initialized before the new slot binding is published, so concurrent readers observe either the complete old vector or the complete new vector.

Signed-off-by: jc543239 <jc543239@antgroup.com> Assisted-by: Codex:gpt-5

vsag-bot

Automated inline review completed.

Review effort: medium (379 changed lines across 8 files).
No actionable inline findings were found.

Reviewed commit 9e03ed1.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 30, 2026 14:07

pull-request-size Bot added the size/L label Jul 30, 2026

vsag-bot assigned LHT129 Jul 30, 2026

jac0626 self-assigned this Jul 30, 2026

jac0626 added kind/feature Brand-new functionality or capabilities 引入全新的功能、新特性或新能力 version/1.1 labels Jul 30, 2026

vsag-bot self-requested a review July 30, 2026 14:07

Copilot started reviewing on behalf of jac0626 July 30, 2026 14:07 View session

mergify Bot added the module/docs label Jul 30, 2026

Copilot AI reviewed Jul 30, 2026

vsag-bot reviewed Jul 30, 2026

View reviewed changes

LHT129 reviewed Jul 30, 2026

View reviewed changes

feat(hgraph): support UpdateVector with deduplicate_storage

9e03ed1

Signed-off-by: jc543239 <jc543239@antgroup.com> Assisted-by: Codex:gpt-5

jac0626 force-pushed the codex/hgraph-dedup-update-vector branch from 097f34f to 9e03ed1 Compare July 31, 2026 03:54

Copilot AI review requested due to automatic review settings July 31, 2026 03:54

vsag-bot self-requested a review July 31, 2026 03:54

jac0626 marked this pull request as ready for review July 31, 2026 03:57

jac0626 requested review from inabao, jiaweizone and wxyucs as code owners July 31, 2026 03:57

vsag-bot reviewed Jul 31, 2026

View reviewed changes

Copilot AI reviewed Jul 31, 2026

Copilot started reviewing on behalf of jac0626 July 31, 2026 04:16 View session

		@@ -69,6 +69,35 @@ CodeSlotMap::PublishSlot(InnerIdType inner_id, CodeSlotIdType code_slot_id) {
		published_logical_count_.fetch_add(1, std::memory_order_acq_rel);

Uh oh!

Conversation

jac0626 commented Jul 30, 2026

Change Type

Linked Issue

What Changed

Test Evidence

Compatibility Impact

Performance and Concurrency Impact

Documentation Impact

Risk and Rollback

Checklist

Uh oh!

vsag-bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsag-bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify Bot commented Jul 30, 2026

Merge Protections

🟢 Require kind label

🟢 Require version label

🟢 Require linked issue for feature/bug PRs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

vsag-bot left a comment

Choose a reason for hiding this comment

Uh oh!

vsag-bot Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

LHT129 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

LHT129 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

LHT129 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

vsag-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vsag-bot commented Jul 30, 2026 •

edited

Loading

vsag-bot commented Jul 30, 2026 •

edited

Loading