Skip to content

fix(hgraph): fix concurrency issues during hgraph insert (#1431) #1496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
inabao wants to merge 5 commits into
base: 0.15
Choose a base branch
from
Open

fix(hgraph): fix concurrency issues during hgraph insert (#1431) #1496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9e7aae1
fix(hgraph): fix concurrency issues during hgraph insert (#1431)
inabao Dec 29, 2025
394599c
modify
inabao Jan 5, 2026
74e1623
fix(ci): decrease mem & disk cost in CI compilation (#1474)
wxyucs Dec 30, 2025
2bb1b25
modify
inabao Jan 6, 2026
43f8a7b
modify
inabao Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 38 additions & 8 deletions .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,35 +21,65 @@ workflows:

jobs:
pull-request-check:
docker:
- image: vsaglib/vsag:ci-x86
machine:
image: ubuntu-2204:2025.09.1
resource_class: large
steps:
- checkout
- restore_cache:
keys:
- fork-cache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
- run:
command: export CMAKE_GENERATOR="Ninja" && make test_parallel
name: prepare_env_and_create_swap_file
command: |
sudo apt update
sudo apt install -y gfortran python3-dev libomp-15-dev gcc make cmake g++ lcov libaio-dev libcurl4-openssl-dev ninja-build
sudo apt-get install -y libopenblas-dev
sudo dd if=/dev/zero of=.swapfile bs=2M count=2048
sudo chmod 600 .swapfile
sudo mkswap .swapfile
sudo swapon .swapfile
- run:
command: export CMAKE_GENERATOR="Ninja" && export VSAG_ENABLE_INTEL_MKL=OFF && export COMPILE_JOBS=4 && make test_parallel
no_output_timeout: 50m
- run:
name: remove_swap_file
command: |
sudo swapoff .swapfile
sudo rm .swapfile
- save_cache:
key: fork-cache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
paths:
- ./build

main-branch-check:
docker:
- image: vsaglib/vsag:ci-x86
machine:
image: ubuntu-2204:2025.09.1
resource_class: large
steps:
- checkout
- restore_cache:
keys:
- main-ccache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
- main-cache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
- run:
command: export CMAKE_GENERATOR="Ninja" && make test_parallel
name: prepare_env_and_create_swap_file
command: |
sudo apt update
sudo apt install -y gfortran python3-dev libomp-15-dev gcc make cmake g++ lcov libaio-dev libcurl4-openssl-dev ninja-build
sudo apt-get install -y libopenblas-dev
sudo dd if=/dev/zero of=.swapfile bs=2M count=2048
sudo chmod 600 .swapfile
sudo mkswap .swapfile
sudo swapon .swapfile
- run:
command: export CMAKE_GENERATOR="Ninja" && export VSAG_ENABLE_INTEL_MKL=OFF && export COMPILE_JOBS=4 && make test_parallel
no_output_timeout: 50m
- run:
name: remove_swap_file
command: |
sudo swapoff .swapfile
sudo rm .swapfile
- save_cache:
key: main-ccache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
key: main-cache-{{ checksum "CMakeLists.txt" }}-{{ checksum ".circleci/fresh_ci_cache.commit" }}
paths:
- ./build
15 changes: 12 additions & 3 deletions .github/workflows/asan_build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ jobs:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
steps:
- name: Free Disk Space
run: rm -rf /useless/hostedtoolcache
run: |
rm -rf /useless/*
rm -rf /opt/hostedtoolcache
rm -rf /opt/intel/oneapi/mkl
- uses: actions/checkout@v4
- name: Load Cache
uses: hendrikmuhs/[email protected]
Expand All @@ -28,9 +31,15 @@ jobs:
save: ${{ github.event_name != 'pull_request' }}
key: build-${{ hashFiles('./CMakeLists.txt') }}-${{ hashFiles('./.circleci/fresh_ci_cache.commit') }}
- name: Make Asan
run: export CMAKE_GENERATOR="Ninja"; make asan
run: export CMAKE_GENERATOR="Ninja"; make asan VSAG_ENABLE_INTEL_MKL=OFF
- name: Clean
run: find ./build -type f -name "*.o" -exec rm -f {} +
run: |
find ./build -type f -name "*.o" -exec rm -f {} +
find ./build -type f -name "*.cpp" -exec rm -f {} +
find ./build -type f -name "*.a" -exec rm -f {} +
rm -rf ./build/hdf5
rm -rf ./build/boost
rm -rf ./build/tools
- name: Save Test
uses: actions/upload-artifact@v4
with:
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/tsan_build_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,13 @@ jobs:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
container:
image: vsaglib/vsag:ci-x86
volumes:
- /opt:/useless
steps:
- name: Free Disk Space
run: |
rm -rf /useless/*
rm -rf /opt/hostedtoolcache
- uses: actions/checkout@v4
with:
fetch-depth: '0'
Expand Down
20 changes: 13 additions & 7 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,22 @@ CMAKE_INSTALL_PREFIX ?= "/usr/local/"
COMPILE_JOBS ?= 6
DEBUG_BUILD_DIR ?= "./build/"
RELEASE_BUILD_DIR ?= "./build-release/"
VSAG_ENABLE_TESTS ?= ON
VSAG_ENABLE_PYBINDS ?= ON
VSAG_ENABLE_TOOLS ?= ON
VSAG_ENABLE_EXAMPLES ?= ON
VSAG_ENABLE_INTEL_MKL ?= ON

VSAG_CMAKE_ARGS := -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DCMAKE_COLOR_DIAGNOSTICS=ON
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} -DNUM_BUILDING_JOBS=${COMPILE_JOBS}
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DENABLE_TESTS=${VSAG_ENABLE_TESTS} -DENABLE_PYBINDS=${VSAG_ENABLE_PYBINDS}
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DENABLE_TOOLS=${VSAG_ENABLE_TOOLS} -DENABLE_EXAMPLES=${VSAG_ENABLE_EXAMPLES}
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DENABLE_INTEL_MKL=${VSAG_ENABLE_INTEL_MKL}

OTHER_DEFINE=""
ifdef EXTRA_DEFINED
OTHER_DEFINE=${EXTRA_DEFINED}
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} ${EXTRA_DEFINED}
endif

VSAG_CMAKE_ARGS := -DCMAKE_EXPORT_COMPILE_COMMANDS=1
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX} -DNUM_BUILDING_JOBS=${COMPILE_JOBS}
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -DENABLE_TESTS=ON -DENABLE_PYBINDS=ON -DENABLE_TOOLS=ON
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} ${OTHER_DEFINE} -G ${CMAKE_GENERATOR} -S.
VSAG_CMAKE_ARGS := ${VSAG_CMAKE_ARGS} -G ${CMAKE_GENERATOR} -S.

UT_FILTER = ""
ifdef CASE
Expand Down
9 changes: 6 additions & 3 deletions src/algorithm/hgraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1003,9 +1003,12 @@ HGraph::GetExtraInfoByIds(const int64_t* ids, int64_t count, char* extra_infos)

void
HGraph::add_one_point(const void* data, int level, InnerIdType inner_id) {
this->basic_flatten_codes_->InsertVector(data, inner_id);
if (use_reorder_) {
this->high_precise_codes_->InsertVector(data, inner_id);
{
std::shared_lock add_lock(add_mutex_);
this->basic_flatten_codes_->InsertVector(data, inner_id);
if (use_reorder_) {
this->high_precise_codes_->InsertVector(data, inner_id);
}
}
Comment on lines +1006 to 1012
Copy link

@gemini-code-assist gemini-code-assist bot Jan 5, 2026

Choose a reason for hiding this comment

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

critical

While adding this shared_lock correctly fixes a race condition with resize, the overall locking strategy in add_one_point is unsafe and can lead to deadlocks.

Immediately after this block, a std::unique_lock is acquired on the same add_mutex_ (line 1013). This shared_lock -> unique_lock sequence on a non-recursive mutex is dangerous:

  1. Deadlock on re-entry: If a caller already holds a unique_lock on add_mutex_ (e.g., in HGraph::Merge) and calls add_one_point, it will deadlock when this shared_lock is acquired.
  2. Deadlock on indirect recursive locking: The unique_lock on line 1013 is held during calls to other functions like graph_add_one. If any function in that call path (e.g., resize) also tries to acquire a unique_lock on add_mutex_, it will deadlock.

This indicates a larger design issue with locking in this part of the code. The function add_one_point should likely not manage locks itself. Instead, it could be refactored into unlocked helper methods, with the public API methods (Add, Merge, etc.) being responsible for acquiring locks once at a higher level.

std::unique_lock add_lock(add_mutex_);
if (level >= static_cast<int>(this->route_graphs_.size()) || bottom_graph_->TotalCount() == 0) {
Expand Down
46 changes: 29 additions & 17 deletions src/data_cell/flatten_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,58 +23,70 @@
#include "sparse_vector_datacell.h"

namespace vsag {

template <typename QuantTemp, typename IOTemp>
static FlattenInterfacePtr
make_instance(const FlattenInterfaceParamPtr& param, const IndexCommonParam& common_param) {
make_instance_flatten(const FlattenInterfaceParamPtr& param, const IndexCommonParam& common_param) {
auto& io_param = param->io_parameter;
auto& quantizer_param = param->quantizer_parameter;
if (param->name == SPARSE_VECTOR_DATA_CELL) {
return std::make_shared<SparseVectorDataCell<QuantTemp, IOTemp>>(
quantizer_param, io_param, common_param);
}
if (param->name == FLATTEN_DATA_CELL) {
return std::make_shared<FlattenDataCell<QuantTemp, IOTemp>>(
quantizer_param, io_param, common_param);
}
return nullptr;
throw VsagException(ErrorType::INVALID_ARGUMENT,
fmt::format("Unknown flatten interface name: {}", param->name));
}

template <typename QuantTemp, typename IOTemp>
static FlattenInterfacePtr
make_instance_sparse(const FlattenInterfaceParamPtr& param, const IndexCommonParam& common_param) {
auto& io_param = param->io_parameter;
auto& quantizer_param = param->quantizer_parameter;

if (param->name == SPARSE_VECTOR_DATA_CELL) {
return std::make_shared<SparseVectorDataCell<QuantTemp, IOTemp>>(
quantizer_param, io_param, common_param);
}
throw VsagException(ErrorType::INVALID_ARGUMENT,
fmt::format("Unknown flatten interface name: {}", param->name));
}

template <MetricType metric, typename IOTemp>
static FlattenInterfacePtr
make_instance(const FlattenInterfaceParamPtr& param, const IndexCommonParam& common_param) {
std::string quantization_string = param->quantizer_parameter->GetTypeName();
if (quantization_string == QUANTIZATION_TYPE_VALUE_SQ8) {
return make_instance<SQ8Quantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<SQ8Quantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_FP32) {
return make_instance<FP32Quantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<FP32Quantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_SQ4) {
return make_instance<SQ4Quantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<SQ4Quantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_SQ4_UNIFORM) {
return make_instance<SQ4UniformQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<SQ4UniformQuantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_SQ8_UNIFORM) {
return make_instance<SQ8UniformQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<SQ8UniformQuantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_BF16) {
return make_instance<BF16Quantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<BF16Quantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_FP16) {
return make_instance<FP16Quantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<FP16Quantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_PQ) {
return make_instance<ProductQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<ProductQuantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_PQFS) {
return make_instance<PQFastScanQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<PQFastScanQuantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_RABITQ) {
return make_instance<RaBitQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_flatten<RaBitQuantizer<metric>, IOTemp>(param, common_param);
}
if (quantization_string == QUANTIZATION_TYPE_VALUE_SPARSE) {
return make_instance<SparseQuantizer<metric>, IOTemp>(param, common_param);
return make_instance_sparse<SparseQuantizer<metric>, IOTemp>(param, common_param);
}
return nullptr;
}
Expand Down
Loading