Skip to content

Conversation

@mkaruza
Copy link
Contributor

@mkaruza mkaruza commented Dec 3, 2025

Implementation of multi-reader multi-writer mutex that supports concurrent writes or reads but not a mix of reads and writes in same time.

Unit test for testing MRMWMutex class.

@mkaruza mkaruza requested a review from romange December 3, 2025 14:28
@mkaruza mkaruza force-pushed the mkaruza/simple-hnsw-locking branch from 54f8826 to 3ba0698 Compare December 3, 2025 14:30
Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. 1 suggestion posted.

Comment augment review to trigger a new review at any time.

}

vector<pair<float, GlobalDocId>> Knn(float* target, size_t k, std::optional<size_t> ef) {
world_.setEf(ef.value_or(kDefaultEfRuntime));
Copy link

Choose a reason for hiding this comment

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

Consider moving the world_.setEf(...) call inside the MRMWMutexLock scope; setEf mutates shared state and, as written, can race with concurrent reads/writes. (Also applies to the filtered Knn overload below.)

🤖 Was this useful? React with 👍 or 👎

return mode == LockMode::kReadLock ? LockMode::kWriteLock : LockMode::kReadLock;
}

util::fb2::Mutex mutex_;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: you also contend on mutex_, though here maybe absl::SpinLock would fit better as you take it for a small period of time. you could use CondVarAny to be able to use locks with a different type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

not the most important thing though.

romange
romange previously approved these changes Dec 3, 2025
Implementation of multi-reader multi-writer mutex that supports
concurrent writes or reads but not a mix of reads and writes in same
time.

Unit test for testing MRMWMutex class.

Signed-off-by: mkaruza <mario@dragonflydb.io>
@mkaruza mkaruza force-pushed the mkaruza/simple-hnsw-locking branch from 45a7599 to 11e61f5 Compare December 5, 2025 12:22
@mkaruza mkaruza requested review from kostasrim and romange December 5, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants