-
Notifications
You must be signed in to change notification settings - Fork 210
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
fix: avoid multiple threads loading same index partition #2559
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2559 +/- ##
==========================================
- Coverage 80.00% 79.99% -0.02%
==========================================
Files 209 209
Lines 59847 59864 +17
Branches 59847 59864 +17
==========================================
+ Hits 47880 47887 +7
- Misses 9143 9148 +5
- Partials 2824 2829 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -65,3 +70,53 @@ pub async fn maybe_sample_training_data( | |||
})?; | |||
Ok(array.as_fixed_size_list().clone()) | |||
} | |||
|
|||
#[derive(Debug)] | |||
pub struct PartitionLoadLock { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this pub(crate)
. Just wanted to make sure this is not public API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is in a private module, so it won't be part of public API
rust/lance/src/index/vector/utils.rs
Outdated
|
||
#[derive(Debug)] | ||
pub struct PartitionLoadLock { | ||
partition_locks: RwLock<HashMap<String, Arc<Mutex<()>>>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we make this vec![Arc<Mutex()]
with pre-allocated mutex, we dont need RWlock
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
13de1b2
to
1b74d8c
Compare
Adds locks for loading of index partitions. This voids multiple threads querying concurrently from both allocating memory to load the partition, which can cause memory issues during application startup.