apache / kvrocks

Apache Kvrocks is a distributed key value NoSQL database that uses RocksDB as storage engine and is compatible with Redis protocol.
https://kvrocks.apache.org/
Apache License 2.0
3.47k stars 452 forks source link

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

Open Beihao-Zhou opened 1 month ago

Beihao-Zhou commented 1 month ago

Search before asking

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 https://github.com/apache/kvrocks/blob/3408318934cb68c0c1f2ffa937f19f02346d0672/src/server/redis_connection.cc#L522-L528

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

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?

PragmaTwice commented 1 month ago

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.