Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Commit

Permalink
Merge #706
Browse files Browse the repository at this point in the history
706: Limit the reindexing caused by updating settings when not needed r=curquiza a=GregoryConrad

## What does this PR do?
When updating index settings using `update::Settings`, sometimes a `reindex` of `update::Settings` is triggered when it doesn't need to be. This PR aims to prevent those unnecessary `reindex` calls.

For reference, here is a snippet from the current `execute` method in `update::Settings`:
```rust
// ...
if stop_words_updated
    || faceted_updated
    || synonyms_updated
    || searchable_updated
    || exact_attributes_updated
{
    self.reindex(&progress_callback, &should_abort, old_fields_ids_map)?;
}
```

- [x] `faceted_updated` - looks good as-is ✅
- [x] `stop_words_updated` - looks good as-is ✅
- [x] `synonyms_updated` - looks good as-is ✅
- [x] `searchable_updated` - fixed in this PR
- [x] `exact_attributes_updated` - fixed in this PR

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: Gregory Conrad <gregorysconrad@gmail.com>
  • Loading branch information
bors[bot] and GregoryConrad committed Dec 1, 2022
2 parents 51a2613 + 87e2bc3 commit d3731dd
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 17 deletions.
10 changes: 5 additions & 5 deletions milli/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,8 +560,9 @@ impl Index {
}

pub(crate) fn delete_all_searchable_fields(&self, wtxn: &mut RwTxn) -> heed::Result<bool> {
self.delete_searchable_fields(wtxn)?;
self.delete_user_defined_searchable_fields(wtxn)
let did_delete_searchable = self.delete_searchable_fields(wtxn)?;
let did_delete_user_defined = self.delete_user_defined_searchable_fields(wtxn)?;
Ok(did_delete_searchable || did_delete_user_defined)
}

/// Writes the searchable fields, when this list is specified, only these are indexed.
Expand Down Expand Up @@ -1145,9 +1146,8 @@ impl Index {
}

/// Clears the exact attributes from the store.
pub(crate) fn delete_exact_attributes(&self, txn: &mut RwTxn) -> Result<()> {
self.main.delete::<_, Str>(txn, main_key::EXACT_ATTRIBUTES)?;
Ok(())
pub(crate) fn delete_exact_attributes(&self, txn: &mut RwTxn) -> heed::Result<bool> {
self.main.delete::<_, Str>(txn, main_key::EXACT_ATTRIBUTES)
}

pub fn max_values_per_facet(&self, txn: &RoTxn) -> heed::Result<Option<usize>> {
Expand Down
41 changes: 29 additions & 12 deletions milli/src/update/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,21 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_searchable(&mut self) -> Result<bool> {
match self.searchable_fields {
Setting::Set(ref fields) => {
// Check to see if the searchable fields changed before doing anything else
let old_fields = self.index.searchable_fields(self.wtxn)?;
let did_change = match old_fields {
// If old_fields is Some, let's check to see if the fields actually changed
Some(old_fields) => {
let new_fields = fields.iter().map(String::as_str).collect::<Vec<_>>();
new_fields != old_fields
}
// If old_fields is None, the fields have changed (because they are being set)
None => true,
};
if !did_change {
return Ok(false);
}

// every time the searchable attributes are updated, we need to update the
// ids for any settings that uses the facets. (distinct_fields, filterable_fields).
let old_fields_ids_map = self.index.fields_ids_map(self.wtxn)?;
Expand All @@ -373,13 +388,11 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
&new_fields_ids_map,
)?;
self.index.put_fields_ids_map(self.wtxn, &new_fields_ids_map)?;
Ok(true)
}
Setting::Reset => {
self.index.delete_all_searchable_fields(self.wtxn)?;
}
Setting::NotSet => return Ok(false),
Setting::Reset => Ok(self.index.delete_all_searchable_fields(self.wtxn)?),
Setting::NotSet => Ok(false),
}
Ok(true)
}

fn update_stop_words(&mut self) -> Result<bool> {
Expand Down Expand Up @@ -465,14 +478,18 @@ impl<'a, 't, 'u, 'i> Settings<'a, 't, 'u, 'i> {
fn update_exact_attributes(&mut self) -> Result<bool> {
match self.exact_attributes {
Setting::Set(ref attrs) => {
let attrs = attrs.iter().map(String::as_str).collect::<Vec<_>>();
self.index.put_exact_attributes(self.wtxn, &attrs)?;
Ok(true)
}
Setting::Reset => {
self.index.delete_exact_attributes(self.wtxn)?;
Ok(true)
let old_attrs = self.index.exact_attributes(self.wtxn)?;
let old_attrs = old_attrs.into_iter().map(String::from).collect::<HashSet<_>>();

if attrs != &old_attrs {
let attrs = attrs.iter().map(String::as_str).collect::<Vec<_>>();
self.index.put_exact_attributes(self.wtxn, &attrs)?;
Ok(true)
} else {
Ok(false)
}
}
Setting::Reset => Ok(self.index.delete_exact_attributes(self.wtxn)?),
Setting::NotSet => Ok(false),
}
}
Expand Down

0 comments on commit d3731dd

Please sign in to comment.