Skip to content

feat: allow selecting inner index label remap map (cherry-pick #1981 to 0.18)#2029

Merged
wxyucs merged 1 commit into
antgroup:0.18from
LHT129:cp-1981-to-0.18
May 14, 2026
Merged

feat: allow selecting inner index label remap map (cherry-pick #1981 to 0.18)#2029
wxyucs merged 1 commit into
antgroup:0.18from
LHT129:cp-1981-to-0.18

Conversation

@LHT129
Copy link
Copy Markdown
Collaborator

@LHT129 LHT129 commented May 11, 2026

Summary

This is a cherry-pick of PR #1981 to branch 0.18.

Original PR: #1981

Changes from original PR:

  • Add an inner-index label_remap_type parameter and expose it on the HGraph build parameter surface
  • Encapsulate label remap storage inside LabelTable so inner indexes can switch between robin_map and robin_pg_map
  • Add unit coverage for parameter parsing, HGraph mapping, and configurable remap behavior

Cherry-pick adaptations:

  • Resolved conflicts in multiple files to adapt to 0.18 branch context
  • Preserved all functionality from original PR
  • Added necessary conflict resolution adjustments

Closes #1986 (original issue)

@LHT129 LHT129 added kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) version/0.18 needs-cherry-pick-release-0.18 labels May 11, 2026
@mergify mergify Bot added the module/api label May 11, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a configurable label remapping mechanism within the LabelTable, allowing for different map implementations like PG and ROBIN. It refactors LabelTable to support hole management, adds a RemoveListFilter for tracking deletions, and updates HGraph and InnerIndex parameters accordingly. The review identifies several critical issues: the logic for RecoverRemove and IsTombstoneLabel is broken because MarkRemove no longer updates the remap table with tombstones; RemoveListFilter may lead to dangling references if it outlives the LabelTable; and MergeOther fails to merge deleted ID information. Additionally, the reviewer noted inefficient locking within a loop and several indentation violations of the style guide.

Comment thread src/impl/label_table.h Outdated
Comment thread src/impl/label_table.h Outdated
Comment thread src/impl/label_table.cpp Outdated
Comment thread src/impl/label_table.cpp
Comment thread src/impl/label_table.cpp Outdated
Comment thread src/algorithm/hnswlib/algorithm_interface.h Outdated
Comment thread src/algorithm/hnswlib/hnswalg.h Outdated
@LHT129 LHT129 self-assigned this May 11, 2026
@LHT129 LHT129 force-pushed the cp-1981-to-0.18 branch 4 times, most recently from 6018f2e to 6f7bb35 Compare May 12, 2026 03:32
@mergify mergify Bot added the module/index label May 12, 2026
@LHT129 LHT129 force-pushed the cp-1981-to-0.18 branch from 6f7bb35 to c0fbdf3 Compare May 12, 2026 06:32
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 91.11111% with 12 lines in your changes missing coverage. Please review.

@@            Coverage Diff             @@
##             0.18    #2029      +/-   ##
==========================================
+ Coverage   90.82%   91.49%   +0.66%     
==========================================
  Files         330      330              
  Lines       19483    19574      +91     
==========================================
+ Hits        17696    17909     +213     
+ Misses       1787     1665     -122     
Flag Coverage Δ
cpp 91.49% <91.11%> (+0.66%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
common 85.94% <ø> (ø)
datacell 93.01% <ø> (+0.61%) ⬆️
index 91.69% <91.66%> (+0.71%) ⬆️
simd 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e7cc15...582a096. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@wxyucs wxyucs left a comment

Choose a reason for hiding this comment

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

lgtm

Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
@LHT129 LHT129 force-pushed the cp-1981-to-0.18 branch from ba306c2 to 582a096 Compare May 13, 2026 08:58
@wxyucs wxyucs merged commit df01d56 into antgroup:0.18 May 14, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/improvement Code improvements (variable/function renaming, refactoring, etc. ) module/api module/index size/XL version/0.18

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants