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>
This commit is contained in:
bors[bot] 2022-12-01 13:58:02 +00:00 committed by GitHub
commit d3731dda48
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 34 additions and 17 deletions

View File

@ -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.
@ -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>> {

View File

@ -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)?;
@ -373,14 +388,12 @@ 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)?;
}
Setting::Reset => {
self.index.delete_all_searchable_fields(self.wtxn)?;
}
Setting::NotSet => return Ok(false),
}
Ok(true)
}
Setting::Reset => Ok(self.index.delete_all_searchable_fields(self.wtxn)?),
Setting::NotSet => Ok(false),
}
}
fn update_stop_words(&mut self) -> Result<bool> {
match self.stop_words {
@ -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 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 => {
self.index.delete_exact_attributes(self.wtxn)?;
Ok(true)
}
Setting::Reset => Ok(self.index.delete_exact_attributes(self.wtxn)?),
Setting::NotSet => Ok(false),
}
}