Fix a few index swap bugs.

1. Details of the indexSwap task
2. Query tasks with type=indexUid
3. Synchronous error message for multiple index not found
This commit is contained in:
Loïc Lecrenier 2022-10-26 12:57:29 +02:00 committed by Clément Renault
parent a16604af80
commit 1f75caae88
No known key found for this signature in database
GPG Key ID: 92ADA4E935E71FA4
13 changed files with 75 additions and 35 deletions

View File

@ -5,7 +5,7 @@ use meilisearch_types::error::ResponseError;
use meilisearch_types::keys::Key; use meilisearch_types::keys::Key;
use meilisearch_types::milli::update::IndexDocumentsMethod; use meilisearch_types::milli::update::IndexDocumentsMethod;
use meilisearch_types::settings::Unchecked; use meilisearch_types::settings::Unchecked;
use meilisearch_types::tasks::{Details, KindWithContent, Status, Task, TaskId}; use meilisearch_types::tasks::{Details, IndexSwap, KindWithContent, Status, Task, TaskId};
use meilisearch_types::InstanceUid; use meilisearch_types::InstanceUid;
use roaring::RoaringBitmap; use roaring::RoaringBitmap;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@ -113,9 +113,7 @@ pub enum KindDump {
IndexUpdate { IndexUpdate {
primary_key: Option<String>, primary_key: Option<String>,
}, },
IndexSwap { IndexSwap { swaps: Vec<IndexSwap> },
swaps: Vec<(String, String)>,
},
TaskCancelation { TaskCancelation {
query: String, query: String,
tasks: RoaringBitmap, tasks: RoaringBitmap,

View File

@ -433,6 +433,7 @@ pub fn autobatch(
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use meilisearch_types::tasks::IndexSwap;
use uuid::Uuid; use uuid::Uuid;
use super::*; use super::*;
@ -492,7 +493,9 @@ mod tests {
} }
fn idx_swap() -> KindWithContent { fn idx_swap() -> KindWithContent {
KindWithContent::IndexSwap { swaps: vec![(String::from("doggo"), String::from("catto"))] } KindWithContent::IndexSwap {
swaps: vec![IndexSwap { indexes: (String::from("doggo"), String::from("catto")) }],
}
} }
#[test] #[test]

View File

@ -836,8 +836,8 @@ impl IndexScheduler {
} else { } else {
unreachable!() unreachable!()
}; };
for (lhs, rhs) in swaps { for swap in swaps {
self.apply_index_swap(&mut wtxn, task.uid, lhs, rhs)?; self.apply_index_swap(&mut wtxn, task.uid, &swap.indexes.0, &swap.indexes.1)?;
} }
wtxn.commit()?; wtxn.commit()?;
task.status = Status::Succeeded; task.status = Status::Succeeded;

View File

@ -194,7 +194,7 @@ fn snapshot_details(d: &Details) -> String {
format!("{{ dump_uid: {dump_uid:?} }}") format!("{{ dump_uid: {dump_uid:?} }}")
}, },
Details::IndexSwap { swaps } => { Details::IndexSwap { swaps } => {
format!("{{ indexes: {swaps:?} }}") format!("{{ swaps: {swaps:?} }}")
} }
} }
} }

View File

@ -971,6 +971,7 @@ mod tests {
use meilisearch_types::milli::update::IndexDocumentsMethod::{ use meilisearch_types::milli::update::IndexDocumentsMethod::{
ReplaceDocuments, UpdateDocuments, ReplaceDocuments, UpdateDocuments,
}; };
use meilisearch_types::tasks::IndexSwap;
use meilisearch_types::VERSION_FILE_NAME; use meilisearch_types::VERSION_FILE_NAME;
use tempfile::TempDir; use tempfile::TempDir;
use time::Duration; use time::Duration;
@ -1543,7 +1544,10 @@ mod tests {
index_scheduler index_scheduler
.register(KindWithContent::IndexSwap { .register(KindWithContent::IndexSwap {
swaps: vec![("a".to_owned(), "b".to_owned()), ("c".to_owned(), "d".to_owned())], swaps: vec![
IndexSwap { indexes: ("a".to_owned(), "b".to_owned()) },
IndexSwap { indexes: ("c".to_owned(), "d".to_owned()) },
],
}) })
.unwrap(); .unwrap();
index_scheduler.assert_internally_consistent(); index_scheduler.assert_internally_consistent();
@ -1553,7 +1557,9 @@ mod tests {
snapshot!(snapshot_index_scheduler(&index_scheduler), name: "first_swap_processed"); snapshot!(snapshot_index_scheduler(&index_scheduler), name: "first_swap_processed");
index_scheduler index_scheduler
.register(KindWithContent::IndexSwap { swaps: vec![("a".to_owned(), "c".to_owned())] }) .register(KindWithContent::IndexSwap {
swaps: vec![IndexSwap { indexes: ("a".to_owned(), "c".to_owned()) }],
})
.unwrap(); .unwrap();
index_scheduler.assert_internally_consistent(); index_scheduler.assert_internally_consistent();

View File

@ -10,7 +10,7 @@ source: index-scheduler/src/lib.rs
1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }} 1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }}
2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }} 2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }}
3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }} 3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }}
4 {uid: 4, status: succeeded, details: { indexes: [("a", "b"), ("c", "d")] }, kind: IndexSwap { swaps: [("a", "b"), ("c", "d")] }} 4 {uid: 4, status: succeeded, details: { swaps: [IndexSwap { indexes: ("a", "b") }, IndexSwap { indexes: ("c", "d") }] }, kind: IndexSwap { swaps: [IndexSwap { indexes: ("a", "b") }, IndexSwap { indexes: ("c", "d") }] }}
---------------------------------------------------------------------- ----------------------------------------------------------------------
### Status: ### Status:
enqueued [] enqueued []

View File

@ -10,8 +10,8 @@ source: index-scheduler/src/lib.rs
1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }} 1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }}
2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }} 2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }}
3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }} 3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }}
4 {uid: 4, status: succeeded, details: { indexes: [("c", "b"), ("a", "d")] }, kind: IndexSwap { swaps: [("c", "b"), ("a", "d")] }} 4 {uid: 4, status: succeeded, details: { swaps: [IndexSwap { indexes: ("c", "b") }, IndexSwap { indexes: ("a", "d") }] }, kind: IndexSwap { swaps: [IndexSwap { indexes: ("c", "b") }, IndexSwap { indexes: ("a", "d") }] }}
5 {uid: 5, status: succeeded, details: { indexes: [("a", "c")] }, kind: IndexSwap { swaps: [("a", "c")] }} 5 {uid: 5, status: succeeded, details: { swaps: [IndexSwap { indexes: ("a", "c") }] }, kind: IndexSwap { swaps: [IndexSwap { indexes: ("a", "c") }] }}
---------------------------------------------------------------------- ----------------------------------------------------------------------
### Status: ### Status:
enqueued [] enqueued []

View File

@ -10,9 +10,9 @@ source: index-scheduler/src/lib.rs
1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }} 1 {uid: 1, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "c", primary_key: Some("id") }}
2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }} 2 {uid: 2, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "d", primary_key: Some("id") }}
3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }} 3 {uid: 3, status: succeeded, details: { primary_key: Some("id") }, kind: IndexCreation { index_uid: "a", primary_key: Some("id") }}
4 {uid: 4, status: succeeded, details: { indexes: [("c", "b"), ("a", "d")] }, kind: IndexSwap { swaps: [("c", "b"), ("a", "d")] }} 4 {uid: 4, status: succeeded, details: { swaps: [IndexSwap { indexes: ("c", "b") }, IndexSwap { indexes: ("a", "d") }] }, kind: IndexSwap { swaps: [IndexSwap { indexes: ("c", "b") }, IndexSwap { indexes: ("a", "d") }] }}
5 {uid: 5, status: succeeded, details: { indexes: [("a", "c")] }, kind: IndexSwap { swaps: [("a", "c")] }} 5 {uid: 5, status: succeeded, details: { swaps: [IndexSwap { indexes: ("a", "c") }] }, kind: IndexSwap { swaps: [IndexSwap { indexes: ("a", "c") }] }}
6 {uid: 6, status: succeeded, details: { indexes: [] }, kind: IndexSwap { swaps: [] }} 6 {uid: 6, status: succeeded, details: { swaps: [] }, kind: IndexSwap { swaps: [] }}
---------------------------------------------------------------------- ----------------------------------------------------------------------
### Status: ### Status:
enqueued [] enqueued []

View File

@ -5,7 +5,7 @@ use std::ops::Bound;
use meilisearch_types::heed::types::{DecodeIgnore, OwnedType}; use meilisearch_types::heed::types::{DecodeIgnore, OwnedType};
use meilisearch_types::heed::{Database, RoTxn, RwTxn}; use meilisearch_types::heed::{Database, RoTxn, RwTxn};
use meilisearch_types::milli::{CboRoaringBitmapCodec, BEU32}; use meilisearch_types::milli::{CboRoaringBitmapCodec, BEU32};
use meilisearch_types::tasks::{Details, Kind, KindWithContent, Status}; use meilisearch_types::tasks::{Details, IndexSwap, Kind, KindWithContent, Status};
use roaring::{MultiOps, RoaringBitmap}; use roaring::{MultiOps, RoaringBitmap};
use time::OffsetDateTime; use time::OffsetDateTime;
@ -244,7 +244,7 @@ pub fn swap_index_uid_in_task(task: &mut Task, swap: (&str, &str)) {
K::IndexCreation { index_uid, .. } => index_uids.push(index_uid), K::IndexCreation { index_uid, .. } => index_uids.push(index_uid),
K::IndexUpdate { index_uid, .. } => index_uids.push(index_uid), K::IndexUpdate { index_uid, .. } => index_uids.push(index_uid),
K::IndexSwap { swaps } => { K::IndexSwap { swaps } => {
for (lhs, rhs) in swaps.iter_mut() { for IndexSwap { indexes: (lhs, rhs) } in swaps.iter_mut() {
if lhs == swap.0 || lhs == swap.1 { if lhs == swap.0 || lhs == swap.1 {
index_uids.push(lhs); index_uids.push(lhs);
} }
@ -259,7 +259,7 @@ pub fn swap_index_uid_in_task(task: &mut Task, swap: (&str, &str)) {
| K::SnapshotCreation => (), | K::SnapshotCreation => (),
}; };
if let Some(Details::IndexSwap { swaps }) = &mut task.details { if let Some(Details::IndexSwap { swaps }) = &mut task.details {
for (lhs, rhs) in swaps.iter_mut() { for IndexSwap { indexes: (lhs, rhs) } in swaps.iter_mut() {
if lhs == swap.0 || lhs == swap.1 { if lhs == swap.0 || lhs == swap.1 {
index_uids.push(lhs); index_uids.push(lhs);
} }

View File

@ -38,6 +38,10 @@ pub enum MeilisearchHttpError {
.0.iter().map(|s| format!("`{}`", s)).collect::<Vec<_>>().join(", ") .0.iter().map(|s| format!("`{}`", s)).collect::<Vec<_>>().join(", ")
)] )]
SwapDuplicateIndexesFound(Vec<String>), SwapDuplicateIndexesFound(Vec<String>),
#[error("Two indexes must be given for each swap. The list `{:?}` contains {} indexes.",
.0, .0.len()
)]
SwapIndexPayloadWrongLength(Vec<String>),
#[error(transparent)] #[error(transparent)]
IndexUid(#[from] IndexUidFormatError), IndexUid(#[from] IndexUidFormatError),
#[error(transparent)] #[error(transparent)]
@ -70,6 +74,7 @@ impl ErrorCode for MeilisearchHttpError {
MeilisearchHttpError::IndexesNotFound(_) => Code::IndexNotFound, MeilisearchHttpError::IndexesNotFound(_) => Code::IndexNotFound,
MeilisearchHttpError::SwapDuplicateIndexFound(_) => Code::DuplicateIndexFound, MeilisearchHttpError::SwapDuplicateIndexFound(_) => Code::DuplicateIndexFound,
MeilisearchHttpError::SwapDuplicateIndexesFound(_) => Code::DuplicateIndexFound, MeilisearchHttpError::SwapDuplicateIndexesFound(_) => Code::DuplicateIndexFound,
MeilisearchHttpError::SwapIndexPayloadWrongLength(_) => Code::BadRequest,
MeilisearchHttpError::IndexUid(e) => e.error_code(), MeilisearchHttpError::IndexUid(e) => e.error_code(),
MeilisearchHttpError::SerdeJson(_) => Code::Internal, MeilisearchHttpError::SerdeJson(_) => Code::Internal,
MeilisearchHttpError::HeedError(_) => Code::Internal, MeilisearchHttpError::HeedError(_) => Code::Internal,

View File

@ -4,7 +4,7 @@ use actix_web::web::Data;
use actix_web::{web, HttpResponse}; use actix_web::{web, HttpResponse};
use index_scheduler::IndexScheduler; use index_scheduler::IndexScheduler;
use meilisearch_types::error::ResponseError; use meilisearch_types::error::ResponseError;
use meilisearch_types::tasks::KindWithContent; use meilisearch_types::tasks::{IndexSwap, KindWithContent};
use serde::Deserialize; use serde::Deserialize;
use crate::error::MeilisearchHttpError; use crate::error::MeilisearchHttpError;
@ -16,11 +16,10 @@ use crate::routes::tasks::TaskView;
pub fn configure(cfg: &mut web::ServiceConfig) { pub fn configure(cfg: &mut web::ServiceConfig) {
cfg.service(web::resource("").route(web::post().to(SeqHandler(swap_indexes)))); cfg.service(web::resource("").route(web::post().to(SeqHandler(swap_indexes))));
} }
#[derive(Deserialize, Debug, Clone, PartialEq, Eq)] #[derive(Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "camelCase", deny_unknown_fields)] #[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct SwapIndexesPayload { pub struct SwapIndexesPayload {
indexes: (String, String), indexes: Vec<String>,
} }
pub async fn swap_indexes( pub async fn swap_indexes(
@ -33,23 +32,43 @@ pub async fn swap_indexes(
let mut indexes_set = HashSet::<String>::default(); let mut indexes_set = HashSet::<String>::default();
let mut unknown_indexes = HashSet::new(); let mut unknown_indexes = HashSet::new();
let mut duplicate_indexes = HashSet::new(); let mut duplicate_indexes = HashSet::new();
for SwapIndexesPayload { indexes: (lhs, rhs) } in params.into_inner().into_iter() { for SwapIndexesPayload { indexes } in params.into_inner().into_iter() {
let (lhs, rhs) = match indexes.as_slice() {
[lhs, rhs] => (lhs, rhs),
_ => {
return Err(MeilisearchHttpError::SwapIndexPayloadWrongLength(indexes).into());
}
};
if !search_rules.is_index_authorized(&lhs) { if !search_rules.is_index_authorized(&lhs) {
unknown_indexes.insert(lhs.clone()); unknown_indexes.insert(lhs.clone());
} }
if !search_rules.is_index_authorized(&rhs) { if !search_rules.is_index_authorized(&rhs) {
unknown_indexes.insert(rhs.clone()); unknown_indexes.insert(rhs.clone());
} }
match index_scheduler.index(&lhs) {
Ok(_) => (),
Err(index_scheduler::Error::IndexNotFound(_)) => {
unknown_indexes.insert(lhs.clone());
}
Err(e) => return Err(e.into()),
}
match index_scheduler.index(&rhs) {
Ok(_) => (),
Err(index_scheduler::Error::IndexNotFound(_)) => {
unknown_indexes.insert(rhs.clone());
}
Err(e) => return Err(e.into()),
}
swaps.push((lhs.clone(), rhs.clone())); swaps.push(IndexSwap { indexes: (lhs.clone(), rhs.clone()) });
let is_unique_index_lhs = indexes_set.insert(lhs.clone()); let is_unique_index_lhs = indexes_set.insert(lhs.clone());
if !is_unique_index_lhs { if !is_unique_index_lhs {
duplicate_indexes.insert(lhs); duplicate_indexes.insert(lhs.clone());
} }
let is_unique_index_rhs = indexes_set.insert(rhs.clone()); let is_unique_index_rhs = indexes_set.insert(rhs.clone());
if !is_unique_index_rhs { if !is_unique_index_rhs {
duplicate_indexes.insert(rhs); duplicate_indexes.insert(rhs.clone());
} }
} }
if !duplicate_indexes.is_empty() { if !duplicate_indexes.is_empty() {

View File

@ -5,7 +5,9 @@ use meilisearch_types::error::ResponseError;
use meilisearch_types::index_uid::IndexUid; use meilisearch_types::index_uid::IndexUid;
use meilisearch_types::settings::{Settings, Unchecked}; use meilisearch_types::settings::{Settings, Unchecked};
use meilisearch_types::star_or::StarOr; use meilisearch_types::star_or::StarOr;
use meilisearch_types::tasks::{serialize_duration, Details, Kind, KindWithContent, Status, Task}; use meilisearch_types::tasks::{
serialize_duration, Details, IndexSwap, Kind, KindWithContent, Status, Task,
};
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use serde_cs::vec::CS; use serde_cs::vec::CS;
use serde_json::json; use serde_json::json;
@ -103,7 +105,7 @@ pub struct DetailsView {
#[serde(flatten)] #[serde(flatten)]
pub settings: Option<Box<Settings<Unchecked>>>, pub settings: Option<Box<Settings<Unchecked>>>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
pub indexes: Option<Vec<(String, String)>>, pub swaps: Option<Vec<IndexSwap>>,
} }
impl From<Details> for DetailsView { impl From<Details> for DetailsView {
@ -151,7 +153,7 @@ impl From<Details> for DetailsView {
DetailsView { dump_uid: Some(dump_uid), ..DetailsView::default() } DetailsView { dump_uid: Some(dump_uid), ..DetailsView::default() }
} }
Details::IndexSwap { swaps } => { Details::IndexSwap { swaps } => {
DetailsView { indexes: Some(swaps), ..Default::default() } DetailsView { swaps: Some(swaps), ..Default::default() }
} }
} }
} }

View File

@ -116,7 +116,7 @@ pub enum KindWithContent {
primary_key: Option<String>, primary_key: Option<String>,
}, },
IndexSwap { IndexSwap {
swaps: Vec<(String, String)>, swaps: Vec<IndexSwap>,
}, },
TaskCancelation { TaskCancelation {
query: String, query: String,
@ -134,6 +134,12 @@ pub enum KindWithContent {
SnapshotCreation, SnapshotCreation,
} }
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct IndexSwap {
pub indexes: (String, String),
}
impl KindWithContent { impl KindWithContent {
pub fn as_kind(&self) -> Kind { pub fn as_kind(&self) -> Kind {
match self { match self {
@ -169,9 +175,9 @@ impl KindWithContent {
| IndexDeletion { index_uid } => vec![index_uid], | IndexDeletion { index_uid } => vec![index_uid],
IndexSwap { swaps } => { IndexSwap { swaps } => {
let mut indexes = HashSet::<&str>::default(); let mut indexes = HashSet::<&str>::default();
for (lhs, rhs) in swaps { for swap in swaps {
indexes.insert(lhs.as_str()); indexes.insert(swap.indexes.0.as_str());
indexes.insert(rhs.as_str()); indexes.insert(swap.indexes.1.as_str());
} }
indexes.into_iter().collect() indexes.into_iter().collect()
} }
@ -383,6 +389,8 @@ impl FromStr for Kind {
Ok(Kind::IndexCreation) Ok(Kind::IndexCreation)
} else if kind.eq_ignore_ascii_case("indexUpdate") { } else if kind.eq_ignore_ascii_case("indexUpdate") {
Ok(Kind::IndexUpdate) Ok(Kind::IndexUpdate)
} else if kind.eq_ignore_ascii_case("indexSwap") {
Ok(Kind::IndexSwap)
} else if kind.eq_ignore_ascii_case("indexDeletion") { } else if kind.eq_ignore_ascii_case("indexDeletion") {
Ok(Kind::IndexDeletion) Ok(Kind::IndexDeletion)
} else if kind.eq_ignore_ascii_case("documentAdditionOrUpdate") { } else if kind.eq_ignore_ascii_case("documentAdditionOrUpdate") {
@ -429,8 +437,7 @@ pub enum Details {
TaskCancelation { matched_tasks: u64, canceled_tasks: Option<u64>, original_query: String }, TaskCancelation { matched_tasks: u64, canceled_tasks: Option<u64>, original_query: String },
TaskDeletion { matched_tasks: u64, deleted_tasks: Option<u64>, original_query: String }, TaskDeletion { matched_tasks: u64, deleted_tasks: Option<u64>, original_query: String },
Dump { dump_uid: String }, Dump { dump_uid: String },
// TODO: Lo: Revisit this variant once we have decided on what the POST payload of swapping indexes should be IndexSwap { swaps: Vec<IndexSwap> },
IndexSwap { swaps: Vec<(String, String)> },
} }
/// Serialize a `time::Duration` as a best effort ISO 8601 while waiting for /// Serialize a `time::Duration` as a best effort ISO 8601 while waiting for