Skip to content
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.

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

Introduce Mutex Map in IndexInfo to Ensure Thread Safety During Index Construction #2489

Open
2 tasks done
Beihao-Zhou opened this issue Aug 12, 2024 · 1 comment · May be fixed by #2491
Open
2 tasks done

Introduce Mutex Map in IndexInfo to Ensure Thread Safety During Index Construction #2489

Beihao-Zhou opened this issue Aug 12, 2024 · 1 comment · May be fixed by #2491
Labels
enhancement type enhancement

Comments

@Beihao-Zhou
Copy link
Member

Search before asking

  • I had searched in the issues and found no similar issues.

Motivation

In the context of index, the absence of a locking mechanism could lead to issues when multiple threads attempt to modify shared resources simultaneously.

[1] Original issue raised from HNSW #2481
[2] Marked TODO

// TODO: transaction support for index updating
for (const auto &record : index_records) {
auto s = GlobalIndexer::Update(record);
if (!s.IsOK() && !s.Is<Status::TypeMismatched>()) {
LOG(WARNING) << "index updating failed for key: " << record.key;
}
}

We might want to introduce locks to solve the problem.

Solution

Solution 1: Mutex (Straight-forward)

We can introduce a mutex map within each IndexInfo structure. This mutex map would contain a mutex for each field that requires protection against concurrent access.

Con

  • Blocking if multiple connections update the same index

Solution 2: Queue

We can store the Update task for each IndexInfo field in a queue, and have a background thread to schedule tasks in order asynchronously. I'm not quite sure about how many number of threads would be appropriate, but can discuss further if everyone leans towards this solution.

cc @PragmaTwice

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@Beihao-Zhou Beihao-Zhou added the enhancement type enhancement label Aug 12, 2024
@PragmaTwice
Copy link
Member

We can first try to implement the first method because it is simpler and does ensure correctness of indexing.

We can then benchmark to determine its impact on performance and consider how to further optimize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement type enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants