Merge both DocumentAddition/Update into one DocumentImport variant

This commit is contained in:
Kerollmops 2022-09-29 15:49:54 +02:00 committed by Clément Renault
parent 5174c78f87
commit f68906f5dc
No known key found for this signature in database
GPG Key ID: 92ADA4E935E71FA4
4 changed files with 167 additions and 297 deletions

View File

@ -1,3 +1,4 @@
use milli::update::IndexDocumentsMethod::{self, ReplaceDocuments, UpdateDocuments};
use std::ops::ControlFlow; use std::ops::ControlFlow;
use crate::{task::Kind, TaskId}; use crate::{task::Kind, TaskId};
@ -7,11 +8,9 @@ pub enum BatchKind {
DocumentClear { DocumentClear {
ids: Vec<TaskId>, ids: Vec<TaskId>,
}, },
DocumentAddition { DocumentImport {
addition_ids: Vec<TaskId>, method: IndexDocumentsMethod,
}, import_ids: Vec<TaskId>,
DocumentUpdate {
update_ids: Vec<TaskId>,
}, },
DocumentDeletion { DocumentDeletion {
deletion_ids: Vec<TaskId>, deletion_ids: Vec<TaskId>,
@ -20,13 +19,10 @@ pub enum BatchKind {
other: Vec<TaskId>, other: Vec<TaskId>,
settings_ids: Vec<TaskId>, settings_ids: Vec<TaskId>,
}, },
SettingsAndDocumentAddition { SettingsAndDocumentImport {
settings_ids: Vec<TaskId>, settings_ids: Vec<TaskId>,
addition_ids: Vec<TaskId>, method: IndexDocumentsMethod,
}, import_ids: Vec<TaskId>,
SettingsAndDocumentUpdate {
settings_ids: Vec<TaskId>,
update_ids: Vec<TaskId>,
}, },
Settings { Settings {
settings_ids: Vec<TaskId>, settings_ids: Vec<TaskId>,
@ -59,14 +55,16 @@ impl BatchKind {
Kind::IndexSwap => (BatchKind::IndexSwap { id: task_id }, true), Kind::IndexSwap => (BatchKind::IndexSwap { id: task_id }, true),
Kind::DocumentClear => (BatchKind::DocumentClear { ids: vec![task_id] }, false), Kind::DocumentClear => (BatchKind::DocumentClear { ids: vec![task_id] }, false),
Kind::DocumentAddition => ( Kind::DocumentAddition => (
BatchKind::DocumentAddition { BatchKind::DocumentImport {
addition_ids: vec![task_id], method: ReplaceDocuments,
import_ids: vec![task_id],
}, },
false, false,
), ),
Kind::DocumentUpdate => ( Kind::DocumentUpdate => (
BatchKind::DocumentUpdate { BatchKind::DocumentImport {
update_ids: vec![task_id], method: UpdateDocuments,
import_ids: vec![task_id],
}, },
false, false,
), ),
@ -98,11 +96,9 @@ impl BatchKind {
// The index deletion can batch with everything but must stop after // The index deletion can batch with everything but must stop after
( (
BatchKind::DocumentClear { mut ids } BatchKind::DocumentClear { mut ids }
| BatchKind::DocumentAddition { | BatchKind::DocumentImport {
addition_ids: mut ids, method: _,
} import_ids: mut ids,
| BatchKind::DocumentUpdate {
update_ids: mut ids,
} }
| BatchKind::DocumentDeletion { | BatchKind::DocumentDeletion {
deletion_ids: mut ids, deletion_ids: mut ids,
@ -120,12 +116,9 @@ impl BatchKind {
settings_ids: mut ids, settings_ids: mut ids,
mut other, mut other,
} }
| BatchKind::SettingsAndDocumentAddition { | BatchKind::SettingsAndDocumentImport {
addition_ids: mut ids, import_ids: mut ids,
settings_ids: mut other, method: _,
}
| BatchKind::SettingsAndDocumentUpdate {
update_ids: mut ids,
settings_ids: mut other, settings_ids: mut other,
}, },
Kind::IndexDeletion, Kind::IndexDeletion,
@ -147,11 +140,9 @@ impl BatchKind {
Kind::DocumentAddition | Kind::DocumentUpdate | Kind::Settings, Kind::DocumentAddition | Kind::DocumentUpdate | Kind::Settings,
) => ControlFlow::Break(this), ) => ControlFlow::Break(this),
( (
BatchKind::DocumentAddition { BatchKind::DocumentImport {
addition_ids: mut ids, method: _,
} import_ids: mut ids,
| BatchKind::DocumentUpdate {
update_ids: mut ids,
}, },
Kind::DocumentClear, Kind::DocumentClear,
) => { ) => {
@ -160,30 +151,43 @@ impl BatchKind {
} }
// we can autobatch the same kind of document additions / updates // we can autobatch the same kind of document additions / updates
(BatchKind::DocumentAddition { mut addition_ids }, Kind::DocumentAddition) => { (
addition_ids.push(id); BatchKind::DocumentImport {
ControlFlow::Continue(BatchKind::DocumentAddition { addition_ids }) method: ReplaceDocuments,
mut import_ids,
},
Kind::DocumentAddition,
) => {
import_ids.push(id);
ControlFlow::Continue(BatchKind::DocumentImport {
method: ReplaceDocuments,
import_ids,
})
} }
(BatchKind::DocumentUpdate { mut update_ids }, Kind::DocumentUpdate) => { (
update_ids.push(id); BatchKind::DocumentImport {
ControlFlow::Continue(BatchKind::DocumentUpdate { update_ids }) method: UpdateDocuments,
mut import_ids,
},
Kind::DocumentUpdate,
) => {
import_ids.push(id);
ControlFlow::Continue(BatchKind::DocumentImport {
method: UpdateDocuments,
import_ids,
})
} }
// but we can't autobatch documents if it's not the same kind // but we can't autobatch documents if it's not the same kind
// this match branch MUST be AFTER the previous one // this match branch MUST be AFTER the previous one
( (
this @ BatchKind::DocumentAddition { .. } | this @ BatchKind::DocumentUpdate { .. }, this @ BatchKind::DocumentImport { .. },
Kind::DocumentDeletion | Kind::DocumentAddition | Kind::DocumentUpdate, Kind::DocumentDeletion | Kind::DocumentAddition | Kind::DocumentUpdate,
) => ControlFlow::Break(this), ) => ControlFlow::Break(this),
(BatchKind::DocumentAddition { addition_ids }, Kind::Settings) => { (BatchKind::DocumentImport { method, import_ids }, Kind::Settings) => {
ControlFlow::Continue(BatchKind::SettingsAndDocumentAddition { ControlFlow::Continue(BatchKind::SettingsAndDocumentImport {
settings_ids: vec![id], settings_ids: vec![id],
addition_ids, method,
}) import_ids,
}
(BatchKind::DocumentUpdate { update_ids }, Kind::Settings) => {
ControlFlow::Continue(BatchKind::SettingsAndDocumentUpdate {
settings_ids: vec![id],
update_ids,
}) })
} }
@ -260,18 +264,14 @@ impl BatchKind {
}) })
} }
( (
BatchKind::SettingsAndDocumentAddition { BatchKind::SettingsAndDocumentImport {
settings_ids, settings_ids,
addition_ids: mut other, method: _,
} import_ids: mut other,
| BatchKind::SettingsAndDocumentUpdate {
settings_ids,
update_ids: mut other,
}, },
Kind::DocumentClear, Kind::DocumentClear,
) => { ) => {
other.push(id); other.push(id);
ControlFlow::Continue(BatchKind::ClearAndSettings { ControlFlow::Continue(BatchKind::ClearAndSettings {
settings_ids, settings_ids,
other, other,
@ -280,62 +280,54 @@ impl BatchKind {
// we can batch the settings with a kind of document operation with the same kind of document operation // we can batch the settings with a kind of document operation with the same kind of document operation
( (
BatchKind::SettingsAndDocumentAddition { BatchKind::SettingsAndDocumentImport {
mut addition_ids,
settings_ids, settings_ids,
method: ReplaceDocuments,
mut import_ids,
}, },
Kind::DocumentAddition, Kind::DocumentAddition,
) => { ) => {
addition_ids.push(id); import_ids.push(id);
ControlFlow::Continue(BatchKind::SettingsAndDocumentAddition { ControlFlow::Continue(BatchKind::SettingsAndDocumentImport {
addition_ids,
settings_ids, settings_ids,
method: ReplaceDocuments,
import_ids,
}) })
} }
( (
BatchKind::SettingsAndDocumentUpdate { BatchKind::SettingsAndDocumentImport {
mut update_ids,
settings_ids, settings_ids,
method: UpdateDocuments,
mut import_ids,
}, },
Kind::DocumentUpdate, Kind::DocumentUpdate,
) => { ) => {
update_ids.push(id); import_ids.push(id);
ControlFlow::Continue(BatchKind::SettingsAndDocumentUpdate { ControlFlow::Continue(BatchKind::SettingsAndDocumentImport {
update_ids,
settings_ids, settings_ids,
method: UpdateDocuments,
import_ids,
}) })
} }
// But we can't batch a settings and a doc op with another doc op // But we can't batch a settings and a doc op with another doc op
// this MUST be AFTER the two previous branch // this MUST be AFTER the two previous branch
( (
this @ BatchKind::SettingsAndDocumentAddition { .. } this @ BatchKind::SettingsAndDocumentImport { .. },
| this @ BatchKind::SettingsAndDocumentUpdate { .. },
Kind::DocumentDeletion | Kind::DocumentAddition | Kind::DocumentUpdate, Kind::DocumentDeletion | Kind::DocumentAddition | Kind::DocumentUpdate,
) => ControlFlow::Break(this), ) => ControlFlow::Break(this),
( (
BatchKind::SettingsAndDocumentAddition { BatchKind::SettingsAndDocumentImport {
mut settings_ids, mut settings_ids,
addition_ids, method,
import_ids,
}, },
Kind::Settings, Kind::Settings,
) => { ) => {
settings_ids.push(id); settings_ids.push(id);
ControlFlow::Continue(BatchKind::SettingsAndDocumentAddition { ControlFlow::Continue(BatchKind::SettingsAndDocumentImport {
settings_ids, settings_ids,
addition_ids, method,
}) import_ids,
}
(
BatchKind::SettingsAndDocumentUpdate {
mut settings_ids,
update_ids,
},
Kind::Settings,
) => {
settings_ids.push(id);
ControlFlow::Continue(BatchKind::SettingsAndDocumentUpdate {
settings_ids,
update_ids,
}) })
} }
(_, Kind::CancelTask | Kind::DumpExport | Kind::Snapshot) => unreachable!(), (_, Kind::CancelTask | Kind::DumpExport | Kind::Snapshot) => unreachable!(),
@ -391,11 +383,11 @@ mod tests {
#[test] #[test]
fn autobatch_simple_operation_together() { fn autobatch_simple_operation_together() {
// we can autobatch one or multiple DocumentAddition together // we can autobatch one or multiple DocumentAddition together
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, DocumentAddition]), @"Some(DocumentAddition { addition_ids: [0, 1, 2] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, DocumentAddition]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0, 1, 2] })");
// we can autobatch one or multiple DocumentUpdate together // we can autobatch one or multiple DocumentUpdate together
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentUpdate, DocumentUpdate]), @"Some(DocumentUpdate { update_ids: [0, 1, 2] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentUpdate, DocumentUpdate]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0, 1, 2] })");
// we can autobatch one or multiple DocumentDeletion together // we can autobatch one or multiple DocumentDeletion together
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentDeletion, DocumentDeletion]), @"Some(DocumentDeletion { deletion_ids: [0, 1, 2] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentDeletion, DocumentDeletion]), @"Some(DocumentDeletion { deletion_ids: [0, 1, 2] })");
@ -407,57 +399,57 @@ mod tests {
#[test] #[test]
fn simple_document_operation_dont_autobatch_with_other() { fn simple_document_operation_dont_autobatch_with_other() {
// addition, updates and deletion can't batch together // addition, updates and deletion can't batch together
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentUpdate]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentUpdate]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentDeletion]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentDeletion]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentAddition]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentAddition]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentDeletion]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, DocumentDeletion]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentAddition]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentAddition]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentUpdate]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, DocumentUpdate]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexCreation]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexCreation]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexCreation]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexCreation]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexCreation]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexCreation]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexUpdate]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexUpdate]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexUpdate]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexUpdate]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexUpdate]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexUpdate]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexRename]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexRename]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexRename]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexRename]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexRename]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexRename]), @"Some(DocumentDeletion { deletion_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexSwap]), @"Some(DocumentAddition { addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, IndexSwap]), @"Some(DocumentImport { method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexSwap]), @"Some(DocumentUpdate { update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, IndexSwap]), @"Some(DocumentImport { method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexSwap]), @"Some(DocumentDeletion { deletion_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentDeletion, IndexSwap]), @"Some(DocumentDeletion { deletion_ids: [0] })");
} }
#[test] #[test]
fn document_addition_batch_with_settings() { fn document_addition_batch_with_settings() {
// simple case // simple case
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
// multiple settings and doc addition // multiple settings and doc addition
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, Settings, Settings]), @"Some(SettingsAndDocumentAddition { settings_ids: [2, 3], addition_ids: [0, 1] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, Settings, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [2, 3], method: ReplaceDocuments, import_ids: [0, 1] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, Settings, Settings]), @"Some(SettingsAndDocumentAddition { settings_ids: [2, 3], addition_ids: [0, 1] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, DocumentAddition, Settings, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [2, 3], method: ReplaceDocuments, import_ids: [0, 1] })");
// addition and setting unordered // addition and setting unordered
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentAddition, Settings]), @"Some(SettingsAndDocumentAddition { settings_ids: [1, 3], addition_ids: [0, 2] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentAddition, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [1, 3], method: ReplaceDocuments, import_ids: [0, 2] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentUpdate, Settings]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1, 3], update_ids: [0, 2] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentUpdate, Settings]), @"Some(SettingsAndDocumentImport { settings_ids: [1, 3], method: UpdateDocuments, import_ids: [0, 2] })");
// We ensure this kind of batch doesn't batch with forbidden operations // We ensure this kind of batch doesn't batch with forbidden operations
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentUpdate]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentUpdate]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentAddition]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentAddition]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentDeletion]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, DocumentDeletion]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentDeletion]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, DocumentDeletion]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexCreation]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexCreation]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexCreation]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexCreation]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexUpdate]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexUpdate]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexUpdate]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexUpdate]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexRename]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexRename]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexRename]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexRename]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexSwap]), @"Some(SettingsAndDocumentAddition { settings_ids: [1], addition_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentAddition, Settings, IndexSwap]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: ReplaceDocuments, import_ids: [0] })");
assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexSwap]), @"Some(SettingsAndDocumentUpdate { settings_ids: [1], update_ids: [0] })"); assert_smol_debug_snapshot!(autobatch_from([DocumentUpdate, Settings, IndexSwap]), @"Some(SettingsAndDocumentImport { settings_ids: [1], method: UpdateDocuments, import_ids: [0] })");
} }
#[test] #[test]

View File

@ -12,15 +12,10 @@ pub(crate) enum Batch {
Cancel(Task), Cancel(Task),
Snapshot(Vec<Task>), Snapshot(Vec<Task>),
Dump(Vec<Task>), Dump(Vec<Task>),
DocumentAddition { DocumentImport {
index_uid: String,
primary_key: Option<String>,
content_files: Vec<Uuid>,
tasks: Vec<Task>,
},
DocumentUpdate {
index_uid: String, index_uid: String,
primary_key: Option<String>, primary_key: Option<String>,
method: IndexDocumentsMethod,
content_files: Vec<Uuid>, content_files: Vec<Uuid>,
tasks: Vec<Task>, tasks: Vec<Task>,
}, },
@ -47,23 +42,13 @@ pub(crate) enum Batch {
settings: Vec<(bool, Settings<Unchecked>)>, settings: Vec<(bool, Settings<Unchecked>)>,
settings_tasks: Vec<Task>, settings_tasks: Vec<Task>,
}, },
SettingsAndDocumentAddition { SettingsAndDocumentImport {
index_uid: String, index_uid: String,
primary_key: Option<String>, primary_key: Option<String>,
method: IndexDocumentsMethod,
content_files: Vec<Uuid>, content_files: Vec<Uuid>,
document_addition_tasks: Vec<Task>, document_import_tasks: Vec<Task>,
// TODO what's that boolean, does it mean that it removes things or what?
settings: Vec<(bool, Settings<Unchecked>)>,
settings_tasks: Vec<Task>,
},
SettingsAndDocumentUpdate {
index_uid: String,
primary_key: Option<String>,
content_files: Vec<Uuid>,
document_update_tasks: Vec<Task>,
// TODO what's that boolean, does it mean that it removes things or what? // TODO what's that boolean, does it mean that it removes things or what?
settings: Vec<(bool, Settings<Unchecked>)>, settings: Vec<(bool, Settings<Unchecked>)>,
@ -93,14 +78,13 @@ impl Batch {
| Batch::IndexUpdate { task, .. } => vec![task.uid], | Batch::IndexUpdate { task, .. } => vec![task.uid],
Batch::Snapshot(tasks) Batch::Snapshot(tasks)
| Batch::Dump(tasks) | Batch::Dump(tasks)
| Batch::DocumentAddition { tasks, .. } | Batch::DocumentImport { tasks, .. }
| Batch::DocumentUpdate { tasks, .. }
| Batch::DocumentDeletion { tasks, .. } | Batch::DocumentDeletion { tasks, .. }
| Batch::Settings { tasks, .. } | Batch::Settings { tasks, .. }
| Batch::DocumentClear { tasks, .. } | Batch::DocumentClear { tasks, .. }
| Batch::IndexDeletion { tasks, .. } => tasks.iter().map(|task| task.uid).collect(), | Batch::IndexDeletion { tasks, .. } => tasks.iter().map(|task| task.uid).collect(),
Batch::SettingsAndDocumentAddition { Batch::SettingsAndDocumentImport {
document_addition_tasks: tasks, document_import_tasks: tasks,
settings_tasks: other, settings_tasks: other,
.. ..
} }
@ -108,11 +92,6 @@ impl Batch {
cleared_tasks: tasks, cleared_tasks: tasks,
settings_tasks: other, settings_tasks: other,
.. ..
}
| Batch::SettingsAndDocumentUpdate {
document_update_tasks: tasks,
settings_tasks: other,
..
} => tasks.iter().chain(other).map(|task| task.uid).collect(), } => tasks.iter().chain(other).map(|task| task.uid).collect(),
} }
} }
@ -130,44 +109,24 @@ impl IndexScheduler {
tasks: self.get_existing_tasks(rtxn, ids)?, tasks: self.get_existing_tasks(rtxn, ids)?,
index_uid, index_uid,
})), })),
BatchKind::DocumentAddition { addition_ids } => { BatchKind::DocumentImport { method, import_ids } => {
let tasks = self.get_existing_tasks(rtxn, addition_ids)?; let tasks = self.get_existing_tasks(rtxn, import_ids)?;
let primary_key = match &tasks[0].kind { let primary_key = match &tasks[0].kind {
KindWithContent::DocumentAddition { primary_key, .. } => primary_key.clone(), KindWithContent::DocumentImport { primary_key, .. } => primary_key.clone(),
_ => unreachable!(), _ => unreachable!(),
}; };
let content_files = tasks let content_files = tasks
.iter() .iter()
.map(|task| match task.kind { .map(|task| match task.kind {
KindWithContent::DocumentAddition { content_file, .. } => content_file, KindWithContent::DocumentImport { content_file, .. } => content_file,
_ => unreachable!(), _ => unreachable!(),
}) })
.collect(); .collect();
Ok(Some(Batch::DocumentAddition { Ok(Some(Batch::DocumentImport {
index_uid,
primary_key,
content_files,
tasks,
}))
}
BatchKind::DocumentUpdate { update_ids } => {
let tasks = self.get_existing_tasks(rtxn, update_ids)?;
let primary_key = match &tasks[0].kind {
KindWithContent::DocumentUpdate { primary_key, .. } => primary_key.clone(),
_ => unreachable!(),
};
let content_files = tasks
.iter()
.map(|task| match task.kind {
KindWithContent::DocumentUpdate { content_file, .. } => content_file,
_ => unreachable!(),
})
.collect();
Ok(Some(Batch::DocumentUpdate {
index_uid, index_uid,
primary_key, primary_key,
method,
content_files, content_files,
tasks, tasks,
})) }))
@ -246,51 +205,10 @@ impl IndexScheduler {
settings_tasks, settings_tasks,
})) }))
} }
BatchKind::SettingsAndDocumentAddition { BatchKind::SettingsAndDocumentImport {
addition_ids,
settings_ids,
} => {
let (index_uid, settings, settings_tasks) = match self
.create_next_batch_index(rtxn, index_uid, BatchKind::Settings { settings_ids })?
.unwrap()
{
Batch::Settings {
index_uid,
settings,
tasks,
} => (index_uid, settings, tasks),
_ => unreachable!(),
};
let (index_uid, primary_key, content_files, document_addition_tasks) = match self
.create_next_batch_index(
rtxn,
index_uid,
BatchKind::DocumentAddition { addition_ids },
)?
.unwrap()
{
Batch::DocumentAddition {
index_uid,
primary_key,
content_files,
tasks,
} => (index_uid, primary_key, content_files, tasks),
_ => unreachable!(),
};
Ok(Some(Batch::SettingsAndDocumentAddition {
index_uid,
primary_key,
content_files,
document_addition_tasks,
settings,
settings_tasks,
}))
}
BatchKind::SettingsAndDocumentUpdate {
update_ids,
settings_ids, settings_ids,
method,
import_ids,
} => { } => {
let settings = self.create_next_batch_index( let settings = self.create_next_batch_index(
rtxn, rtxn,
@ -298,18 +216,18 @@ impl IndexScheduler {
BatchKind::Settings { settings_ids }, BatchKind::Settings { settings_ids },
)?; )?;
let document_update = self.create_next_batch_index( let document_import = self.create_next_batch_index(
rtxn, rtxn,
index_uid.clone(), index_uid.clone(),
BatchKind::DocumentUpdate { update_ids }, BatchKind::DocumentImport { method, import_ids },
)?; )?;
match (document_update, settings) { match (document_import, settings) {
( (
Some(Batch::DocumentUpdate { Some(Batch::DocumentImport {
primary_key, primary_key,
content_files, content_files,
tasks: document_update_tasks, tasks: document_import_tasks,
.. ..
}), }),
Some(Batch::Settings { Some(Batch::Settings {
@ -317,11 +235,12 @@ impl IndexScheduler {
tasks: settings_tasks, tasks: settings_tasks,
.. ..
}), }),
) => Ok(Some(Batch::SettingsAndDocumentUpdate { ) => Ok(Some(Batch::SettingsAndDocumentImport {
index_uid, index_uid,
primary_key, primary_key,
method,
content_files, content_files,
document_update_tasks, document_import_tasks,
settings, settings,
settings_tasks, settings_tasks,
})), })),
@ -453,10 +372,10 @@ impl IndexScheduler {
Ok(tasks) Ok(tasks)
} }
// TODO we should merge both document import with a method field Batch::DocumentImport {
Batch::DocumentAddition {
index_uid, index_uid,
primary_key, primary_key,
method,
content_files, content_files,
mut tasks, mut tasks,
} => { } => {
@ -467,7 +386,7 @@ impl IndexScheduler {
wtxn.commit()?; wtxn.commit()?;
let ret = index.update_documents( let ret = index.update_documents(
IndexDocumentsMethod::ReplaceDocuments, method,
primary_key, primary_key,
self.file_store.clone(), self.file_store.clone(),
content_files, content_files,
@ -490,53 +409,17 @@ impl IndexScheduler {
Ok(tasks) Ok(tasks)
} }
Batch::SettingsAndDocumentAddition { Batch::SettingsAndDocumentImport {
index_uid, index_uid,
primary_key, primary_key,
method,
content_files, content_files,
document_addition_tasks, document_import_tasks,
settings: _, settings: _,
settings_tasks: _, settings_tasks: _,
} => { } => {
todo!(); todo!();
} }
// TODO we should merge both document import with a method field
Batch::DocumentUpdate {
index_uid,
primary_key,
content_files,
mut tasks,
} => {
// we NEED a write transaction for the index creation.
// To avoid blocking the whole process we're going to commit asap.
let mut wtxn = self.env.write_txn()?;
let index = self.index_mapper.create_index(&mut wtxn, &index_uid)?;
wtxn.commit()?;
let ret = index.update_documents(
IndexDocumentsMethod::UpdateDocuments,
primary_key,
self.file_store.clone(),
content_files,
)?;
for (task, ret) in tasks.iter_mut().zip(ret) {
match ret {
Ok(DocumentAdditionResult {
indexed_documents,
number_of_documents,
}) => {
task.details = Some(Details::DocumentAddition {
received_documents: number_of_documents,
indexed_documents,
});
}
Err(error) => task.error = Some(error.into()),
}
}
Ok(tasks)
}
Batch::DocumentDeletion { Batch::DocumentDeletion {
index_uid, index_uid,
documents, documents,
@ -628,14 +511,6 @@ impl IndexScheduler {
tasks.append(&mut settings_tasks); tasks.append(&mut settings_tasks);
Ok(tasks) Ok(tasks)
} }
Batch::SettingsAndDocumentUpdate {
index_uid,
primary_key,
content_files,
document_update_tasks,
settings,
settings_tasks,
} => todo!(),
Batch::IndexCreation { Batch::IndexCreation {
index_uid, index_uid,
primary_key, primary_key,

View File

@ -84,10 +84,7 @@ impl Query {
} }
pub fn with_limit(self, limit: u32) -> Self { pub fn with_limit(self, limit: u32) -> Self {
Self { Self { limit, ..self }
limit,
..self
}
} }
} }
@ -435,6 +432,7 @@ impl IndexScheduler {
mod tests { mod tests {
use big_s::S; use big_s::S;
use insta::*; use insta::*;
use milli::update::IndexDocumentsMethod::{self, ReplaceDocuments, UpdateDocuments};
use tempfile::TempDir; use tempfile::TempDir;
use uuid::Uuid; use uuid::Uuid;
@ -501,24 +499,27 @@ mod tests {
index_uid: S("catto"), index_uid: S("catto"),
primary_key: Some(S("mouse")), primary_key: Some(S("mouse")),
}, },
KindWithContent::DocumentAddition { KindWithContent::DocumentImport {
index_uid: S("catto"), index_uid: S("catto"),
primary_key: None, primary_key: None,
method: ReplaceDocuments,
content_file: Uuid::new_v4(), content_file: Uuid::new_v4(),
documents_count: 12, documents_count: 12,
allow_index_creation: true, allow_index_creation: true,
}, },
KindWithContent::CancelTask { tasks: vec![0, 1] }, KindWithContent::CancelTask { tasks: vec![0, 1] },
KindWithContent::DocumentAddition { KindWithContent::DocumentImport {
index_uid: S("catto"), index_uid: S("catto"),
primary_key: None, primary_key: None,
method: ReplaceDocuments,
content_file: Uuid::new_v4(), content_file: Uuid::new_v4(),
documents_count: 50, documents_count: 50,
allow_index_creation: true, allow_index_creation: true,
}, },
KindWithContent::DocumentAddition { KindWithContent::DocumentImport {
index_uid: S("doggo"), index_uid: S("doggo"),
primary_key: Some(S("bone")), primary_key: Some(S("bone")),
method: ReplaceDocuments,
content_file: Uuid::new_v4(), content_file: Uuid::new_v4(),
documents_count: 5000, documents_count: 5000,
allow_index_creation: true, allow_index_creation: true,
@ -603,9 +604,10 @@ mod tests {
let documents_count = let documents_count =
document_formats::read_json(content.as_bytes(), file.as_file_mut()).unwrap(); document_formats::read_json(content.as_bytes(), file.as_file_mut()).unwrap();
index_scheduler index_scheduler
.register(KindWithContent::DocumentAddition { .register(KindWithContent::DocumentImport {
index_uid: S("doggos"), index_uid: S("doggos"),
primary_key: Some(S("id")), primary_key: Some(S("id")),
method: ReplaceDocuments,
content_file: uuid, content_file: uuid,
documents_count, documents_count,
allow_index_creation: true, allow_index_creation: true,

View File

@ -1,6 +1,7 @@
use anyhow::Result; use anyhow::Result;
use index::{Settings, Unchecked}; use index::{Settings, Unchecked};
use meilisearch_types::error::ResponseError; use meilisearch_types::error::ResponseError;
use milli::update::IndexDocumentsMethod;
use serde::{Deserialize, Serialize, Serializer}; use serde::{Deserialize, Serialize, Serializer};
use std::{fmt::Write, path::PathBuf, str::FromStr}; use std::{fmt::Write, path::PathBuf, str::FromStr};
@ -125,16 +126,10 @@ impl FromStr for Status {
#[derive(Debug, PartialEq, Serialize, Deserialize)] #[derive(Debug, PartialEq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub enum KindWithContent { pub enum KindWithContent {
DocumentAddition { DocumentImport {
index_uid: String,
primary_key: Option<String>,
content_file: Uuid,
documents_count: usize,
allow_index_creation: bool,
},
DocumentUpdate {
index_uid: String, index_uid: String,
primary_key: Option<String>, primary_key: Option<String>,
method: IndexDocumentsMethod,
content_file: Uuid, content_file: Uuid,
documents_count: usize, documents_count: usize,
allow_index_creation: bool, allow_index_creation: bool,
@ -183,8 +178,15 @@ pub enum KindWithContent {
impl KindWithContent { impl KindWithContent {
pub fn as_kind(&self) -> Kind { pub fn as_kind(&self) -> Kind {
match self { match self {
KindWithContent::DocumentAddition { .. } => Kind::DocumentAddition, KindWithContent::DocumentImport {
KindWithContent::DocumentUpdate { .. } => Kind::DocumentUpdate, method: IndexDocumentsMethod::ReplaceDocuments,
..
} => Kind::DocumentAddition,
KindWithContent::DocumentImport {
method: IndexDocumentsMethod::UpdateDocuments,
..
} => Kind::DocumentUpdate,
KindWithContent::DocumentImport { .. } => unreachable!(),
KindWithContent::DocumentDeletion { .. } => Kind::DocumentDeletion, KindWithContent::DocumentDeletion { .. } => Kind::DocumentDeletion,
KindWithContent::DocumentClear { .. } => Kind::DocumentClear, KindWithContent::DocumentClear { .. } => Kind::DocumentClear,
KindWithContent::Settings { .. } => Kind::Settings, KindWithContent::Settings { .. } => Kind::Settings,
@ -203,7 +205,7 @@ impl KindWithContent {
use KindWithContent::*; use KindWithContent::*;
match self { match self {
DocumentAddition { .. } | DocumentUpdate { .. } => { DocumentImport { .. } => {
// TODO: TAMO: persist the file // TODO: TAMO: persist the file
// content_file.persist(); // content_file.persist();
Ok(()) Ok(())
@ -226,7 +228,7 @@ impl KindWithContent {
use KindWithContent::*; use KindWithContent::*;
match self { match self {
DocumentAddition { .. } | DocumentUpdate { .. } => { DocumentImport { .. } => {
// TODO: TAMO: delete the file // TODO: TAMO: delete the file
// content_file.delete(); // content_file.delete();
Ok(()) Ok(())
@ -250,8 +252,7 @@ impl KindWithContent {
match self { match self {
DumpExport { .. } | Snapshot | CancelTask { .. } => None, DumpExport { .. } | Snapshot | CancelTask { .. } => None,
DocumentAddition { index_uid, .. } DocumentImport { index_uid, .. }
| DocumentUpdate { index_uid, .. }
| DocumentDeletion { index_uid, .. } | DocumentDeletion { index_uid, .. }
| DocumentClear { index_uid } | DocumentClear { index_uid }
| Settings { index_uid, .. } | Settings { index_uid, .. }