diff --git a/quickwit/Cargo.lock b/quickwit/Cargo.lock index e925e879ba4..8c6ad9971d2 100644 --- a/quickwit/Cargo.lock +++ b/quickwit/Cargo.lock @@ -69,9 +69,9 @@ dependencies = [ [[package]] name = "aho-corasick" -version = "1.0.3" +version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "86b8f9420f797f2d9e935edf629310eb938a0d839f984e25327f3c7eed22300c" +checksum = "6748e8def348ed4d14996fa801f4122cd763fff530258cdc03f64b25f89d3a5a" dependencies = [ "memchr", ] @@ -178,9 +178,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.72" +version = "1.0.75" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b13c32d80ecc7ab747b80c3784bce54ee8a7a0cc4fbda9bf4cda2cf6fe90854" +checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6" [[package]] name = "anymap" @@ -291,7 +291,7 @@ checksum = "16e62a023e7c117e27523144c5d2459f4397fcc3cab0085af8e2224f643a0193" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -302,7 +302,7 @@ checksum = "bc00ceb34980c03614e35a3a4e218276a0a824e911d07651cd0d858a51e8c0f0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -1277,18 +1277,18 @@ dependencies = [ [[package]] name = "clap" -version = "4.3.21" +version = "4.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c27cdf28c0f604ba3f512b0c9a409f8de8513e4816705deb0498b627e7c3a3fd" +checksum = "b417ae4361bca3f5de378294fc7472d3c4ed86a5ef9f49e93ae722f432aae8d2" dependencies = [ "clap_builder", ] [[package]] name = "clap_builder" -version = "4.3.21" +version = "4.3.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08a9f1ab5e9f01a9b81f202e8562eb9a10de70abf9eaeac1be465c28b75aa4aa" +checksum = "9c90dc0f0e42c64bff177ca9d7be6fcc9ddb0f26a6e062174a61c84dd6c644d4" dependencies = [ "anstream", "anstyle", @@ -1680,7 +1680,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -1702,7 +1702,7 @@ checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" dependencies = [ "darling_core 0.20.3", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -1912,9 +1912,9 @@ dependencies = [ [[package]] name = "dyn-clone" -version = "1.0.12" +version = "1.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "304e6508efa593091e97a9abbc10f90aa7ca635b6d2784feff3c89d41dd12272" +checksum = "bbfc4744c1b8f2a09adc0e55242f60b1af195d88596bd8700be74418c056c555" [[package]] name = "either" @@ -2051,7 +2051,7 @@ checksum = "eecf8589574ce9b895052fa12d69af7a233f99e6107f5cb8dd1044f2a17bfdcb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -2075,9 +2075,9 @@ checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] name = "erased-serde" -version = "0.3.28" +version = "0.3.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da96524cc884f6558f1769b6c46686af2fe8e8b4cd253bd5a3cdba8181b8e070" +checksum = "fc978899517288e3ebbd1a3bfc1d9537dbb87eeab149e53ea490e63bcdff561a" dependencies = [ "serde", ] @@ -2184,9 +2184,9 @@ checksum = "0ce7134b9999ecaf8bcd65542e436736ef32ddca1b3e06094cb6ec5755203b80" [[package]] name = "flate2" -version = "1.0.26" +version = "1.0.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b9429470923de8e8cbd4d2dc513535400b4b3fef0319fb5c4e1f520a7bef743" +checksum = "c6c98ee8095e9d1dcbf2fcc6d95acccb90d1c81db1e44725c6a984b1dbdfb010" dependencies = [ "crc32fast", "miniz_oxide", @@ -2348,7 +2348,7 @@ checksum = "89ca545a94061b6365f2c7355b4b32bd20df3ff95f02da9329b34ccc3bd6ee72" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -4049,7 +4049,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -4196,9 +4196,9 @@ dependencies = [ [[package]] name = "ordered-float" -version = "3.7.0" +version = "3.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2fc2dbde8f8a79f2102cc474ceb0ad68e3b80b85289ea62389b60e66777e4213" +checksum = "7417b1484e3641a8791af3c3123cdc083ac60a0d262a2f281b6125d58917caf4" dependencies = [ "num-traits", ] @@ -4234,7 +4234,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -4296,7 +4296,7 @@ dependencies = [ "libc", "redox_syscall 0.3.5", "smallvec", - "windows-targets 0.48.1", + "windows-targets 0.48.3", ] [[package]] @@ -4446,7 +4446,7 @@ checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -4707,7 +4707,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c64d9ba0963cdcea2e1b2230fbae2bab30eb25a174be395c41e764bfb65dd62" dependencies = [ "proc-macro2", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -5085,7 +5085,7 @@ dependencies = [ "prost-build", "quote", "serde", - "syn 2.0.28", + "syn 2.0.29", "tonic-build", ] @@ -5528,7 +5528,7 @@ dependencies = [ "proc-macro2", "quickwit-macros-impl", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -5538,7 +5538,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -5623,6 +5623,7 @@ dependencies = [ "quickwit-common", "serde", "serde_json", + "sqlx", "thiserror", "tokio", "tonic 0.9.2", @@ -5859,9 +5860,9 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.32" +version = "1.0.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50f3b39ccfb720540debaa0164757101c08ecb8d326b15358ce76a62c7e85965" +checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" dependencies = [ "proc-macro2", ] @@ -6259,7 +6260,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn 2.0.28", + "syn 2.0.29", "walkdir", ] @@ -6551,14 +6552,14 @@ checksum = "aafe972d60b0b9bee71a91b92fee2d4fb3c9d7e8f6b179aa99f27203d99a4816" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] name = "serde_json" -version = "1.0.104" +version = "1.0.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "076066c5f1078eac5b722a31827a8832fe108bed65dfa75e233c89f8206e976c" +checksum = "693151e1ac27563d6dbcec9dee9fbd5da8539b20fa14ad3752b2e6d363ace360" dependencies = [ "itoa", "ryu", @@ -6678,7 +6679,7 @@ dependencies = [ "darling 0.20.3", "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -7196,9 +7197,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.28" +version = "2.0.29" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04361975b3f5e348b2189d8dc55bc942f278b2d482a6a0365de5bdd62d351567" +checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" dependencies = [ "proc-macro2", "quote", @@ -7441,22 +7442,22 @@ checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76" [[package]] name = "thiserror" -version = "1.0.44" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "611040a08a0439f8248d1990b111c95baa9c704c805fa1f62104b39655fd7f90" +checksum = "97a802ec30afc17eee47b2855fc72e0c4cd62be9b4efe6591edde0ec5bd68d8f" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.44" +version = "1.0.47" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "090198534930841fab3a5d1bb637cde49e339654e606195f8d9c76eeb081dc96" +checksum = "6bb623b56e39ab7dcd4b1b98bb6c8f8d907ed255b18de254088016b27a8ee19b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -7651,7 +7652,7 @@ checksum = "630bdcf245f78637c13ec01ffae6187cca34625e8c63150d424b59e55af2675e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -7933,7 +7934,7 @@ checksum = "5f4f31f56159e98206da9efd823404b79b6ef3143b4a7ab76e67b1751b25a4ab" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -8042,9 +8043,9 @@ checksum = "497961ef93d974e23eb6f433eb5fe1b7930b659f06d12dec6fc44a8f554c0bba" [[package]] name = "typetag" -version = "0.2.12" +version = "0.2.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aec6850cc671cd0cfb3ab285465e48a3b927d9de155051c35797446b32f9169f" +checksum = "80960fd143d4c96275c0e60b08f14b81fbb468e79bc0ef8fbda69fb0afafae43" dependencies = [ "erased-serde", "inventory", @@ -8055,13 +8056,13 @@ dependencies = [ [[package]] name = "typetag-impl" -version = "0.2.12" +version = "0.2.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30c49a6815b4f8379c36f06618bc1b80ca77aaf8a3fd4d8549dca6fdb016000f" +checksum = "bfc13d450dc4a695200da3074dacf43d449b968baee95e341920e47f61a3b40f" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -8233,7 +8234,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", ] [[package]] @@ -8261,7 +8262,7 @@ dependencies = [ "bytes", "chrono", "once_cell", - "ordered-float 3.7.0", + "ordered-float 3.8.0", "path", "regex", "serde", @@ -8305,7 +8306,7 @@ dependencies = [ "getrandom 0.2.10", "indoc", "lalrpop-util", - "ordered-float 3.7.0", + "ordered-float 3.8.0", "paste", "path", "regex", @@ -8328,7 +8329,7 @@ dependencies = [ "chrono-tz", "derivative", "nom", - "ordered-float 3.7.0", + "ordered-float 3.8.0", "path", "serde", "serde_json", @@ -8353,7 +8354,7 @@ source = "git+https://github.com/vectordotdev/vrl?rev=v0.3.0#113005bcee6cd7b5ea0 dependencies = [ "lalrpop", "lalrpop-util", - "ordered-float 3.7.0", + "ordered-float 3.8.0", "paste", "path", "thiserror", @@ -8388,7 +8389,7 @@ dependencies = [ "nom", "ofb", "once_cell", - "ordered-float 3.7.0", + "ordered-float 3.8.0", "path", "percent-encoding", "quoted_printable", @@ -8548,7 +8549,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", "wasm-bindgen-shared", ] @@ -8582,7 +8583,7 @@ checksum = "54681b18a46765f095758388f2d0cf16eb8d4169b639ab575a8f5693af210c7b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.28", + "syn 2.0.29", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -8724,7 +8725,7 @@ version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e686886bc078bc1b0b600cac0147aadb815089b6e4da64016cbd754b6342700f" dependencies = [ - "windows-targets 0.48.1", + "windows-targets 0.48.3", ] [[package]] @@ -8742,7 +8743,7 @@ version = "0.48.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" dependencies = [ - "windows-targets 0.48.1", + "windows-targets 0.48.3", ] [[package]] @@ -8762,17 +8763,17 @@ dependencies = [ [[package]] name = "windows-targets" -version = "0.48.1" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05d4b17490f70499f20b9e791dcf6a299785ce8af4d709018206dc5b4953e95f" +checksum = "27f51fb4c64f8b770a823c043c7fad036323e1c48f55287b7bbb7987b2fcdf3b" dependencies = [ - "windows_aarch64_gnullvm 0.48.0", - "windows_aarch64_msvc 0.48.0", - "windows_i686_gnu 0.48.0", - "windows_i686_msvc 0.48.0", - "windows_x86_64_gnu 0.48.0", - "windows_x86_64_gnullvm 0.48.0", - "windows_x86_64_msvc 0.48.0", + "windows_aarch64_gnullvm 0.48.3", + "windows_aarch64_msvc 0.48.3", + "windows_i686_gnu 0.48.3", + "windows_i686_msvc 0.48.3", + "windows_x86_64_gnu 0.48.3", + "windows_x86_64_gnullvm 0.48.3", + "windows_x86_64_msvc 0.48.3", ] [[package]] @@ -8783,9 +8784,9 @@ checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8" [[package]] name = "windows_aarch64_gnullvm" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc" +checksum = "fde1bb55ae4ce76a597a8566d82c57432bc69c039449d61572a7a353da28f68c" [[package]] name = "windows_aarch64_msvc" @@ -8795,9 +8796,9 @@ checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43" [[package]] name = "windows_aarch64_msvc" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3" +checksum = "1513e8d48365a78adad7322fd6b5e4c4e99d92a69db8df2d435b25b1f1f286d4" [[package]] name = "windows_i686_gnu" @@ -8807,9 +8808,9 @@ checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f" [[package]] name = "windows_i686_gnu" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241" +checksum = "60587c0265d2b842298f5858e1a5d79d146f9ee0c37be5782e92a6eb5e1d7a83" [[package]] name = "windows_i686_msvc" @@ -8819,9 +8820,9 @@ checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060" [[package]] name = "windows_i686_msvc" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00" +checksum = "224fe0e0ffff5d2ea6a29f82026c8f43870038a0ffc247aa95a52b47df381ac4" [[package]] name = "windows_x86_64_gnu" @@ -8831,9 +8832,9 @@ checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36" [[package]] name = "windows_x86_64_gnu" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1" +checksum = "62fc52a0f50a088de499712cbc012df7ebd94e2d6eb948435449d76a6287e7ad" [[package]] name = "windows_x86_64_gnullvm" @@ -8843,9 +8844,9 @@ checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3" [[package]] name = "windows_x86_64_gnullvm" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953" +checksum = "2093925509d91ea3d69bcd20238f4c2ecdb1a29d3c281d026a09705d0dd35f3d" [[package]] name = "windows_x86_64_msvc" @@ -8855,15 +8856,15 @@ checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0" [[package]] name = "windows_x86_64_msvc" -version = "0.48.0" +version = "0.48.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" +checksum = "b6ade45bc8bf02ae2aa34a9d54ba660a1a58204da34ba793c00d83ca3730b5f1" [[package]] name = "winnow" -version = "0.5.10" +version = "0.5.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5504cc7644f4b593cbc05c4a55bf9bd4e94b867c3c0bd440934174d50482427d" +checksum = "d09770118a7eb1ccaf4a594a221334119a44a814fcb0d31c5b85e83e97227a97" dependencies = [ "memchr", ] diff --git a/quickwit/quickwit-cli/tests/cli.rs b/quickwit/quickwit-cli/tests/cli.rs index 7cd01ee7b06..458b07c7373 100644 --- a/quickwit/quickwit-cli/tests/cli.rs +++ b/quickwit/quickwit-cli/tests/cli.rs @@ -41,7 +41,8 @@ use quickwit_common::fs::get_cache_directory_path; use quickwit_common::rand::append_random_suffix; use quickwit_common::uri::Uri; use quickwit_config::{SourceInputFormat, CLI_INGEST_SOURCE_ID}; -use quickwit_metastore::{MetastoreError, MetastoreResolver, SplitState}; +use quickwit_metastore::{MetastoreResolver, SplitState}; +use quickwit_proto::metastore::{EntityKind, MetastoreError}; use serde_json::{json, Number, Value}; use tokio::time::{sleep, Duration}; @@ -98,7 +99,7 @@ async fn test_cmd_create() { // Creating an existing index should fail. let error = create_logs_index(&test_env).await.unwrap_err(); - assert!(error.to_string().contains("already exists"),); + assert!(error.to_string().contains("already exist(s)"),); } #[tokio::test] @@ -187,9 +188,9 @@ async fn test_cmd_ingest_on_non_existing_index() { assert_eq!( error.root_cause().downcast_ref::().unwrap(), - &MetastoreError::IndexesDoNotExist { - index_ids: vec!["index-does-not-exist".to_string()] - } + &MetastoreError::NotFound(EntityKind::Index { + index_id: "index-does-not-exist".to_string() + }) ); } diff --git a/quickwit/quickwit-index-management/src/garbage_collection.rs b/quickwit/quickwit-index-management/src/garbage_collection.rs index 57399fa73b9..6fdd53036fa 100644 --- a/quickwit/quickwit-index-management/src/garbage_collection.rs +++ b/quickwit/quickwit-index-management/src/garbage_collection.rs @@ -24,9 +24,8 @@ use std::time::Duration; use futures::Future; use quickwit_common::{PrettySample, Progress}; -use quickwit_metastore::{ - ListSplitsQuery, Metastore, MetastoreError, SplitInfo, SplitMetadata, SplitState, -}; +use quickwit_metastore::{ListSplitsQuery, Metastore, SplitInfo, SplitMetadata, SplitState}; +use quickwit_proto::metastore::MetastoreError; use quickwit_proto::IndexUid; use quickwit_storage::{BulkDeleteError, Storage}; use thiserror::Error; @@ -330,6 +329,7 @@ mod tests { use quickwit_metastore::{ metastore_for_test, ListSplitsQuery, MockMetastore, SplitMetadata, SplitState, }; + use quickwit_proto::metastore::EntityKind; use quickwit_proto::IndexUid; use quickwit_storage::{ storage_for_test, BulkDeleteError, DeleteFailure, MockStorage, PutPayload, @@ -652,9 +652,9 @@ mod tests { let mut mock_metastore = MockMetastore::new(); mock_metastore.expect_delete_splits().return_once(|_, _| { - Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }) + Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })) }); let metastore = Arc::new(mock_metastore); diff --git a/quickwit/quickwit-index-management/src/index.rs b/quickwit/quickwit-index-management/src/index.rs index 5e29ee7b9e6..10e262d7caa 100644 --- a/quickwit/quickwit-index-management/src/index.rs +++ b/quickwit/quickwit-index-management/src/index.rs @@ -25,8 +25,9 @@ use quickwit_common::fs::{empty_dir, get_cache_directory_path}; use quickwit_config::{validate_identifier, IndexConfig, SourceConfig}; use quickwit_indexing::check_source_connectivity; use quickwit_metastore::{ - IndexMetadata, ListSplitsQuery, Metastore, MetastoreError, SplitInfo, SplitMetadata, SplitState, + IndexMetadata, ListSplitsQuery, Metastore, SplitInfo, SplitMetadata, SplitState, }; +use quickwit_proto::metastore::{EntityKind, MetastoreError}; use quickwit_proto::{IndexUid, ServiceError, ServiceErrorCode}; use quickwit_storage::{StorageResolver, StorageResolverError}; use thiserror::Error; @@ -40,11 +41,11 @@ use crate::garbage_collection::{ #[derive(Error, Debug)] pub enum IndexServiceError { #[error("Failed to resolve the storage `{0}`.")] - StorageError(#[from] StorageResolverError), + Storage(#[from] StorageResolverError), #[error("Metastore error `{0}`.")] - MetastoreError(#[from] MetastoreError), + Metastore(#[from] MetastoreError), #[error("Split deletion error `{0}`.")] - SplitDeletionError(#[from] DeleteSplitsError), + SplitDeletion(#[from] DeleteSplitsError), #[error("Invalid config: {0:#}.")] InvalidConfig(anyhow::Error), #[error("Invalid identifier: {0}.")] @@ -58,13 +59,13 @@ pub enum IndexServiceError { impl ServiceError for IndexServiceError { fn status_code(&self) -> ServiceErrorCode { match self { - Self::StorageError(_) => ServiceErrorCode::Internal, - Self::MetastoreError(error) => error.status_code(), - Self::SplitDeletionError(_) => ServiceErrorCode::Internal, + Self::Internal(_) => ServiceErrorCode::Internal, Self::InvalidConfig(_) => ServiceErrorCode::BadRequest, Self::InvalidIdentifier(_) => ServiceErrorCode::BadRequest, + Self::Metastore(error) => error.status_code(), Self::OperationNotAllowed(_) => ServiceErrorCode::MethodNotAllowed, - Self::Internal(_) => ServiceErrorCode::Internal, + Self::SplitDeletion(_) => ServiceErrorCode::Internal, + Self::Storage(_) => ServiceErrorCode::Internal, } } } @@ -102,10 +103,10 @@ impl IndexService { if overwrite { match self.delete_index(&index_config.index_id, false).await { Ok(_) - | Err(IndexServiceError::MetastoreError(MetastoreError::IndexesDoNotExist { - index_ids: _, - })) => { - // Ignore IndexesDoNotExist error. + | Err(IndexServiceError::Metastore(MetastoreError::NotFound( + EntityKind::Index { .. }, + ))) => { + // Ignore index not found error. } Err(error) => { return Err(error); @@ -320,9 +321,9 @@ impl IndexService { .sources .get(source_id) .ok_or_else(|| { - IndexServiceError::MetastoreError(MetastoreError::SourceDoesNotExist { + IndexServiceError::Metastore(MetastoreError::NotFound(EntityKind::Source { source_id: source_id.to_string(), - }) + })) })? .clone(); @@ -380,11 +381,11 @@ mod tests { .create_index(index_config.clone(), false) .await .unwrap_err(); - let IndexServiceError::MetastoreError(inner_error) = error else { + let IndexServiceError::Metastore(inner_error) = error else { panic!("Expected `MetastoreError` variant, got {:?}", error) }; assert!( - matches!(inner_error, MetastoreError::IndexAlreadyExists { index_id } if index_id == index_metadata_0.index_id()) + matches!(inner_error, MetastoreError::AlreadyExists(EntityKind::Index { index_id }) if index_id == index_metadata_0.index_id()) ); let index_metadata_1 = index_service @@ -442,7 +443,7 @@ mod tests { .await .unwrap_err(); assert!( - matches!(error, MetastoreError::IndexesDoNotExist { index_ids } if index_ids == vec![index_uid.index_id().to_string()]) + matches!(error, MetastoreError::NotFound(EntityKind::Index { index_id }) if index_id == index_uid.index_id()) ); assert!(!storage.exists(split_path).await.unwrap()); } diff --git a/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs b/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs index 099e63670d8..a00218de0b4 100644 --- a/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs +++ b/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs @@ -30,8 +30,9 @@ use quickwit_common::temp_dir::TempDirectory; use quickwit_common::KillSwitch; use quickwit_config::{IndexingSettings, SourceConfig}; use quickwit_doc_mapper::DocMapper; -use quickwit_metastore::{Metastore, MetastoreError}; +use quickwit_metastore::Metastore; use quickwit_proto::indexing::IndexingPipelineId; +use quickwit_proto::metastore::MetastoreError; use quickwit_storage::Storage; use tokio::join; use tokio::sync::Semaphore; @@ -504,7 +505,7 @@ impl Handler for IndexingPipeline { } self.previous_generations_statistics.num_spawn_attempts = 1 + spawn.retry_count; if let Err(spawn_error) = self.spawn_pipeline(ctx).await { - if let Some(MetastoreError::IndexesDoNotExist { .. }) = + if let Some(MetastoreError::NotFound { .. }) = spawn_error.downcast_ref::() { info!(error = ?spawn_error, "Could not spawn pipeline, index might have been deleted."); @@ -550,7 +551,8 @@ mod tests { use quickwit_actors::{Command, Universe}; use quickwit_config::{IndexingSettings, SourceInputFormat, SourceParams, VoidSourceParams}; use quickwit_doc_mapper::{default_doc_mapper_for_test, DefaultDocMapper}; - use quickwit_metastore::{IndexMetadata, MetastoreError, MockMetastore}; + use quickwit_metastore::{IndexMetadata, MockMetastore}; + use quickwit_proto::metastore::MetastoreError; use quickwit_proto::IndexUid; use quickwit_storage::RamStorage; @@ -583,7 +585,7 @@ mod tests { return Ok(index_metadata); } num_fails -= 1; - Err(MetastoreError::ConnectionError { + Err(MetastoreError::Connection { message: "MetastoreError Alarm".to_string(), }) }); diff --git a/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs b/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs index d69aea54b20..0e5fcbf4fdd 100644 --- a/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs +++ b/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs @@ -30,8 +30,9 @@ use quickwit_common::io::IoControls; use quickwit_common::temp_dir::TempDirectory; use quickwit_common::KillSwitch; use quickwit_doc_mapper::DocMapper; -use quickwit_metastore::{ListSplitsQuery, Metastore, MetastoreError, SplitState}; +use quickwit_metastore::{ListSplitsQuery, Metastore, SplitState}; use quickwit_proto::indexing::IndexingPipelineId; +use quickwit_proto::metastore::MetastoreError; use time::OffsetDateTime; use tokio::join; use tracing::{debug, error, info, instrument}; @@ -428,7 +429,7 @@ impl Handler for MergePipeline { } self.previous_generations_statistics.num_spawn_attempts = 1 + spawn.retry_count; if let Err(spawn_error) = self.spawn_pipeline(ctx).await { - if let Some(MetastoreError::IndexesDoNotExist { .. }) = + if let Some(MetastoreError::NotFound { .. }) = spawn_error.downcast_ref::() { info!(error = ?spawn_error, "Could not spawn pipeline, index might have been deleted."); @@ -491,17 +492,14 @@ mod tests { .expect_list_splits() .times(1) .returning(move |list_split_query| { - assert_eq!(list_split_query.index_uids, vec![index_uid.clone()]); + assert_eq!(list_split_query.index_uids, &[index_uid.clone()]); assert_eq!( list_split_query.split_states, vec![quickwit_metastore::SplitState::Published] ); - match list_split_query.mature { - Bound::Excluded(_) => {} - _ => { - panic!("Expected excluded bound."); - } - } + let Bound::Excluded(_) = list_split_query.mature else { + panic!("Expected excluded bound."); + }; Ok(Vec::new()) }); let universe = Universe::with_accelerated_time(); diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs b/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs index 9ef4a0c230d..ecf98b8a6c1 100644 --- a/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs +++ b/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs @@ -376,7 +376,7 @@ mod tests { .returning(move |_: LeafSearchRequest| { if leaf_search_num_failures > 0 { leaf_search_num_failures -= 1; - return Err(SearchError::InternalError("leaf search error".to_string())); + return Err(SearchError::Internal("leaf search error".to_string())); } Ok(LeafSearchResponse { num_hits: 1, diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs b/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs index 55c9902e05e..8ab752204d9 100644 --- a/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs +++ b/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs @@ -29,10 +29,8 @@ use quickwit_common::uri::Uri; use quickwit_doc_mapper::tag_pruning::extract_tags_from_query; use quickwit_indexing::actors::MergeSplitDownloader; use quickwit_indexing::merge_policy::MergeOperation; -use quickwit_metastore::{ - split_tag_filter, split_time_range_filter, Metastore, MetastoreResult, Split, -}; -use quickwit_proto::metastore::DeleteTask; +use quickwit_metastore::{split_tag_filter, split_time_range_filter, Metastore, Split}; +use quickwit_proto::metastore::{DeleteTask, MetastoreResult}; use quickwit_proto::search::SearchRequest; use quickwit_proto::IndexUid; use quickwit_search::{jobs_to_leaf_requests, IndexMetasForLeafSearch, SearchJob, SearchJobPlacer}; diff --git a/quickwit/quickwit-janitor/src/actors/garbage_collector.rs b/quickwit/quickwit-janitor/src/actors/garbage_collector.rs index 9a115f34c92..1edfba84325 100644 --- a/quickwit/quickwit-janitor/src/actors/garbage_collector.rs +++ b/quickwit/quickwit-janitor/src/actors/garbage_collector.rs @@ -209,9 +209,9 @@ mod tests { use quickwit_actors::Universe; use quickwit_common::shared_consts::DELETION_GRACE_PERIOD; use quickwit_metastore::{ - IndexMetadata, ListSplitsQuery, MetastoreError, MockMetastore, Split, SplitMetadata, - SplitState, + IndexMetadata, ListSplitsQuery, MockMetastore, Split, SplitMetadata, SplitState, }; + use quickwit_proto::metastore::MetastoreError; use quickwit_storage::MockStorage; use time::OffsetDateTime; @@ -479,7 +479,7 @@ mod tests { .expect_list_indexes_metadatas() .times(4) .returning(move |_list_indexes_query: ListIndexesQuery| { - Err(MetastoreError::DbError { + Err(MetastoreError::Db { message: "Fail to list indexes.".to_string(), }) }); @@ -552,11 +552,10 @@ mod tests { assert!(["test-index-1", "test-index-2"].contains(&query.index_uids[0].index_id())); if query.index_uids[0].index_id() == "test-index-2" { - return Err(MetastoreError::DbError { + return Err(MetastoreError::Db { message: "fail to delete".to_string(), }); } - let splits = match query.split_states[0] { SplitState::Staged => make_splits(&["a"], SplitState::Staged), SplitState::MarkedForDeletion => { @@ -649,7 +648,7 @@ mod tests { // instead this should simply get logged and return the list of splits // which have successfully been deleted. if index_uid.index_id() == "test-index-2" { - Err(MetastoreError::DbError { + Err(MetastoreError::Db { message: "fail to delete".to_string(), }) } else { diff --git a/quickwit/quickwit-janitor/src/error.rs b/quickwit/quickwit-janitor/src/error.rs index c8a7e06c5ec..4b6f73cf638 100644 --- a/quickwit/quickwit-janitor/src/error.rs +++ b/quickwit/quickwit-janitor/src/error.rs @@ -17,7 +17,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use quickwit_metastore::MetastoreError; +use quickwit_proto::metastore::MetastoreError; use quickwit_proto::{ServiceError, ServiceErrorCode}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -28,18 +28,18 @@ use thiserror::Error; pub enum JanitorError { #[error("Invalid delete query: `{0}`.")] InvalidDeleteQuery(String), - #[error("Internal error: {0}")] - InternalError(String), - #[error("Metastore error `{0}`.")] - MetastoreError(#[from] MetastoreError), + #[error("Internal error: `{0}`")] + Internal(String), + #[error("Metastore error: `{0}`.")] + Metastore(#[from] MetastoreError), } impl ServiceError for JanitorError { fn status_code(&self) -> ServiceErrorCode { match self { JanitorError::InvalidDeleteQuery(_) => ServiceErrorCode::BadRequest, - JanitorError::InternalError(_) => ServiceErrorCode::Internal, - JanitorError::MetastoreError(error) => error.status_code(), + JanitorError::Internal(_) => ServiceErrorCode::Internal, + JanitorError::Metastore(error) => error.status_code(), } } } diff --git a/quickwit/quickwit-metastore/Cargo.toml b/quickwit/quickwit-metastore/Cargo.toml index 03d17e82722..0dd51313690 100644 --- a/quickwit/quickwit-metastore/Cargo.toml +++ b/quickwit/quickwit-metastore/Cargo.toml @@ -13,7 +13,6 @@ documentation = "https://quickwit.io/docs/" anyhow = { workspace = true } async-trait = { workspace = true } byte-unit = { workspace = true } -sqlx = { workspace = true, optional = true } futures = { workspace = true } http = { workspace = true } itertools = { workspace = true } @@ -24,6 +23,7 @@ regex = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } serde_with = { workspace = true } +sqlx = { workspace = true, optional = true } tempfile = { workspace = true, optional = true } thiserror = { workspace = true } time = { workspace = true } @@ -31,8 +31,8 @@ tokio = { workspace = true } tokio-stream = { workspace = true } tower = { workspace = true } tracing = { workspace = true } -utoipa = { workspace = true } ulid = { workspace = true, features = ["serde"] } +utoipa = { workspace = true } quickwit-common = { workspace = true } quickwit-config = { workspace = true } @@ -47,15 +47,16 @@ futures = { workspace = true } md5 = { workspace = true } mockall = { workspace = true } rand = { workspace = true } -tracing-subscriber = { workspace = true } tempfile = { workspace = true } +tracing-subscriber = { workspace = true } +quickwit-common = { workspace = true, features = ["testsuite"] } quickwit-config = { workspace = true, features = ["testsuite"] } quickwit-doc-mapper = { workspace = true, features = ["testsuite"] } quickwit-storage = { workspace = true, features = ["testsuite"] } [features] -testsuite = ["mockall", "tempfile", "quickwit-config/testsuite"] -ci-test = [] -postgres = ["sqlx"] azure = ["quickwit-storage/azure"] +ci-test = [] +postgres = ["quickwit-proto/postgres", "sqlx"] +testsuite = ["mockall", "tempfile", "quickwit-config/testsuite"] diff --git a/quickwit/quickwit-metastore/src/checkpoint.rs b/quickwit/quickwit-metastore/src/checkpoint.rs index cdd67be4785..0baabcb9d7c 100644 --- a/quickwit/quickwit-metastore/src/checkpoint.rs +++ b/quickwit/quickwit-metastore/src/checkpoint.rs @@ -34,6 +34,12 @@ use tracing::{info, warn}; #[derive(Clone, Debug, Default, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize)] pub struct PartitionId(pub Arc); +impl fmt::Display for PartitionId { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.0) + } +} + impl From for PartitionId { fn from(partition_id_str: String) -> Self { PartitionId(Arc::new(partition_id_str)) @@ -269,11 +275,11 @@ impl<'de> Deserialize<'de> for SourceCheckpoint { /// the checkpoint. #[derive(Clone, Debug, Error, Eq, PartialEq, Serialize, Deserialize)] #[error( - "IncompatibleChkptDelta at partition: {partition_id:?} cur_pos:{current_position:?} \ + "Incompatible checkpoint delta at partition `{partition_id}`: cur_pos:{current_position:?} \ delta_pos:{delta_position_from:?}" )] pub struct IncompatibleCheckpointDelta { - /// One PartitionId for which the incompatibility has been detected. + /// The partition ID for which the incompatibility has been detected. pub partition_id: PartitionId, /// The current position within this partition. pub current_position: Position, @@ -286,7 +292,8 @@ pub enum PartitionDeltaError { #[error(transparent)] IncompatibleCheckpointDelta(#[from] IncompatibleCheckpointDelta), #[error( - "EmptyOrNegativeDelta at partition: {partition_id:?} {from_position:?} >= {to_position:?}" + "Empty or negative delta at partition `{partition_id}`: {from_position:?} >= \ + {to_position:?}" )] EmptyOrNegativeDelta { /// One PartitionId for which the negative delta has been detected. diff --git a/quickwit/quickwit-metastore/src/error.rs b/quickwit/quickwit-metastore/src/error.rs index 2de40b743fb..2e0fdc2b1b2 100644 --- a/quickwit/quickwit-metastore/src/error.rs +++ b/quickwit/quickwit-metastore/src/error.rs @@ -17,123 +17,10 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use quickwit_proto::{ServiceError, ServiceErrorCode}; -use serde::{Deserialize, Serialize}; -use thiserror::Error as ThisError; +use quickwit_proto::metastore::MetastoreError; -use crate::checkpoint::IncompatibleCheckpointDelta; - -/// Metastore error kinds. -#[allow(missing_docs)] -#[derive(Clone, Debug, ThisError, Serialize, Deserialize, PartialEq, Eq)] -pub enum MetastoreError { - #[error("Connection error: `{message}`.")] - ConnectionError { message: String }, - - #[error("Index `{index_id}` already exists.")] - IndexAlreadyExists { index_id: String }, - - #[error("Access forbidden: `{message}`.")] - Forbidden { message: String }, - - #[error("Indexes `{index_ids:?}` do not exist.")] - IndexesDoNotExist { index_ids: Vec }, - - /// Any generic internal error. - /// The message can be helpful to users, but the detail of the error - /// are judged uncoverable and not useful for error handling. - #[error("Internal error: `{message}` Cause: `{cause}`.")] - InternalError { message: String, cause: String }, - - #[error("Failed to deserialize index metadata: `{message}`")] - InvalidManifest { message: String }, - - #[error("IOError `{message}`")] - Io { message: String }, - - #[error("Splits `{split_ids:?}` do not exist.")] - SplitsDoNotExist { split_ids: Vec }, - - #[error("Splits `{split_ids:?}` are not in a deletable state.")] - SplitsNotDeletable { split_ids: Vec }, - - #[error("Splits `{split_ids:?}` are not staged.")] - SplitsNotStaged { split_ids: Vec }, - - #[error("Publish checkpoint delta overlaps with the current checkpoint: {0:?}.")] - IncompatibleCheckpointDelta(#[from] IncompatibleCheckpointDelta), - - #[error("Source `{source_id}` of type `{source_type}` already exists.")] - SourceAlreadyExists { - source_id: String, - source_type: String, - }, - - #[error("Source `{source_id}` does not exist.")] - SourceDoesNotExist { source_id: String }, - - #[error("Database error: `{message}`.")] - DbError { message: String }, - - #[error("Failed to deserialize `{struct_name}` from JSON: `{message}`.")] - JsonDeserializeError { - struct_name: String, - message: String, - }, - - #[error("Failed to serialize `{struct_name}` to JSON: `{message}`.")] - JsonSerializeError { - struct_name: String, - message: String, - }, -} - -#[cfg(feature = "postgres")] -impl From for MetastoreError { - fn from(error: sqlx::Error) -> Self { - MetastoreError::DbError { - message: error.to_string(), - } - } -} - -impl From for quickwit_proto::tonic::Status { - fn from(metastore_error: MetastoreError) -> Self { - let grpc_code = metastore_error.status_code().to_grpc_status_code(); - let error_msg = serde_json::to_string(&metastore_error) - .unwrap_or_else(|_| format!("Raw metastore error: {metastore_error}")); - quickwit_proto::tonic::Status::new(grpc_code, error_msg) - } -} - -impl ServiceError for MetastoreError { - fn status_code(&self) -> ServiceErrorCode { - match self { - Self::ConnectionError { .. } => ServiceErrorCode::Internal, - Self::Forbidden { .. } => ServiceErrorCode::MethodNotAllowed, - Self::IncompatibleCheckpointDelta(_) => ServiceErrorCode::BadRequest, - Self::IndexAlreadyExists { .. } => ServiceErrorCode::BadRequest, - Self::IndexesDoNotExist { .. } => ServiceErrorCode::NotFound, - Self::InternalError { .. } => ServiceErrorCode::Internal, - Self::InvalidManifest { .. } => ServiceErrorCode::Internal, - Self::Io { .. } => ServiceErrorCode::Internal, - Self::SourceAlreadyExists { .. } => ServiceErrorCode::BadRequest, - Self::SourceDoesNotExist { .. } => ServiceErrorCode::NotFound, - Self::SplitsDoNotExist { .. } => ServiceErrorCode::BadRequest, - Self::SplitsNotDeletable { .. } => ServiceErrorCode::BadRequest, - Self::SplitsNotStaged { .. } => ServiceErrorCode::BadRequest, - Self::DbError { .. } => ServiceErrorCode::Internal, - Self::JsonDeserializeError { .. } => ServiceErrorCode::Internal, - Self::JsonSerializeError { .. } => ServiceErrorCode::Internal, - } - } -} - -/// Generic Result type for metastore operations. -pub type MetastoreResult = Result; - -/// Generic Storage Resolver Error. -#[derive(Debug, ThisError)] +/// Generic Storage Resolver error. +#[derive(Debug, thiserror::Error)] pub enum MetastoreResolverError { /// The metastore config is invalid. #[error("Invalid metastore config: `{0}`")] @@ -151,5 +38,5 @@ pub enum MetastoreResolverError { /// resolver failed to actually connect to the backend. e.g. connection error, credentials /// error, incompatible version, internal error in a third party, etc. #[error("Failed to connect to metastore: `{0}`")] - FailedToOpenMetastore(MetastoreError), + Initialization(#[from] MetastoreError), } diff --git a/quickwit/quickwit-metastore/src/lib.rs b/quickwit/quickwit-metastore/src/lib.rs index f8c8fe0ef27..d1973c63448 100644 --- a/quickwit/quickwit-metastore/src/lib.rs +++ b/quickwit/quickwit-metastore/src/lib.rs @@ -22,9 +22,10 @@ #![deny(clippy::disallowed_methods)] #![allow(rustdoc::invalid_html_tags)] -//! `quickwit-metastore` is the abstraction used in quickwit to interface itself to different +//! `quickwit-metastore` is the abstraction used in Quickwit to interface itself to different //! metastore: //! - file-backed metastore +//! - PostgreSQL metastore //! etc. #[macro_use] @@ -41,7 +42,7 @@ mod split_metadata_version; use std::ops::Range; -pub use error::{MetastoreError, MetastoreResolverError, MetastoreResult}; +pub use error::MetastoreResolverError; pub use metastore::file_backed_metastore::FileBackedMetastore; pub use metastore::grpc_metastore::{GrpcMetastoreAdapter, MetastoreGrpcClient}; pub(crate) use metastore::index_metadata::serialize::{IndexMetadataV0_6, VersionedIndexMetadata}; diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/mod.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/mod.rs index 58ba02ddc22..30c22681ae8 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_index/mod.rs @@ -29,7 +29,9 @@ use std::ops::Bound; use quickwit_common::PrettySample; use quickwit_config::SourceConfig; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{ + DeleteQuery, DeleteTask, EntityKind, MetastoreError, MetastoreResult, +}; use quickwit_proto::IndexUid; use serde::{Deserialize, Serialize}; use serialize::VersionedFileBackedIndex; @@ -37,10 +39,7 @@ use time::OffsetDateTime; use tracing::{info, warn}; use crate::checkpoint::IndexCheckpointDelta; -use crate::{ - split_tag_filter, IndexMetadata, ListSplitsQuery, MetastoreError, MetastoreResult, Split, - SplitMetadata, SplitState, -}; +use crate::{split_tag_filter, IndexMetadata, ListSplitsQuery, Split, SplitMetadata, SplitState}; /// A `FileBackedIndex` object carries an index metadata and its split metadata. // This struct is meant to be used only within the [`FileBackedMetastore`]. The public visibility is @@ -177,7 +176,7 @@ impl FileBackedIndex { /// it is simply updated/overwritten. /// /// If a split already exists and is *not* in the [SplitState::Staged] state, a - /// [MetastoreError::SplitsNotStaged] error is returned providing the split ID to go with + /// [MetastoreError::NotFound] error is returned providing the split ID to go with /// it. pub(crate) fn stage_split( &mut self, @@ -186,24 +185,23 @@ impl FileBackedIndex { // Check whether the split exists. // If the split exists, we check what state it is in. If it's anything other than `Staged` // something has gone very wrong and we should abort the operation. - if let Some(existing_split) = self.splits.get(split_metadata.split_id()) { - if existing_split.split_state != SplitState::Staged { - return Err(MetastoreError::SplitsNotStaged { - split_ids: vec![existing_split.split_id().to_string()], - }); + if let Some(split) = self.splits.get(split_metadata.split_id()) { + if split.split_state != SplitState::Staged { + let entity = EntityKind::Split { + split_id: split.split_id().to_string(), + }; + let message = "split is not staged".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } } - let now_timestamp = OffsetDateTime::now_utc().unix_timestamp(); - let metadata = Split { + let split = Split { split_state: SplitState::Staged, update_timestamp: now_timestamp, publish_timestamp: None, split_metadata, }; - - self.splits - .insert(metadata.split_id().to_string(), metadata); + self.splits.insert(split.split_id().to_string(), split); Ok(()) } @@ -243,9 +241,9 @@ impl FileBackedIndex { } if !split_not_found_ids.is_empty() { if return_error_on_splits_not_found { - return Err(MetastoreError::SplitsDoNotExist { + return Err(MetastoreError::NotFound(EntityKind::Splits { split_ids: split_not_found_ids, - }); + })); } else { warn!( index_id=%self.index_id(), @@ -256,9 +254,11 @@ impl FileBackedIndex { } } if !non_deletable_split_ids.is_empty() { - return Err(MetastoreError::SplitsNotDeletable { + let entity = EntityKind::Splits { split_ids: non_deletable_split_ids, - }); + }; + let message = "splits are not deletable".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } Ok(mutation_occurred) } @@ -285,15 +285,17 @@ impl FileBackedIndex { } if !split_not_found_ids.is_empty() { - return Err(MetastoreError::SplitsDoNotExist { + return Err(MetastoreError::NotFound(EntityKind::Splits { split_ids: split_not_found_ids, - }); + })); } if !split_not_staged_ids.is_empty() { - return Err(MetastoreError::SplitsNotStaged { + let entity = EntityKind::Splits { split_ids: split_not_staged_ids, - }); + }; + let message = "splits are not staged".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } Ok(()) @@ -307,7 +309,18 @@ impl FileBackedIndex { checkpoint_delta_opt: Option, ) -> MetastoreResult<()> { if let Some(checkpoint_delta) = checkpoint_delta_opt { - self.metadata.checkpoint.try_apply_delta(checkpoint_delta)?; + let source_id = checkpoint_delta.source_id.clone(); + self.metadata + .checkpoint + .try_apply_delta(checkpoint_delta) + .map_err(|error| { + let entity = EntityKind::CheckpointDelta { + index_id: self.index_id().to_string(), + source_id, + }; + let message = error.to_string(); + MetastoreError::FailedPrecondition { entity, message } + })?; } self.mark_splits_as_published_helper(split_ids)?; self.mark_splits_for_deletion(replaced_split_ids, &[SplitState::Published], true)?; @@ -360,9 +373,11 @@ impl FileBackedIndex { } } if !split_not_deletable_ids.is_empty() { - return Err(MetastoreError::SplitsNotDeletable { + let entity = EntityKind::Splits { split_ids: split_not_deletable_ids, - }); + }; + let message = "splits are not deletable".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } info!(index_id=%self.index_id(), "Deleted {} splits from index.", split_ids.len()); @@ -427,12 +442,11 @@ impl FileBackedIndex { delete_opstamp: u64, ) -> MetastoreResult { for split_id in split_ids { - let split = - self.splits - .get_mut(*split_id) - .ok_or_else(|| MetastoreError::SplitsDoNotExist { - split_ids: vec![split_id.to_string()], - })?; + let split = self.splits.get_mut(*split_id).ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Splits { + split_ids: vec![split_id.to_string()], + }) + })?; split.split_metadata.delete_opstamp = delete_opstamp; } Ok(true) diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_metastore_factory.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_metastore_factory.rs index fc1642c1902..accdf832d24 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_metastore_factory.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/file_backed_metastore_factory.rs @@ -25,15 +25,14 @@ use async_trait::async_trait; use once_cell::sync::OnceCell; use quickwit_common::uri::Uri; use quickwit_config::{MetastoreBackend, MetastoreConfig}; +use quickwit_proto::metastore::MetastoreError; use quickwit_storage::{StorageResolver, StorageResolverError}; use regex::Regex; use tokio::sync::Mutex; use tracing::debug; use crate::metastore::instrumented_metastore::InstrumentedMetastore; -use crate::{ - FileBackedMetastore, Metastore, MetastoreError, MetastoreFactory, MetastoreResolverError, -}; +use crate::{FileBackedMetastore, Metastore, MetastoreFactory, MetastoreResolverError}; /// A file-backed metastore factory. /// @@ -134,7 +133,7 @@ impl MetastoreFactory for FileBackedMetastoreFactory { MetastoreResolverError::UnsupportedBackend(message) } StorageResolverError::FailedToOpenStorage { kind, message } => { - MetastoreResolverError::FailedToOpenMetastore(MetastoreError::InternalError { + MetastoreResolverError::Initialization(MetastoreError::Internal { message: format!("Failed to open metastore file `{uri}`."), cause: format!("StorageError {kind:?}: {message}."), }) @@ -142,7 +141,7 @@ impl MetastoreFactory for FileBackedMetastoreFactory { })?; let file_backed_metastore = FileBackedMetastore::try_new(storage, polling_interval_opt) .await - .map_err(MetastoreResolverError::FailedToOpenMetastore)?; + .map_err(MetastoreResolverError::Initialization)?; let instrumented_metastore = InstrumentedMetastore::new(Box::new(file_backed_metastore)); let unique_metastore_for_uri = self .cache_metastore(uri, Arc::new(instrumented_metastore)) diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/lazy_file_backed_index.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/lazy_file_backed_index.rs index 56d7325154a..7cc110ef477 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/lazy_file_backed_index.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/lazy_file_backed_index.rs @@ -20,13 +20,13 @@ use std::sync::{Arc, Weak}; use std::time::Duration; +use quickwit_proto::metastore::MetastoreResult; use quickwit_storage::Storage; use tokio::sync::{Mutex, OnceCell}; use tracing::error; use super::file_backed_index::FileBackedIndex; use super::store_operations::fetch_index; -use crate::MetastoreResult; /// Lazy [`FileBackedIndex`]. It loads a `FileBackedIndex` /// on demand and optionally spawns a task to poll diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs index a6c07090bab..41593845de6 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/mod.rs @@ -35,7 +35,9 @@ use futures::future::try_join_all; use itertools::Itertools; use quickwit_common::uri::Uri; use quickwit_config::{validate_index_id_pattern, IndexConfig, SourceConfig}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{ + DeleteQuery, DeleteTask, EntityKind, MetastoreError, MetastoreResult, +}; use quickwit_proto::IndexUid; use quickwit_storage::Storage; use regex::RegexSet; @@ -48,11 +50,9 @@ use self::store_operations::{ check_indexes_states_exist, delete_index, fetch_index, fetch_or_init_indexes_states, index_exists, put_index, put_indexes_states, }; -use super::ListIndexesQuery; use crate::checkpoint::IndexCheckpointDelta; use crate::{ - IndexMetadata, ListSplitsQuery, Metastore, MetastoreError, MetastoreResult, Split, - SplitMetadata, SplitState, + IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMetadata, SplitState, }; /// State of an index tracked by the metastore. @@ -161,14 +161,14 @@ impl FileBackedMetastore { async fn mutate( &self, index_uid: IndexUid, - mutate_fn: impl FnOnce(&mut FileBackedIndex) -> crate::MetastoreResult>, + mutate_fn: impl FnOnce(&mut FileBackedIndex) -> MetastoreResult>, ) -> MetastoreResult { let index_id = index_uid.index_id(); let mut locked_index = self.get_locked_index(index_id).await?; if locked_index.index_uid() != index_uid { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })); } let mut index = locked_index.clone(); let value = match mutate_fn(&mut index)? { @@ -215,9 +215,9 @@ impl FileBackedMetastore { if locked_index.index_uid() == index_uid { view(&locked_index) } else { - Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }) + Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })) } } @@ -324,12 +324,12 @@ impl Metastore for FileBackedMetastore { // don't want to override an existing metadata file. if let Some(index_state) = per_index_metastores_wlock.get(&index_id) { if let IndexState::Alive(_) = index_state { - return Err(MetastoreError::IndexAlreadyExists { + return Err(MetastoreError::AlreadyExists(EntityKind::Index { index_id: index_id.clone(), - }); + })); } } else if index_exists(&*self.storage, &index_id).await? { - return Err(MetastoreError::InternalError { + return Err(MetastoreError::Internal { message: format!("Index {index_id} cannot be created."), cause: format!( "Index {index_id} is not present in the `indexes_states.json` file but its \ @@ -380,9 +380,9 @@ impl Metastore for FileBackedMetastore { if !per_index_metastores_wlock.contains_key(index_id) && !index_exists(&*self.storage, index_id).await? { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })); } // Set state to `Deleting` and keep the previous state in memory in case we need to insert @@ -405,7 +405,7 @@ impl Metastore for FileBackedMetastore { Ok(()) | // If the index file does not exist, we still need to return an error, // but it makes sense to ensure that the index state is removed. - Err(MetastoreError::IndexesDoNotExist { .. }) => { + Err(MetastoreError::NotFound(EntityKind::Index { .. })) => { per_index_metastores_wlock.remove(index_id); if let Err(error) = put_indexes_states(&*self.storage, &per_index_metastores_wlock).await { per_index_metastores_wlock.insert(index_id.to_string(), IndexState::Deleting); @@ -431,16 +431,21 @@ impl Metastore for FileBackedMetastore { for split_metadata in split_metadata_list { match index.stage_split(split_metadata) { Ok(()) => {} - Err(MetastoreError::SplitsNotStaged { split_ids }) => { - failed_split_ids.extend(split_ids); + Err(MetastoreError::FailedPrecondition { + entity: EntityKind::Split { split_id }, + .. + }) => { + failed_split_ids.push(split_id); } Err(error) => return Err(error), }; } if !failed_split_ids.is_empty() { - Err(MetastoreError::SplitsNotStaged { + let entity = EntityKind::Splits { split_ids: failed_split_ids, - }) + }; + let message = "splits are not staged".to_string(); + Err(MetastoreError::FailedPrecondition { entity, message }) } else { Ok(MutationOccurred::Yes(())) } @@ -549,14 +554,15 @@ impl Metastore for FileBackedMetastore { /// Read-only accessors async fn list_splits(&self, query: ListSplitsQuery) -> MetastoreResult> { - let mut splits = Vec::new(); - for index_uid in query.index_uids.iter() { - let index_splits = self + let mut all_splits = Vec::new(); + + for index_uid in &query.index_uids { + let splits = self .read(index_uid.clone(), |index| index.list_splits(&query)) .await?; - splits.extend(index_splits); + all_splits.extend(splits); } - Ok(splits) + Ok(all_splits) } async fn index_metadata(&self, index_id: &str) -> MetastoreResult { @@ -577,17 +583,16 @@ impl Metastore for FileBackedMetastore { ListIndexesQuery::IndexIdPatterns(patterns) => build_regex_set_from_patterns(patterns), ListIndexesQuery::All => build_regex_set_from_patterns(vec!["*".to_string()]), }; - let index_matcher = - index_matcher_result.map_err(|error| MetastoreError::InternalError { - message: "Failed to build RegexSet from index patterns`".to_string(), - cause: error.to_string(), - })?; + let index_matcher = index_matcher_result.map_err(|error| MetastoreError::Internal { + message: "Failed to build RegexSet from index patterns`".to_string(), + cause: error.to_string(), + })?; let index_ids: Vec = { let per_index_metastores_rlock = self.per_index_metastores.read().await; per_index_metastores_rlock .iter() - .flat_map(|(index_id, index_state)| match index_state { + .filter_map(|(index_id, index_state)| match index_state { IndexState::Alive(_) => Some(index_id), _ => None, }) @@ -599,14 +604,14 @@ impl Metastore for FileBackedMetastore { try_join_all(index_ids.iter().map(|index_id| async move { match self.index_metadata(index_id).await { Ok(index_metadata) => Ok(Some(index_metadata)), - Err(MetastoreError::IndexesDoNotExist { index_ids: _ }) => Ok(None), - Err(MetastoreError::InternalError { message, cause }) => { + Err(MetastoreError::NotFound(EntityKind::Index { .. })) => Ok(None), + Err(MetastoreError::Internal { message, cause }) => { // Indexes can be in a transition state `Creating` or `Deleting`. // This is fine to ignore them. if cause.contains("is in transitioning state") { Ok(None) } else { - Err(MetastoreError::InternalError { message, cause }) + Err(MetastoreError::Internal { message, cause }) } } Err(error) => Err(error), @@ -683,13 +688,13 @@ async fn get_index_mutex( ) -> MetastoreResult>> { match index_state { IndexState::Alive(lazy_index) => lazy_index.get().await, - IndexState::Creating => Err(MetastoreError::InternalError { + IndexState::Creating => Err(MetastoreError::Internal { message: format!("Index `{index_id}` cannot be retrieved."), cause: "Index `{index_id}` is in transitioning state `Creating` and this should not \ happened. Either recreate or delete it." .to_string(), }), - IndexState::Deleting => Err(MetastoreError::InternalError { + IndexState::Deleting => Err(MetastoreError::Internal { message: format!("Index `{index_id}` cannot be retrieved."), cause: "Index `{index_id}` is in transitioning state `Deleting` and this should not \ happened. Try to delete it again." @@ -748,7 +753,7 @@ mod tests { use futures::executor::block_on; use quickwit_config::IndexConfig; - use quickwit_proto::metastore::DeleteQuery; + use quickwit_proto::metastore::{DeleteQuery, MetastoreError}; use quickwit_query::query_ast::qast_json_helper; use quickwit_storage::{MockStorage, RamStorage, Storage, StorageErrorKind}; use rand::Rng; @@ -761,9 +766,7 @@ mod tests { }; use super::*; use crate::tests::test_suite::DefaultForTest; - use crate::{ - IndexMetadata, ListSplitsQuery, Metastore, MetastoreError, SplitMetadata, SplitState, - }; + use crate::{IndexMetadata, ListSplitsQuery, Metastore, SplitMetadata, SplitState}; #[tokio::test] async fn test_file_backed_metastore_connectivity_fails_if_states_file_does_not_exist() { @@ -829,20 +832,14 @@ mod tests { .get_index(IndexUid::new("index-does-not-exist")) .await .unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::IndexesDoNotExist { .. } - )); + assert!(matches!(metastore_error, MetastoreError::NotFound { .. })); // Open a index with a different incarnation_id. let metastore_error = metastore .get_index(IndexUid::new(index_id)) .await .unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::IndexesDoNotExist { .. } - )); + assert!(matches!(metastore_error, MetastoreError::NotFound { .. })); } #[tokio::test] @@ -921,7 +918,7 @@ mod tests { #[tokio::test] async fn test_file_backed_metastore_get_index_checks_for_inconsistent_index_id( - ) -> crate::MetastoreResult<()> { + ) -> MetastoreResult<()> { let storage = Arc::new(RamStorage::default()); let index_id = "test-index"; let index_metadata = @@ -953,16 +950,13 @@ mod tests { .get_index(IndexUid::new(index_id)) .await .unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); Ok(()) } #[tokio::test] - async fn test_file_backed_metastore_wrt_directly_visible() -> crate::MetastoreResult<()> { + async fn test_file_backed_metastore_wrt_directly_visible() -> MetastoreResult<()> { let metastore = FileBackedMetastore::default_for_test().await; let index_config = IndexConfig::for_test("test-index", "ram:///indexes/test-index"); let index_uid = metastore.create_index(index_config).await?; @@ -991,7 +985,8 @@ mod tests { } #[tokio::test] - async fn test_file_backed_metastore_polling() -> crate::MetastoreResult<()> { + async fn test_file_backed_metastore_polling() -> quickwit_proto::metastore::MetastoreResult<()> + { let storage = Arc::new(RamStorage::default()); let metastore_wrt = FileBackedMetastore::try_new(storage.clone(), None) @@ -1165,10 +1160,7 @@ mod tests { // Create index. let metastore_error = metastore.create_index(index_config).await.unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); // Try fetch the not created index. let created_index_error = metastore .get_index(IndexUid::new(index_id)) @@ -1176,7 +1168,7 @@ mod tests { .unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound { .. } )); } @@ -1214,16 +1206,13 @@ mod tests { // Create index let metastore_error = metastore.create_index(index_config).await.unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); // Let's fetch the index, we expect an internal error as the index state is in `Creating` // state. let created_index_error = metastore.get_index(index_uid.clone()).await.unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::InternalError { .. } + MetastoreError::Internal { .. } )); // Check index state is in `Creating` in the states file. let index_states = @@ -1238,7 +1227,7 @@ mod tests { let deleted_index_error = metastore.delete_index(index_uid.clone()).await.unwrap_err(); assert!(matches!( deleted_index_error, - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound { .. } )); let index_states = fetch_or_init_indexes_states(Arc::new(ram_storage_clone_2), None) .await @@ -1248,7 +1237,7 @@ mod tests { let created_index_error = metastore.get_index(index_uid).await.unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound { .. } )); } @@ -1284,10 +1273,7 @@ mod tests { // Create index let metastore_error = metastore.create_index(index_config).await.unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); // Let's fetch the index, we expect an internal error as the index state is in `Creating` // state. let created_index_error = metastore @@ -1296,7 +1282,7 @@ mod tests { .unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::InternalError { .. } + MetastoreError::Internal { .. } )); } @@ -1333,16 +1319,13 @@ mod tests { // Delete index let metastore_error = metastore.delete_index(index_uid.clone()).await.unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); // Let's fetch the index, we expect an internal error as the index state is in `Deleting` // state. let created_index_error = metastore.get_index(index_uid).await.unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::InternalError { .. } + MetastoreError::Internal { .. } )); } @@ -1385,21 +1368,18 @@ mod tests { // Delete index let metastore_error = metastore.delete_index(index_uid.clone()).await.unwrap_err(); - assert!(matches!( - metastore_error, - MetastoreError::InternalError { .. } - )); + assert!(matches!(metastore_error, MetastoreError::Internal { .. })); // Let's fetch the index, we expect an internal error as the index state is in `Deleting` // state. let created_index_error = metastore.get_index(index_uid).await.unwrap_err(); assert!(matches!( created_index_error, - MetastoreError::InternalError { .. } + MetastoreError::Internal { .. } )); } #[tokio::test] - async fn test_file_backed_metastore_get_list_indexes() -> crate::MetastoreResult<()> { + async fn test_file_backed_metastore_get_list_indexes() -> MetastoreResult<()> { let index_id_creating = "test-index--creating"; let index_id_alive = "testing-index--alive"; let index_id_unregistered = "test-index--unregistered"; diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/store_operations.rs b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/store_operations.rs index c67ebb327c6..ac73ada2ceb 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/store_operations.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed_metastore/store_operations.rs @@ -22,12 +22,12 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; +use quickwit_proto::metastore::{EntityKind, MetastoreError, MetastoreResult}; use quickwit_storage::{Storage, StorageError, StorageErrorKind}; use serde::{Deserialize, Serialize}; use super::{IndexState, LazyFileBackedIndex}; use crate::metastore::file_backed_metastore::file_backed_index::FileBackedIndex; -use crate::{MetastoreError, MetastoreResult}; /// Indexes states file managed by [`FileBackedMetastore`](crate::FileBackedMetastore). const INDEXES_STATES_FILENAME: &str = "indexes_states.json"; @@ -60,13 +60,13 @@ pub(crate) fn meta_path(index_id: &str) -> PathBuf { fn convert_error(index_id: &str, storage_err: StorageError) -> MetastoreError { match storage_err.kind() { - StorageErrorKind::NotFound => MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }, + StorageErrorKind::NotFound => MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + }), StorageErrorKind::Unauthorized => MetastoreError::Forbidden { message: "The request credentials do not allow for this operation.".to_string(), }, - _ => MetastoreError::InternalError { + _ => MetastoreError::Internal { message: "Failed to get index files.".to_string(), cause: storage_err.to_string(), }, @@ -99,14 +99,15 @@ pub(crate) async fn fetch_or_init_indexes_states( let content = storage .get_all(indexes_list_path) .await - .map_err(|storage_err| MetastoreError::InternalError { + .map_err(|storage_err| MetastoreError::Internal { message: format!("Failed to get `{INDEXES_STATES_FILENAME}` file."), cause: storage_err.to_string(), })?; let indexes_states_deserialized: HashMap = - serde_json::from_slice(&content[..]).map_err(|serde_err| { - MetastoreError::InvalidManifest { - message: serde_err.to_string(), + serde_json::from_slice(&content[..]).map_err(|error| { + MetastoreError::JsonDeserializeError { + struct_name: "IndexManifest".to_string(), + message: error.to_string(), } })?; Ok(indexes_states_deserialized @@ -138,7 +139,7 @@ pub(crate) async fn put_indexes_states( let indexes_list_path = Path::new(INDEXES_STATES_FILENAME); let content: Vec = serde_json::to_vec_pretty(&indexes_states_serializable).map_err(|serde_err| { - MetastoreError::InternalError { + MetastoreError::Internal { message: "Failed to serialize indexes map".to_string(), cause: serde_err.to_string(), } @@ -146,7 +147,7 @@ pub(crate) async fn put_indexes_states( storage .put(indexes_list_path, Box::new(content)) .await - .map_err(|storage_err| MetastoreError::InternalError { + .map_err(|storage_err| MetastoreError::Internal { message: format!("Failed to put `{INDEXES_STATES_FILENAME}` file."), cause: storage_err.to_string(), })?; @@ -163,14 +164,15 @@ pub(crate) async fn fetch_index( .await .map_err(|storage_err| convert_error(index_id, storage_err))?; - let index: FileBackedIndex = serde_json::from_slice(&content[..]).map_err(|serde_err| { - MetastoreError::InvalidManifest { - message: serde_err.to_string(), + let index: FileBackedIndex = serde_json::from_slice(&content[..]).map_err(|serde_error| { + MetastoreError::JsonDeserializeError { + struct_name: "FileBackedIndex".to_string(), + message: serde_error.to_string(), } })?; if index.index_id() != index_id { - return Err(MetastoreError::InternalError { + return Err(MetastoreError::Internal { message: "Inconsistent manifest: index_id mismatch.".to_string(), cause: format!( "Expected index_id `{}`, but found `{}`", @@ -202,7 +204,7 @@ pub(crate) async fn put_index_given_index_id( ) -> MetastoreResult<()> { // Serialize Index. let content: Vec = - serde_json::to_vec_pretty(&index).map_err(|serde_err| MetastoreError::InternalError { + serde_json::to_vec_pretty(&index).map_err(|serde_err| MetastoreError::Internal { message: "Failed to serialize Metadata set".to_string(), cause: serde_err.to_string(), })?; @@ -234,11 +236,10 @@ pub(crate) async fn delete_index(storage: &dyn Storage, index_id: &str) -> Metas .map_err(|storage_err| convert_error(index_id, storage_err))?; if !file_exists { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })); } - // Put data back into storage. storage .delete(&metadata_path) @@ -247,7 +248,7 @@ pub(crate) async fn delete_index(storage: &dyn Storage, index_id: &str) -> Metas StorageErrorKind::Unauthorized => MetastoreError::Forbidden { message: "The request credentials do not allow for this operation.".to_string(), }, - _ => MetastoreError::InternalError { + _ => MetastoreError::Internal { message: format!( "Failed to write metastore file to `{}`.", metadata_path.display() diff --git a/quickwit/quickwit-metastore/src/metastore/grpc_metastore/grpc_adapter.rs b/quickwit/quickwit-metastore/src/metastore/grpc_metastore/grpc_adapter.rs index d5c6ddfd25f..8db034fbe01 100644 --- a/quickwit/quickwit-metastore/src/metastore/grpc_metastore/grpc_adapter.rs +++ b/quickwit/quickwit-metastore/src/metastore/grpc_metastore/grpc_adapter.rs @@ -29,15 +29,15 @@ use quickwit_proto::metastore::{ LastDeleteOpstampResponse, ListAllSplitsRequest, ListDeleteTasksRequest, ListDeleteTasksResponse, ListIndexesMetadatasRequest, ListIndexesMetadatasResponse, ListSplitsRequest, ListSplitsResponse, ListStaleSplitsRequest, MarkSplitsForDeletionRequest, - MetastoreService, PublishSplitsRequest, ResetSourceCheckpointRequest, SourceResponse, - SplitResponse, StageSplitsRequest, ToggleSourceRequest, UpdateSplitsDeleteOpstampRequest, - UpdateSplitsDeleteOpstampResponse, + MetastoreError, MetastoreService, PublishSplitsRequest, ResetSourceCheckpointRequest, + SourceResponse, SplitResponse, StageSplitsRequest, ToggleSourceRequest, + UpdateSplitsDeleteOpstampRequest, UpdateSplitsDeleteOpstampResponse, }; use quickwit_proto::tonic::{Request, Response, Status}; use quickwit_proto::{set_parent_span_from_request_metadata, tonic}; use tracing::instrument; -use crate::{ListSplitsQuery, Metastore, MetastoreError}; +use crate::{ListSplitsQuery, Metastore}; #[allow(missing_docs)] #[derive(Clone)] @@ -102,7 +102,7 @@ impl MetastoreService for GrpcMetastoreAdapter { request: tonic::Request, ) -> Result, tonic::Status> { set_parent_span_from_request_metadata(request.metadata()); - let query = serde_json::from_str(&request.into_inner().filter_json).map_err(|error| { + let query = serde_json::from_str(&request.into_inner().query_json).map_err(|error| { MetastoreError::JsonSerializeError { struct_name: "ListIndexesQuery".to_string(), message: error.to_string(), @@ -166,7 +166,7 @@ impl MetastoreService for GrpcMetastoreAdapter { ) -> Result, tonic::Status> { set_parent_span_from_request_metadata(request.metadata()); let list_splits_request = request.into_inner(); - let query: ListSplitsQuery = serde_json::from_str(&list_splits_request.filter_json) + let query: ListSplitsQuery = serde_json::from_str(&list_splits_request.query_json) .map_err(|error| MetastoreError::JsonDeserializeError { struct_name: "ListSplitsQuery".to_string(), message: error.to_string(), diff --git a/quickwit/quickwit-metastore/src/metastore/grpc_metastore/mod.rs b/quickwit/quickwit-metastore/src/metastore/grpc_metastore/mod.rs index 8becceb0554..6b790595f7f 100644 --- a/quickwit/quickwit-metastore/src/metastore/grpc_metastore/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/grpc_metastore/mod.rs @@ -33,9 +33,9 @@ use quickwit_proto::metastore::{ AddSourceRequest, CreateIndexRequest, DeleteIndexRequest, DeleteQuery, DeleteSourceRequest, DeleteSplitsRequest, DeleteTask, IndexMetadataRequest, LastDeleteOpstampRequest, ListAllSplitsRequest, ListDeleteTasksRequest, ListIndexesMetadatasRequest, ListSplitsRequest, - ListStaleSplitsRequest, MarkSplitsForDeletionRequest, MetastoreServiceClient, - PublishSplitsRequest, ResetSourceCheckpointRequest, StageSplitsRequest, ToggleSourceRequest, - UpdateSplitsDeleteOpstampRequest, + ListStaleSplitsRequest, MarkSplitsForDeletionRequest, MetastoreError, MetastoreResult, + MetastoreServiceClient, PublishSplitsRequest, ResetSourceCheckpointRequest, StageSplitsRequest, + ToggleSourceRequest, UpdateSplitsDeleteOpstampRequest, }; use quickwit_proto::tonic::codegen::InterceptedService; use quickwit_proto::tonic::Status; @@ -43,10 +43,7 @@ use quickwit_proto::{IndexUid, SpanContextInterceptor}; use tower::timeout::error::Elapsed; use crate::checkpoint::IndexCheckpointDelta; -use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreError, MetastoreResult, - Split, SplitMetadata, -}; +use crate::{IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMetadata}; // URI describing in a generic way the metastore services resource present in the cluster (= // discovered by Quickwit gossip). This value is used to build the URI of `MetastoreGrpcClient` and @@ -157,12 +154,11 @@ impl Metastore for MetastoreGrpcClient { Ok(index_uid) } - /// Lists indexes. async fn list_indexes_metadatas( &self, query: ListIndexesQuery, ) -> MetastoreResult> { - let filter_json = serde_json::to_string(&query).map_err(|error| { + let query_json = serde_json::to_string(&query).map_err(|error| { MetastoreError::JsonDeserializeError { struct_name: "ListIndexesQuery".to_string(), message: error.to_string(), @@ -171,7 +167,7 @@ impl Metastore for MetastoreGrpcClient { let response = self .underlying .clone() - .list_indexes_metadatas(ListIndexesMetadatasRequest { filter_json }) + .list_indexes_metadatas(ListIndexesMetadatasRequest { query_json }) .await .map_err(|tonic_error| parse_grpc_error(&tonic_error))?; let indexes_metadatas = @@ -279,13 +275,13 @@ impl Metastore for MetastoreGrpcClient { /// Lists the splits. async fn list_splits(&self, query: ListSplitsQuery) -> MetastoreResult> { - let filter_json = + let query_json = serde_json::to_string(&query).map_err(|error| MetastoreError::JsonSerializeError { struct_name: "ListSplitsQuery".to_string(), message: error.to_string(), })?; - let request = ListSplitsRequest { filter_json }; + let request = ListSplitsRequest { query_json }; let response = self .underlying .clone() @@ -559,14 +555,14 @@ pub fn parse_grpc_error(grpc_error: &Status) -> MetastoreError { .and_then(|error| error.downcast_ref::()); if elapsed_error_opt.is_some() { - return MetastoreError::ConnectionError { + return MetastoreError::Connection { message: "gRPC request timeout triggered by the channel timeout. This can happens \ when tonic channel has no registered endpoints." .to_string(), }; } - serde_json::from_str(grpc_error.message()).unwrap_or_else(|_| MetastoreError::InternalError { + serde_json::from_str(grpc_error.message()).unwrap_or_else(|_| MetastoreError::Internal { message: grpc_error.message().to_string(), cause: "".to_string(), }) diff --git a/quickwit/quickwit-metastore/src/metastore/index_metadata/mod.rs b/quickwit/quickwit-metastore/src/metastore/index_metadata/mod.rs index 489dc727033..88e248b583f 100644 --- a/quickwit/quickwit-metastore/src/metastore/index_metadata/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/index_metadata/mod.rs @@ -24,6 +24,7 @@ use std::collections::{BTreeMap, HashMap}; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig, TestableForRegression}; +use quickwit_proto::metastore::{EntityKind, MetastoreError, MetastoreResult}; use quickwit_proto::IndexUid; use serde::{Deserialize, Serialize}; use serialize::VersionedIndexMetadata; @@ -32,7 +33,6 @@ use time::OffsetDateTime; use crate::checkpoint::{ IndexCheckpoint, PartitionId, Position, SourceCheckpoint, SourceCheckpointDelta, }; -use crate::{MetastoreError, MetastoreResult}; /// An index metadata carries all meta data about an index. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] @@ -94,10 +94,9 @@ impl IndexMetadata { let entry = self.sources.entry(source.source_id.clone()); let source_id = source.source_id.clone(); if let Entry::Occupied(_) = entry { - return Err(MetastoreError::SourceAlreadyExists { + return Err(MetastoreError::AlreadyExists(EntityKind::Source { source_id: source_id.clone(), - source_type: source.source_type().to_string(), - }); + })); } entry.or_insert(source); self.checkpoint.add_source(&source_id); @@ -105,12 +104,11 @@ impl IndexMetadata { } pub(crate) fn toggle_source(&mut self, source_id: &str, enable: bool) -> MetastoreResult { - let source = - self.sources - .get_mut(source_id) - .ok_or_else(|| MetastoreError::SourceDoesNotExist { - source_id: source_id.to_string(), - })?; + let source = self.sources.get_mut(source_id).ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Source { + source_id: source_id.to_string(), + }) + })?; let mutation_occurred = source.enabled != enable; source.enabled = enable; Ok(mutation_occurred) @@ -118,11 +116,11 @@ impl IndexMetadata { /// Deletes a source from the index. Returns whether the index was modified (true). pub(crate) fn delete_source(&mut self, source_id: &str) -> MetastoreResult { - self.sources - .remove(source_id) - .ok_or_else(|| MetastoreError::SourceDoesNotExist { + self.sources.remove(source_id).ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Source { source_id: source_id.to_string(), - })?; + }) + })?; self.checkpoint.remove_source(source_id); Ok(true) } diff --git a/quickwit/quickwit-metastore/src/metastore/instrumented_metastore.rs b/quickwit/quickwit-metastore/src/metastore/instrumented_metastore.rs index 82ff3055e45..d2407545149 100644 --- a/quickwit/quickwit-metastore/src/metastore/instrumented_metastore.rs +++ b/quickwit/quickwit-metastore/src/metastore/instrumented_metastore.rs @@ -21,14 +21,11 @@ use async_trait::async_trait; use itertools::Itertools; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{DeleteQuery, DeleteTask, MetastoreResult}; use quickwit_proto::IndexUid; use crate::checkpoint::IndexCheckpointDelta; -use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreResult, Split, - SplitMetadata, -}; +use crate::{IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMetadata}; macro_rules! instrument { ($expr:expr, [$operation:ident, $($label:expr),*]) => { @@ -163,14 +160,14 @@ impl Metastore for InstrumentedMetastore { } async fn list_splits(&self, query: ListSplitsQuery) -> MetastoreResult> { - let index_uids = query + let index_ids = query .index_uids .iter() - .map(|index_uid| index_uid.to_string()) + .map(|index_uid| index_uid.index_id()) .join(","); instrument!( - self.underlying.list_splits(query.clone()).await, - [list_splits, &index_uids] + self.underlying.list_splits(query).await, + [list_splits, &index_ids] ); } diff --git a/quickwit/quickwit-metastore/src/metastore/metastore_event_publisher.rs b/quickwit/quickwit-metastore/src/metastore/metastore_event_publisher.rs index fc0f079f2cd..82588f7c18d 100644 --- a/quickwit/quickwit-metastore/src/metastore/metastore_event_publisher.rs +++ b/quickwit/quickwit-metastore/src/metastore/metastore_event_publisher.rs @@ -24,15 +24,12 @@ use async_trait::async_trait; use quickwit_common::pubsub::{Event, EventBroker}; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{DeleteQuery, DeleteTask, MetastoreResult}; use quickwit_proto::IndexUid; use tracing::info; use crate::checkpoint::IndexCheckpointDelta; -use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreResult, Split, - SplitMetadata, -}; +use crate::{IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMetadata}; /// Metastore events dispatched to subscribers. #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/quickwit/quickwit-metastore/src/metastore/mod.rs b/quickwit/quickwit-metastore/src/metastore/mod.rs index b616df63381..fcaa002048c 100644 --- a/quickwit/quickwit-metastore/src/metastore/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/mod.rs @@ -35,12 +35,14 @@ pub use index_metadata::IndexMetadata; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig}; use quickwit_doc_mapper::tag_pruning::TagFilterAst; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{ + DeleteQuery, DeleteTask, EntityKind, MetastoreError, MetastoreResult, +}; use quickwit_proto::IndexUid; use time::OffsetDateTime; use crate::checkpoint::IndexCheckpointDelta; -use crate::{MetastoreError, MetastoreResult, Split, SplitMetadata, SplitState}; +use crate::{Split, SplitMetadata, SplitState}; /// Metastore meant to manage Quickwit's indexes, their splits and delete tasks. /// @@ -109,7 +111,7 @@ pub trait Metastore: Send + Sync + 'static { async fn index_exists(&self, index_id: &str) -> MetastoreResult { match self.index_metadata(index_id).await { Ok(_) => Ok(true), - Err(MetastoreError::IndexesDoNotExist { .. }) => Ok(false), + Err(MetastoreError::NotFound { .. }) => Ok(false), Err(error) => Err(error), } } @@ -129,9 +131,8 @@ pub trait Metastore: Send + Sync + 'static { let index_metadata = self.index_metadata(index_uid.index_id()).await?; if index_metadata.index_uid != *index_uid { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + let index_id = index_uid.index_id().to_string(); + return Err(MetastoreError::NotFound(EntityKind::Index { index_id })); } Ok(index_metadata) } @@ -142,7 +143,7 @@ pub trait Metastore: Send + Sync + 'static { /// [`IndexMetadata`]. async fn list_indexes_metadatas( &self, - list_indexes_query: ListIndexesQuery, + query: ListIndexesQuery, ) -> MetastoreResult>; /// Deletes an index. @@ -257,9 +258,8 @@ pub trait Metastore: Send + Sync + 'static { // Source API - /// Adds a new source. Fails with - /// [`SourceAlreadyExists`](crate::MetastoreError::SourceAlreadyExists) if a source with the - /// same ID is already defined for the index. + /// Adds a new source. Fails with [`MetastoreError::NotFound`] if a source with the same ID is + /// already defined for the index. /// /// If a checkpoint is already registered for the source, it is kept. async fn add_source(&self, index_uid: IndexUid, source: SourceConfig) -> MetastoreResult<()>; @@ -280,9 +280,8 @@ pub trait Metastore: Send + Sync + 'static { source_id: &str, ) -> MetastoreResult<()>; - /// Deletes a source. Fails with - /// [`SourceDoesNotExist`](crate::MetastoreError::SourceDoesNotExist) if the specified source - /// does not exist. + /// Deletes a source. Fails with [`MetastoreError::NotFound`] if the specified source does not + /// exist. /// /// The checkpoint associated to the source is deleted as well. /// If the checkpoint is missing, this does not trigger an error. @@ -312,20 +311,20 @@ pub trait Metastore: Send + Sync + 'static { ) -> MetastoreResult>; } -/// Query builder for listing indexes within the metastore. +/// A query object for listing indexes stored in the metastore. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub enum ListIndexesQuery { + /// Matches all indexes. + All, /// List of index ID patterns. /// A pattern can contain the wildcard character `*`. IndexIdPatterns(Vec), - /// Matches all indexes. - All, } +/// A query object for listing splits stored in the metastore. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -/// A query builder for listing splits within the metastore. pub struct ListSplitsQuery { - /// A non empty list of index UIDs to get splits from. + /// A non-empty list of index UIDs to get splits from. pub index_uids: Vec, /// The maximum number of splits to retrieve. @@ -358,7 +357,7 @@ pub struct ListSplitsQuery { #[allow(unused_attributes)] impl ListSplitsQuery { - /// Creates a new [ListSplitsQuery] for a specific index. + /// Creates a new [`ListSplitsQuery`] for the designated index. pub fn for_index(index_uid: IndexUid) -> Self { Self { index_uids: vec![index_uid], @@ -374,11 +373,11 @@ impl ListSplitsQuery { } } - /// Creates a new [ListSplitsQuery] from a non-empty list of index Uids. - /// Returns an error if the list of index uids is empty. + /// Creates a new [`ListSplitsQuery`] from a non-empty list of index UIDs. + /// Returns an error if the list is empty. pub fn try_from_index_uids(index_uids: Vec) -> MetastoreResult { if index_uids.is_empty() { - return Err(MetastoreError::InternalError { + return Err(MetastoreError::Internal { message: "ListSplitQuery should define at least one index uid.".to_string(), cause: "".to_string(), }); diff --git a/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs b/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs index cebd2ed0ee1..4241ce5c8a1 100644 --- a/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs +++ b/quickwit/quickwit-metastore/src/metastore/postgresql_metastore.rs @@ -33,7 +33,9 @@ use quickwit_config::{ PostgresMetastoreConfig, SourceConfig, }; use quickwit_doc_mapper::tag_pruning::TagFilterAst; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{ + DeleteQuery, DeleteTask, EntityKind, MetastoreError, MetastoreResult, +}; use quickwit_proto::IndexUid; use sqlx::migrate::Migrator; use sqlx::postgres::{PgConnectOptions, PgDatabaseError, PgPoolOptions}; @@ -44,13 +46,11 @@ use tracing::{debug, error, info, instrument, warn}; use crate::checkpoint::IndexCheckpointDelta; use crate::metastore::instrumented_metastore::InstrumentedMetastore; -use crate::metastore::postgresql_model::{ - DeleteTask as PgDeleteTask, Index as PgIndex, Split as PgSplit, -}; +use crate::metastore::postgresql_model::{PgDeleteTask, PgIndex, PgSplit}; use crate::metastore::FilterRange; use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreError, MetastoreFactory, - MetastoreResolverError, MetastoreResult, Split, SplitMaturity, SplitMetadata, SplitState, + IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreFactory, + MetastoreResolverError, Split, SplitMaturity, SplitMetadata, SplitState, }; static MIGRATOR: Migrator = sqlx::migrate!("migrations/postgresql"); @@ -83,7 +83,7 @@ async fn establish_connection( .await .map_err(|error| { error!(connection_uri=%connection_uri, error=?error, "Failed to establish connection to database."); - MetastoreError::ConnectionError { + MetastoreError::Connection { message: error.to_string(), } }) @@ -98,7 +98,7 @@ async fn run_postgres_migrations(pool: &Pool) -> MetastoreResult<()> { if let Err(migration_err) = migration_res { tx.rollback().await?; error!(err=?migration_err, "Database migrations failed"); - return Err(MetastoreError::InternalError { + return Err(MetastoreError::Internal { message: "Failed to run migration on Postgresql database.".to_string(), cause: migration_err.to_string(), }); @@ -156,7 +156,7 @@ where E: sqlx::Executor<'a, Database = Postgres> { .bind(index_id) .fetch_optional(executor) .await - .map_err(|error| MetastoreError::DbError { + .map_err(|error| MetastoreError::Db { message: error.to_string(), })?; Ok(index_opt) @@ -181,7 +181,7 @@ where .bind(index_uid.to_string()) .fetch_optional(executor) .await - .map_err(|error| MetastoreError::DbError { + .map_err(|error| MetastoreError::Db { message: error.to_string(), })?; Ok(index_opt) @@ -193,8 +193,10 @@ async fn index_metadata( ) -> MetastoreResult { index_opt(tx.as_mut(), index_id) .await? - .ok_or_else(|| MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], + .ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + }) })? .index_metadata() } @@ -349,24 +351,26 @@ fn convert_sqlx_err(index_id: &str, sqlx_err: sqlx::Error) -> MetastoreError { let pg_error_table = pg_db_error.table(); match (pg_error_code, pg_error_table) { - (pg_error_code::FOREIGN_KEY_VIOLATION, _) => MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }, + (pg_error_code::FOREIGN_KEY_VIOLATION, _) => { + MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + }) + } (pg_error_code::UNIQUE_VIOLATION, Some(table)) if table.starts_with("indexes") => { - MetastoreError::IndexAlreadyExists { + MetastoreError::AlreadyExists(EntityKind::Index { index_id: index_id.to_string(), - } + }) } (pg_error_code::UNIQUE_VIOLATION, _) => { error!(pg_db_err=?boxed_db_err, "postgresql-error"); - MetastoreError::InternalError { + MetastoreError::Internal { message: "Unique key violation.".to_string(), cause: format!("DB error {boxed_db_err:?}"), } } _ => { error!(pg_db_err=?boxed_db_err, "postgresql-error"); - MetastoreError::DbError { + MetastoreError::Db { message: boxed_db_err.to_string(), } } @@ -374,7 +378,7 @@ fn convert_sqlx_err(index_id: &str, sqlx_err: sqlx::Error) -> MetastoreError { } _ => { error!(err=?sqlx_err, "An error has occurred in the database operation."); - MetastoreError::DbError { + MetastoreError::Db { message: sqlx_err.to_string(), } } @@ -417,9 +421,9 @@ where let index_id = index_uid.index_id(); let mut index_metadata = index_metadata(tx, index_id).await?; if index_metadata.index_uid != index_uid { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })); } let mutation_occurred = mutate_fn(&mut index_metadata)?; if !mutation_occurred { @@ -443,9 +447,9 @@ where .execute(tx.as_mut()) .await?; if update_index_res.rows_affected() == 0 { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + })); } Ok(mutation_occurred) } @@ -466,7 +470,7 @@ impl Metastore for PostgresqlMetastore { ListIndexesQuery::All => "SELECT * FROM indexes".to_string(), ListIndexesQuery::IndexIdPatterns(index_id_patterns) => { build_index_id_patterns_sql_query(index_id_patterns).map_err(|error| { - MetastoreError::InternalError { + MetastoreError::Internal { message: "Failed to build `list_indexes_metadatas` SQL query".to_string(), cause: error.to_string(), } @@ -510,9 +514,9 @@ impl Metastore for PostgresqlMetastore { .execute(&self.connection_pool) .await?; if delete_res.rows_affected() == 0 { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } Ok(()) } @@ -602,13 +606,15 @@ impl Metastore for PostgresqlMetastore { .map_err(|error| convert_sqlx_err(index_uid.index_id(), error))?; if upserted_split_ids.len() != split_ids.len() { - let failed_split_ids = split_ids + let failed_split_ids: Vec = split_ids .into_iter() - .filter(|id| !upserted_split_ids.contains(id)) + .filter(|split_id| !upserted_split_ids.contains(split_id)) .collect(); - return Err(MetastoreError::SplitsNotStaged { + let entity = EntityKind::Splits { split_ids: failed_split_ids, - }); + }; + let message = "splits are not staged".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } debug!(index_id=%index_uid.index_id(), num_splits=split_ids.len(), "Splits successfully staged."); @@ -628,14 +634,23 @@ impl Metastore for PostgresqlMetastore { run_with_tx!(self.connection_pool, tx, { let mut index_metadata = index_metadata(tx, index_uid.index_id()).await?; if index_metadata.index_uid != index_uid { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } if let Some(checkpoint_delta) = checkpoint_delta_opt { + let source_id = checkpoint_delta.source_id.clone(); index_metadata .checkpoint - .try_apply_delta(checkpoint_delta)?; + .try_apply_delta(checkpoint_delta) + .map_err(|error| { + let entity = EntityKind::CheckpointDelta { + index_id: index_uid.index_id().to_string(), + source_id, + }; + let message = error.to_string(); + MetastoreError::FailedPrecondition { entity, message } + })?; } let index_metadata_json = serde_json::to_string(&index_metadata).map_err(|error| { MetastoreError::JsonSerializeError { @@ -727,19 +742,23 @@ impl Metastore for PostgresqlMetastore { .map_err(|error| convert_sqlx_err(index_uid.index_id(), error))?; if !not_found_split_ids.is_empty() { - return Err(MetastoreError::SplitsDoNotExist { + return Err(MetastoreError::NotFound(EntityKind::Splits { split_ids: not_found_split_ids, - }); + })); } if !not_staged_split_ids.is_empty() { - return Err(MetastoreError::SplitsNotStaged { + let entity = EntityKind::Splits { split_ids: not_staged_split_ids, - }); + }; + let message = "splits are not staged".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } if !not_marked_split_ids.is_empty() { - return Err(MetastoreError::SplitsNotDeletable { + let entity = EntityKind::Splits { split_ids: not_marked_split_ids, - }); + }; + let message = "splits are not marked for deletion".to_string(); + return Err(MetastoreError::FailedPrecondition { entity, message }); } info!( index_id=%index_uid.index_id(), @@ -775,14 +794,14 @@ impl Metastore for PostgresqlMetastore { .into_iter() .map(|index_metadata| index_metadata.index_id().to_string()) .collect(); - let missing_index_ids: Vec = index_ids_str + let not_found_index_ids: Vec = index_ids_str .into_iter() .filter(|index_id| !found_index_ids.contains(index_id)) .collect(); - if !missing_index_ids.is_empty() { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: missing_index_ids, - }); + if !not_found_index_ids.is_empty() { + return Err(MetastoreError::NotFound(EntityKind::Indexes { + index_ids: not_found_index_ids, + })); } } pg_splits @@ -845,9 +864,9 @@ impl Metastore for PostgresqlMetastore { .await? .is_none() { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } info!( index_id=%index_uid.index_id(), @@ -927,14 +946,19 @@ impl Metastore for PostgresqlMetastore { .await? .is_none() { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } if !not_deletable_split_ids.is_empty() { - return Err(MetastoreError::SplitsNotDeletable { + let message = format!( + "splits `{}` are not deletable", + not_deletable_split_ids.join(", ") + ); + let entity = EntityKind::Splits { split_ids: not_deletable_split_ids, - }); + }; + return Err(MetastoreError::FailedPrecondition { entity, message }); } info!(index_id=%index_uid.index_id(), "Deleted {} splits from index.", num_deleted_splits); @@ -953,8 +977,10 @@ impl Metastore for PostgresqlMetastore { async fn index_metadata(&self, index_id: &str) -> MetastoreResult { index_opt(&self.connection_pool, index_id) .await? - .ok_or_else(|| MetastoreError::IndexesDoNotExist { - index_ids: vec![index_id.to_string()], + .ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Index { + index_id: index_id.to_string(), + }) })? .index_metadata() } @@ -1034,7 +1060,7 @@ impl Metastore for PostgresqlMetastore { .bind(index_uid.to_string()) .fetch_one(&self.connection_pool) .await - .map_err(|error| MetastoreError::DbError { + .map_err(|error| MetastoreError::Db { message: error.to_string(), })?; @@ -1113,9 +1139,9 @@ impl Metastore for PostgresqlMetastore { .await? .is_none() { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } Ok(()) } @@ -1181,9 +1207,9 @@ impl Metastore for PostgresqlMetastore { .await? .is_none() { - return Err(MetastoreError::IndexesDoNotExist { - index_ids: vec![index_uid.index_id().to_string()], - }); + return Err(MetastoreError::NotFound(EntityKind::Index { + index_id: index_uid.index_id().to_string(), + })); } pg_stale_splits .into_iter() @@ -1258,11 +1284,9 @@ fn build_index_id_patterns_sql_query(index_id_patterns: Vec) -> anyhow:: } let mut where_like_query = String::new(); for (index_id_pattern_idx, index_id_pattern) in index_id_patterns.iter().enumerate() { - validate_index_id_pattern(index_id_pattern).map_err(|error| { - MetastoreError::InternalError { - message: "Failed to build list indexes query".to_string(), - cause: error.to_string(), - } + validate_index_id_pattern(index_id_pattern).map_err(|error| MetastoreError::Internal { + message: "Failed to build list indexes query".to_string(), + cause: error.to_string(), })?; if index_id_pattern.contains('*') { let sql_pattern = index_id_pattern.replace('*', "%"); @@ -1334,7 +1358,7 @@ impl MetastoreFactory for PostgresqlMetastoreFactory { })?; let postgresql_metastore = PostgresqlMetastore::new(postgresql_metastore_config, uri) .await - .map_err(MetastoreResolverError::FailedToOpenMetastore)?; + .map_err(MetastoreResolverError::Initialization)?; let instrumented_metastore = InstrumentedMetastore::new(Box::new(postgresql_metastore)); let unique_metastore_for_uri = self .cache_metastore(uri.clone(), Arc::new(instrumented_metastore)) @@ -1638,7 +1662,7 @@ mod tests { build_index_id_patterns_sql_query(vec!["*-index-*-&-last**".to_string()]) .unwrap_err() .to_string(), - "Internal error: `Failed to build list indexes query` Cause: `Index ID pattern \ + "Internal error: Failed to build list indexes query Cause: `Index ID pattern \ `*-index-*-&-last**` is invalid. Patterns must match the following regular \ expression: `^[a-zA-Z\\*][a-zA-Z0-9-_\\.\\*]{0,254}$`.`." ); diff --git a/quickwit/quickwit-metastore/src/metastore/postgresql_model.rs b/quickwit/quickwit-metastore/src/metastore/postgresql_model.rs index ae5aca36c13..85c59f793e1 100644 --- a/quickwit/quickwit-metastore/src/metastore/postgresql_model.rs +++ b/quickwit/quickwit-metastore/src/metastore/postgresql_model.rs @@ -20,18 +20,15 @@ use std::convert::TryInto; use std::str::FromStr; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask as QuickwitDeleteTask}; +use quickwit_proto::metastore::{DeleteQuery, DeleteTask, MetastoreError, MetastoreResult}; use quickwit_proto::IndexUid; use tracing::error; -use crate::{ - IndexMetadata, MetastoreError, MetastoreResult, Split as QuickwitSplit, SplitMetadata, - SplitState, -}; +use crate::{IndexMetadata, Split, SplitMetadata, SplitState}; /// A model structure for handling index metadata in a database. #[derive(sqlx::FromRow)] -pub struct Index { +pub struct PgIndex { /// Index UID. The index UID identifies the index when querying the metastore from the /// application. #[sqlx(try_from = "String")] @@ -44,7 +41,7 @@ pub struct Index { pub create_timestamp: sqlx::types::time::PrimitiveDateTime, } -impl Index { +impl PgIndex { /// Deserializes index metadata from JSON string stored in column and sets appropriate /// timestamps. pub fn index_metadata(&self) -> MetastoreResult { @@ -67,7 +64,7 @@ impl Index { /// A model structure for handling split metadata in a database. #[derive(sqlx::FromRow)] -pub struct Split { +pub struct PgSplit { /// Split ID. pub split_id: String, /// The state of the split. With `update_timestamp`, this is the only mutable attribute of the @@ -97,7 +94,7 @@ pub struct Split { pub delete_opstamp: i64, } -impl Split { +impl PgSplit { /// Deserializes and returns the split's metadata. fn split_metadata(&self) -> MetastoreResult { serde_json::from_str::(&self.split_metadata_json).map_err(|error| { @@ -123,10 +120,10 @@ impl Split { } } -impl TryInto for Split { +impl TryInto for PgSplit { type Error = MetastoreError; - fn try_into(self) -> Result { + fn try_into(self) -> Result { let mut split_metadata = self.split_metadata()?; // `create_timestamp` and `delete_opstamp` are duplicated in `SplitMetadata` and needs to be // overridden with the "true" value stored in a column. @@ -138,7 +135,7 @@ impl TryInto for Split { .map(|publish_timestamp| publish_timestamp.assume_utc().unix_timestamp()); split_metadata.index_uid = self.index_uid; split_metadata.delete_opstamp = self.delete_opstamp as u64; - Ok(QuickwitSplit { + Ok(Split { split_metadata, split_state, update_timestamp, @@ -149,7 +146,7 @@ impl TryInto for Split { /// A model structure for handling split metadata in a database. #[derive(sqlx::FromRow)] -pub struct DeleteTask { +pub struct PgDeleteTask { /// Create timestamp. pub create_timestamp: sqlx::types::time::PrimitiveDateTime, /// Monotonic increasing unique opstamp. @@ -161,7 +158,7 @@ pub struct DeleteTask { pub delete_query_json: String, } -impl DeleteTask { +impl PgDeleteTask { /// Deserializes and returns the split's metadata. fn delete_query(&self) -> MetastoreResult { serde_json::from_str::(&self.delete_query_json).map_err(|error| { @@ -175,12 +172,12 @@ impl DeleteTask { } } -impl TryInto for DeleteTask { +impl TryInto for PgDeleteTask { type Error = MetastoreError; - fn try_into(self) -> Result { + fn try_into(self) -> Result { let delete_query = self.delete_query()?; - Ok(QuickwitDeleteTask { + Ok(DeleteTask { create_timestamp: self.create_timestamp.assume_utc().unix_timestamp(), opstamp: self.opstamp as u64, delete_query: Some(delete_query), diff --git a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/error.rs b/quickwit/quickwit-metastore/src/metastore/retrying_metastore/error.rs deleted file mode 100644 index f3054c17e7d..00000000000 --- a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/error.rs +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (C) 2023 Quickwit, Inc. -// -// Quickwit is offered under the AGPL v3.0 and as commercial software. -// For commercial licensing, contact us at hello@quickwit.io. -// -// AGPL: -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU Affero General Public License as -// published by the Free Software Foundation, either version 3 of the -// License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Affero General Public License for more details. -// -// You should have received a copy of the GNU Affero General Public License -// along with this program. If not, see . - -use quickwit_common::retry::Retryable; - -use crate::MetastoreError; - -impl Retryable for MetastoreError { - fn is_retryable(&self) -> bool { - matches!( - self, - MetastoreError::ConnectionError { message: _ } - | MetastoreError::Io { message: _ } - | MetastoreError::DbError { message: _ } - | MetastoreError::InternalError { - message: _, - cause: _, - } - ) - } -} diff --git a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/mod.rs b/quickwit/quickwit-metastore/src/metastore/retrying_metastore/mod.rs index 115655699fe..a00b5f7862f 100644 --- a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/retrying_metastore/mod.rs @@ -17,7 +17,6 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -mod error; mod retry; #[cfg(test)] mod test; @@ -26,15 +25,12 @@ use async_trait::async_trait; use quickwit_common::retry::RetryParams; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{DeleteQuery, DeleteTask, MetastoreResult}; use quickwit_proto::IndexUid; use self::retry::retry; use crate::checkpoint::IndexCheckpointDelta; -use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreResult, Split, - SplitMetadata, -}; +use crate::{IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMetadata}; /// Retry layer for a [`Metastore`]. /// This is a band-aid solution for now. This will be removed after retry can be usable on diff --git a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/test.rs b/quickwit/quickwit-metastore/src/metastore/retrying_metastore/test.rs index 53cd515b398..157fbcb1459 100644 --- a/quickwit/quickwit-metastore/src/metastore/retrying_metastore/test.rs +++ b/quickwit/quickwit-metastore/src/metastore/retrying_metastore/test.rs @@ -23,13 +23,15 @@ use async_trait::async_trait; use quickwit_common::retry::RetryParams; use quickwit_common::uri::Uri; use quickwit_config::{IndexConfig, SourceConfig}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_proto::metastore::{ + DeleteQuery, DeleteTask, EntityKind, MetastoreError, MetastoreResult, +}; use quickwit_proto::IndexUid; use crate::checkpoint::IndexCheckpointDelta; use crate::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreError, MetastoreResult, - RetryingMetastore, Split, SplitMetadata, + IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, RetryingMetastore, Split, + SplitMetadata, }; struct RetryTestMetastore { @@ -226,16 +228,16 @@ async fn test_retryable_metastore_errors() { let metastore: RetryingMetastore = RetryTestMetastore::new_retrying_with_errors( 5, &[ - MetastoreError::ConnectionError { + MetastoreError::Connection { message: "".to_string(), }, MetastoreError::Io { message: "".to_string(), }, - MetastoreError::DbError { + MetastoreError::Db { message: "".to_string(), }, - MetastoreError::InternalError { + MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }, @@ -251,9 +253,9 @@ async fn test_retryable_metastore_errors() { let metastore: RetryingMetastore = RetryTestMetastore::new_retrying_with_errors( 5, - &[MetastoreError::IndexesDoNotExist { - index_ids: vec!["".to_string()], - }], + &[MetastoreError::NotFound(EntityKind::Index { + index_id: "".to_string(), + })], ); // On non-retryable errors, RetryingMetastore should exit with an error. @@ -270,7 +272,7 @@ async fn test_retryable_more_than_max_retry() { &(0..4) .collect::>() .iter() - .map(|index| MetastoreError::ConnectionError { + .map(|index| MetastoreError::Connection { message: format!("{index}"), }) .collect::>(), @@ -282,7 +284,7 @@ async fn test_retryable_more_than_max_retry() { .unwrap_err(); assert_eq!( error, - MetastoreError::ConnectionError { + MetastoreError::Connection { message: "2".to_string() // Max 3 retries, last error index is 2 } ) @@ -293,18 +295,17 @@ async fn test_mixed_retryable_metastore_errors() { let metastore: RetryingMetastore = RetryTestMetastore::new_retrying_with_errors( 5, &[ - MetastoreError::ConnectionError { + MetastoreError::Connection { message: "".to_string(), }, MetastoreError::Io { message: "".to_string(), }, // Non-retryable - MetastoreError::SourceAlreadyExists { + MetastoreError::AlreadyExists(EntityKind::Source { source_id: "".to_string(), - source_type: "".to_string(), - }, - MetastoreError::InternalError { + }), + MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }, @@ -318,9 +319,8 @@ async fn test_mixed_retryable_metastore_errors() { assert_eq!( error, - MetastoreError::SourceAlreadyExists { - source_id: "".to_string(), - source_type: "".to_string(), - } + MetastoreError::AlreadyExists(EntityKind::Source { + source_id: "".to_string() + }), ) } diff --git a/quickwit/quickwit-metastore/src/tests.rs b/quickwit/quickwit-metastore/src/tests.rs index 779a353b0f2..9cff865fad2 100644 --- a/quickwit/quickwit-metastore/src/tests.rs +++ b/quickwit/quickwit-metastore/src/tests.rs @@ -30,7 +30,7 @@ pub mod test_suite { use quickwit_common::rand::append_random_suffix; use quickwit_config::{IndexConfig, SourceConfig, SourceInputFormat, SourceParams}; use quickwit_doc_mapper::tag_pruning::{no_tag, tag, TagFilterAst}; - use quickwit_proto::metastore::DeleteQuery; + use quickwit_proto::metastore::{DeleteQuery, EntityKind, MetastoreError}; use quickwit_proto::IndexUid; use quickwit_query::query_ast::qast_json_helper; use time::OffsetDateTime; @@ -41,8 +41,8 @@ pub mod test_suite { IndexCheckpointDelta, PartitionId, Position, SourceCheckpoint, SourceCheckpointDelta, }; use crate::{ - ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreError, Split, SplitMaturity, - SplitMetadata, SplitState, + ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitMaturity, SplitMetadata, + SplitState, }; #[async_trait] @@ -111,7 +111,7 @@ pub mod test_suite { assert_eq!(index_metadata.index_uri(), &index_uri); let error = metastore.create_index(index_config).await.unwrap_err(); - assert!(matches!(error, MetastoreError::IndexAlreadyExists { .. })); + assert!(matches!(error, MetastoreError::AlreadyExists { .. })); cleanup_index(&metastore, index_uid).await; } @@ -162,7 +162,10 @@ pub mod test_suite { .index_metadata("index-not-found") .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); let index_uid = metastore.create_index(index_config.clone()).await.unwrap(); @@ -272,13 +275,19 @@ pub mod test_suite { .delete_index(IndexUid::new("index-not-found")) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); let error = metastore .delete_index(IndexUid::new("test-delete-index")) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); let index_uid = metastore.create_index(index_config.clone()).await.unwrap(); @@ -363,21 +372,21 @@ pub mod test_suite { .add_source(index_uid.clone(), source.clone()) .await .unwrap_err(), - MetastoreError::SourceAlreadyExists { .. } + MetastoreError::AlreadyExists(EntityKind::Source { .. }) )); assert!(matches!( metastore .add_source(IndexUid::new("index-not-found"), source.clone()) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); assert!(matches!( metastore .add_source(IndexUid::new(index_id), source) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); cleanup_index(&metastore, index_uid).await; } @@ -455,14 +464,14 @@ pub mod test_suite { .add_source(IndexUid::new("index-not-found"), source.clone()) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); assert!(matches!( metastore .add_source(IndexUid::new(&index_id), source.clone()) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); metastore @@ -482,21 +491,21 @@ pub mod test_suite { .delete_source(index_uid.clone(), &source_id) .await .unwrap_err(), - MetastoreError::SourceDoesNotExist { .. } + MetastoreError::NotFound(EntityKind::Source { .. }) )); assert!(matches!( metastore .delete_source(IndexUid::new("index-not-found"), &source_id) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); assert!(matches!( metastore .delete_source(IndexUid::new(index_id), &source_id) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); cleanup_index(&metastore, index_uid).await; @@ -571,7 +580,7 @@ pub mod test_suite { .reset_source_checkpoint(IndexUid::new("index-not-found"), &source_ids[1]) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); assert!(matches!( @@ -579,7 +588,7 @@ pub mod test_suite { .reset_source_checkpoint(IndexUid::new(&index_id), &source_ids[1]) .await .unwrap_err(), - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); metastore @@ -622,7 +631,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); } // Update the checkpoint, by publishing an empty array of splits with a non-empty @@ -706,7 +718,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); } // Publish a split on a wrong index uid @@ -724,7 +739,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); } // Publish a non-existent split on an index @@ -735,7 +753,10 @@ pub mod test_suite { .publish_splits(index_uid.clone(), &["split-not-found"], &[], None) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -795,7 +816,10 @@ pub mod test_suite { .unwrap_err(); assert!(matches!( error, - MetastoreError::IncompatibleCheckpointDelta(_) + MetastoreError::FailedPrecondition { + entity: EntityKind::CheckpointDelta { .. }, + .. + } )); cleanup_index(&metastore, index_uid).await; @@ -842,7 +866,13 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsNotStaged { .. })); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + )); cleanup_index(&metastore, index_uid).await; } @@ -869,7 +899,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -910,7 +943,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -956,7 +992,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -1031,7 +1070,13 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsNotStaged { .. })); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + )); cleanup_index(&metastore, index_uid).await; } @@ -1077,7 +1122,10 @@ pub mod test_suite { .unwrap_err(); assert!(matches!( error, - MetastoreError::IncompatibleCheckpointDelta(_) + MetastoreError::FailedPrecondition { + entity: EntityKind::CheckpointDelta { .. }, + .. + } )); cleanup_index(&metastore, index_uid).await; @@ -1201,7 +1249,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); } // Replace a non-existent split on an index @@ -1218,7 +1269,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -1242,7 +1296,10 @@ pub mod test_suite { .publish_splits(index_uid.clone(), &[&split_id_2], &[&split_id_1], None) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -1274,7 +1331,13 @@ pub mod test_suite { .publish_splits(index_uid.clone(), &[&split_id_2], &[&split_id_1], None) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsNotStaged { .. })); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + )); cleanup_index(&metastore, index_uid).await; } @@ -1307,7 +1370,10 @@ pub mod test_suite { ) // TODO source id .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Splits { .. }) + )); cleanup_index(&metastore, index_uid).await; } @@ -1341,7 +1407,7 @@ pub mod test_suite { .await .unwrap_err(); assert!( - matches!(error, MetastoreError::SplitsNotDeletable { split_ids } if split_ids == [split_id_1.clone()]) + matches!(error, MetastoreError::FailedPrecondition { entity: EntityKind::Splits { split_ids }, .. } if split_ids == [split_id_1.clone()]) ); cleanup_index(&metastore, index_uid).await; @@ -1401,7 +1467,10 @@ pub mod test_suite { .mark_splits_for_deletion(IndexUid::new("index-not-found"), &[]) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); metastore .mark_splits_for_deletion(index_uid.clone(), &["split-not-found"]) @@ -1523,14 +1592,20 @@ pub mod test_suite { .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); let error = metastore .delete_splits(IndexUid::new(&index_id), &[]) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); metastore .delete_splits(index_uid.clone(), &["split-not-found"]) @@ -1568,7 +1643,13 @@ pub mod test_suite { .await .unwrap_err(); - assert!(matches!(error, MetastoreError::SplitsNotDeletable { .. })); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + )); assert_eq!( metastore @@ -1653,7 +1734,11 @@ pub mod test_suite { .list_all_splits(IndexUid::new("index-not-found")) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + // TODO: This discrepancy is tracked in #3760. + MetastoreError::NotFound(EntityKind::Index { .. } | EntityKind::Indexes { .. }) + )); let index_uid = metastore.create_index(index_config).await.unwrap(); @@ -1776,7 +1861,11 @@ pub mod test_suite { let query = ListSplitsQuery::for_index(index_uid.clone()).with_split_state(SplitState::Staged); let error = metastore.list_splits(query).await.unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + // TODO: This discrepancy is tracked in #3760. + MetastoreError::NotFound(EntityKind::Index { .. } | EntityKind::Indexes { .. }) + )); } { let index_uid = metastore.create_index(index_config.clone()).await.unwrap(); @@ -2250,7 +2339,10 @@ pub mod test_suite { }) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); // Create a delete task on an index with wrong incarnation_id let error = metastore @@ -2260,7 +2352,10 @@ pub mod test_suite { }) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); // Create a delete task. let delete_task_1 = metastore @@ -2490,8 +2585,10 @@ pub mod test_suite { .list_stale_splits(IndexUid::new("index-not-found"), 0, 10) .await .unwrap_err(); - println!("{:?}", error); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); { info!("List stale splits on an index"); @@ -2618,7 +2715,7 @@ pub mod test_suite { error!(err=?metastore_err); assert!(matches!( metastore_err, - MetastoreError::IndexesDoNotExist { .. } + MetastoreError::NotFound(EntityKind::Index { .. }) )); } @@ -2704,7 +2801,10 @@ pub mod test_suite { ) .await .unwrap_err(); - assert!(matches!(error, MetastoreError::IndexesDoNotExist { .. })); + assert!(matches!( + error, + MetastoreError::NotFound(EntityKind::Index { .. }) + )); let index_uid = metastore.create_index(index_config.clone()).await.unwrap(); @@ -2733,33 +2833,37 @@ pub mod test_suite { .publish_splits(index_uid.clone(), &[&split_id_1, &split_id_2], &[], None) .await .unwrap(); - let err = metastore + let error = metastore .stage_splits(index_uid.clone(), vec![split_metadata_1.clone()]) .await .expect_err( "Metastore should not allow splits which are not `Staged` to be overwritten.", ); - assert!( - matches!(err, MetastoreError::SplitsNotStaged { .. }), - "Metastore should return a `SplitsNotStaged` error when attempting to perform an \ - operation that must not occur.", - ); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + ),); metastore .mark_splits_for_deletion(index_uid.clone(), &[&split_id_2]) .await .unwrap(); - let err = metastore + let error = metastore .stage_splits(index_uid.clone(), vec![split_metadata_2.clone()]) .await .expect_err( "Metastore should not allow splits which are not `Staged` to be overwritten.", ); - assert!( - matches!(err, MetastoreError::SplitsNotStaged { .. }), - "Metastore should return a `SplitsNotStaged` error when attempting to perform an \ - operation that must not occur.", - ); + assert!(matches!( + error, + MetastoreError::FailedPrecondition { + entity: EntityKind::Splits { .. }, + .. + } + ),); cleanup_index(&metastore, index_uid).await; } diff --git a/quickwit/quickwit-proto/Cargo.toml b/quickwit/quickwit-proto/Cargo.toml index 2e7eb9c3ec2..c1b0e8bb3a7 100644 --- a/quickwit/quickwit-proto/Cargo.toml +++ b/quickwit/quickwit-proto/Cargo.toml @@ -22,6 +22,7 @@ prost = { workspace = true } prost-types = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +sqlx = { workspace = true, optional = true } thiserror = { workspace = true } tonic = { workspace = true } tokio = { workspace = true } @@ -45,4 +46,5 @@ tonic-build = { workspace = true } quickwit-codegen = { workspace = true } [features] +postgres = [ "sqlx" ] testsuite = [ "mockall" ] diff --git a/quickwit/quickwit-proto/protos/quickwit/metastore.proto b/quickwit/quickwit-proto/protos/quickwit/metastore.proto index 940d2693fa6..44112c5ba74 100644 --- a/quickwit/quickwit-proto/protos/quickwit/metastore.proto +++ b/quickwit/quickwit-proto/protos/quickwit/metastore.proto @@ -104,7 +104,7 @@ message CreateIndexResponse { } message ListIndexesMetadatasRequest { - string filter_json = 1; + string query_json = 1; } message ListIndexesMetadatasResponse { @@ -132,7 +132,7 @@ message ListAllSplitsRequest { } message ListSplitsRequest { - string filter_json = 1; + string query_json = 1; } message ListSplitsResponse { diff --git a/quickwit/quickwit-proto/src/codegen/quickwit/quickwit.metastore.rs b/quickwit/quickwit-proto/src/codegen/quickwit/quickwit.metastore.rs index a2a371675ea..ad621bbad79 100644 --- a/quickwit/quickwit-proto/src/codegen/quickwit/quickwit.metastore.rs +++ b/quickwit/quickwit-proto/src/codegen/quickwit/quickwit.metastore.rs @@ -17,7 +17,7 @@ pub struct CreateIndexResponse { #[derive(Clone, PartialEq, ::prost::Message)] pub struct ListIndexesMetadatasRequest { #[prost(string, tag = "1")] - pub filter_json: ::prost::alloc::string::String, + pub query_json: ::prost::alloc::string::String, } #[derive(Serialize, Deserialize, utoipa::ToSchema)] #[allow(clippy::derive_partial_eq_without_eq)] @@ -67,7 +67,7 @@ pub struct ListAllSplitsRequest { #[derive(Clone, PartialEq, ::prost::Message)] pub struct ListSplitsRequest { #[prost(string, tag = "1")] - pub filter_json: ::prost::alloc::string::String, + pub query_json: ::prost::alloc::string::String, } #[derive(Serialize, Deserialize, utoipa::ToSchema)] #[allow(clippy::derive_partial_eq_without_eq)] diff --git a/quickwit/quickwit-proto/src/metastore/mod.rs b/quickwit/quickwit-proto/src/metastore/mod.rs index 24793f8c5ee..2cbf99163d1 100644 --- a/quickwit/quickwit-proto/src/metastore/mod.rs +++ b/quickwit/quickwit-proto/src/metastore/mod.rs @@ -17,7 +17,160 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +use std::fmt; + +use quickwit_common::retry::Retryable; + +use crate::{IndexId, QueueId, ServiceError, ServiceErrorCode, SourceId, SplitId}; + include!("../codegen/quickwit/quickwit.metastore.rs"); pub use metastore_service_client::MetastoreServiceClient; pub use metastore_service_server::{MetastoreService, MetastoreServiceServer}; + +pub type MetastoreResult = Result; + +/// Lists the object types stored and managed by the metastore. +#[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] +pub enum EntityKind { + /// A checkpoint delta. + CheckpointDelta { + /// Index ID. + index_id: IndexId, + /// Source Id. + source_id: SourceId, + }, + /// An index. + Index { + /// Index ID. + index_id: IndexId, + }, + /// A set of indexes. + Indexes { + /// Index IDs. + index_ids: Vec, + }, + /// A source. + Source { + /// Source ID. + source_id: SourceId, + }, + /// A shard. + Shard { + /// Shard queue ID: // + queue_id: QueueId, + }, + /// A split. + Split { + /// Split ID. + split_id: SplitId, + }, + /// A set of splits. + Splits { + /// Split IDs. + split_ids: Vec, + }, +} + +impl fmt::Display for EntityKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + EntityKind::CheckpointDelta { + index_id, + source_id, + } => write!(f, "checkpoint delta `{index_id}/{source_id}`"), + EntityKind::Index { index_id } => write!(f, "index `{}`", index_id), + EntityKind::Indexes { index_ids } => write!(f, "indexes `{}`", index_ids.join(", ")), + EntityKind::Shard { queue_id } => write!(f, "shard `{queue_id}`"), + EntityKind::Source { source_id } => write!(f, "source `{source_id}`"), + EntityKind::Split { split_id } => write!(f, "split `{split_id}`"), + EntityKind::Splits { split_ids } => write!(f, "splits `{}`", split_ids.join(", ")), + } + } +} + +#[derive(Debug, Clone, thiserror::Error, Eq, PartialEq, Serialize, Deserialize)] +pub enum MetastoreError { + #[error("Database error: {message}.")] + Db { message: String }, + + #[error("Connection error: {message}.")] + Connection { message: String }, + + #[error("Precondition failed for {entity}: {message}")] + FailedPrecondition { entity: EntityKind, message: String }, + + #[error("{0} do(es) not exist.")] + NotFound(EntityKind), + + #[error("{0} already exist(s).")] + AlreadyExists(EntityKind), + + #[error("Access forbidden: {message}.")] + Forbidden { message: String }, + + #[error("Internal error: {message} Cause: `{cause}`.")] + Internal { message: String, cause: String }, + + #[error("IO error: {message}.")] + Io { message: String }, + + #[error("Failed to deserialize `{struct_name}` from JSON: {message}.")] + JsonDeserializeError { + struct_name: String, + message: String, + }, + + #[error("Failed to serialize `{struct_name}` to JSON: {message}.")] + JsonSerializeError { + struct_name: String, + message: String, + }, +} + +#[cfg(feature = "postgres")] +impl From for MetastoreError { + fn from(error: sqlx::Error) -> Self { + MetastoreError::Db { + message: error.to_string(), + } + } +} + +impl From for tonic::Status { + fn from(metastore_error: MetastoreError) -> Self { + let grpc_code = metastore_error.status_code().to_grpc_status_code(); + let error_msg = serde_json::to_string(&metastore_error) + .unwrap_or_else(|_| format!("Raw metastore error: {metastore_error}")); + tonic::Status::new(grpc_code, error_msg) + } +} + +impl crate::ServiceError for MetastoreError { + fn status_code(&self) -> ServiceErrorCode { + match self { + Self::Connection { .. } => ServiceErrorCode::Internal, + Self::Db { .. } => ServiceErrorCode::Internal, + Self::FailedPrecondition { .. } => ServiceErrorCode::BadRequest, + Self::Forbidden { .. } => ServiceErrorCode::MethodNotAllowed, + Self::AlreadyExists { .. } => ServiceErrorCode::BadRequest, + Self::NotFound { .. } => ServiceErrorCode::NotFound, + Self::Internal { .. } => ServiceErrorCode::Internal, + Self::Io { .. } => ServiceErrorCode::Internal, + Self::JsonDeserializeError { .. } => ServiceErrorCode::Internal, + Self::JsonSerializeError { .. } => ServiceErrorCode::Internal, + } + } +} + +impl Retryable for MetastoreError { + fn is_retryable(&self) -> bool { + matches!( + self, + MetastoreError::Connection { .. } + | MetastoreError::Db { .. } + | MetastoreError::Io { .. } + | MetastoreError::Internal { .. } + ) + } +} diff --git a/quickwit/quickwit-proto/src/types.rs b/quickwit/quickwit-proto/src/types.rs index d7a6b8d5e9e..97bacea5ccd 100644 --- a/quickwit/quickwit-proto/src/types.rs +++ b/quickwit/quickwit-proto/src/types.rs @@ -31,6 +31,8 @@ pub type IndexId = String; pub type SourceId = String; +pub type SplitId = String; + pub type ShardId = u64; /// Uniquely identifies a shard and its underlying mrecordlog queue. diff --git a/quickwit/quickwit-search/src/cluster_client.rs b/quickwit/quickwit-search/src/cluster_client.rs index e963b1c0121..14d30cb1fd4 100644 --- a/quickwit/quickwit-search/src/cluster_client.rs +++ b/quickwit/quickwit-search/src/cluster_client.rs @@ -469,7 +469,7 @@ mod tests { let mut mock_search_service_1 = MockSearchService::new(); mock_search_service_1.expect_fetch_docs().return_once( |_: quickwit_proto::search::FetchDocsRequest| { - Err(SearchError::InternalError("error".to_string())) + Err(SearchError::Internal("error".to_string())) }, ); let mut mock_search_service_2 = MockSearchService::new(); @@ -499,7 +499,7 @@ mod tests { let mut mock_search_service = MockSearchService::new(); mock_search_service.expect_fetch_docs().returning( |_: quickwit_proto::search::FetchDocsRequest| { - Err(SearchError::InternalError("error".to_string())) + Err(SearchError::Internal("error".to_string())) }, ); let searcher_pool = searcher_pool_for_test([("127.0.0.1:1001", mock_search_service)]); @@ -511,7 +511,7 @@ mod tests { .fetch_docs(request, first_client) .await .unwrap_err(); - assert!(matches!(search_error, SearchError::InternalError(_))); + assert!(matches!(search_error, SearchError::Internal(_))); } #[tokio::test] @@ -632,7 +632,7 @@ mod tests { ..Default::default() }; let merged_result = merge_leaf_search_results( - Err(SearchError::InternalError("error".to_string())), + Err(SearchError::Internal("error".to_string())), Ok(leaf_response), ) .unwrap(); @@ -646,8 +646,8 @@ mod tests { #[test] fn test_merge_leaf_search_retry_error_on_error() -> anyhow::Result<()> { let merge_error = merge_leaf_search_results( - Err(SearchError::InternalError("error".to_string())), - Err(SearchError::InternalError("retry error".to_string())), + Err(SearchError::Internal("error".to_string())), + Err(SearchError::Internal("retry error".to_string())), ) .unwrap_err(); assert_eq!(merge_error.to_string(), "Internal error: `error`."); @@ -661,7 +661,7 @@ mod tests { let mut mock_search_service_1 = MockSearchService::new(); mock_search_service_1 .expect_leaf_search_stream() - .return_once(|_| Err(SearchError::InternalError("error".to_string()))); + .return_once(|_| Err(SearchError::Internal("error".to_string()))); let mut mock_search_service_2 = MockSearchService::new(); let (result_sender, result_receiver) = tokio::sync::mpsc::unbounded_channel(); @@ -683,9 +683,7 @@ mod tests { })) .unwrap(); result_sender - .send(Err(SearchError::InternalError( - "last split error".to_string(), - ))) + .send(Err(SearchError::Internal("last split error".to_string()))) .unwrap(); drop(result_sender); diff --git a/quickwit/quickwit-search/src/error.rs b/quickwit/quickwit-search/src/error.rs index 9f92176427b..6ffae9465ac 100644 --- a/quickwit/quickwit-search/src/error.rs +++ b/quickwit/quickwit-search/src/error.rs @@ -18,7 +18,7 @@ // along with this program. If not, see . use quickwit_doc_mapper::QueryParserError; -use quickwit_metastore::MetastoreError; +use quickwit_proto::metastore::{EntityKind, MetastoreError}; use quickwit_proto::{tonic, ServiceError, ServiceErrorCode}; use quickwit_storage::StorageResolverError; use serde::{Deserialize, Serialize}; @@ -26,33 +26,33 @@ use tantivy::TantivyError; use thiserror::Error; use tokio::task::JoinError; -/// Possible SearchError. +/// Possible SearchError #[allow(missing_docs)] #[derive(Error, Debug, Serialize, Deserialize, Clone)] pub enum SearchError { - #[error("Indexes IDs or index ID patterns `{index_id_patterns:?}` do not exist.")] - IndexesDoNotExist { index_id_patterns: Vec }, + #[error("Could not find indexes matching the IDs or patterns `{index_id_patterns:?}`.")] + IndexesNotFound { index_id_patterns: Vec }, #[error("Internal error: `{0}`.")] - InternalError(String), - #[error("Storage not found: `{0}`)")] - StorageResolverError(#[from] StorageResolverError), + Internal(String), #[error("Invalid aggregation request: {0}")] InvalidAggregationRequest(String), #[error("Invalid argument: {0}")] InvalidArgument(String), #[error("{0}")] InvalidQuery(String), + #[error("Storage not found: `{0}`)")] + StorageResolver(#[from] StorageResolverError), } impl ServiceError for SearchError { fn status_code(&self) -> ServiceErrorCode { match self { - SearchError::IndexesDoNotExist { .. } => ServiceErrorCode::NotFound, - SearchError::InternalError(_) => ServiceErrorCode::Internal, - SearchError::StorageResolverError(_) => ServiceErrorCode::BadRequest, - SearchError::InvalidQuery(_) => ServiceErrorCode::BadRequest, - SearchError::InvalidArgument(_) => ServiceErrorCode::BadRequest, + SearchError::IndexesNotFound { .. } => ServiceErrorCode::NotFound, + SearchError::Internal(_) => ServiceErrorCode::Internal, SearchError::InvalidAggregationRequest(_) => ServiceErrorCode::BadRequest, + SearchError::InvalidArgument(_) => ServiceErrorCode::BadRequest, + SearchError::InvalidQuery(_) => ServiceErrorCode::BadRequest, + SearchError::StorageResolver(_) => ServiceErrorCode::BadRequest, } } } @@ -66,53 +66,60 @@ impl From for tonic::Status { /// Parse tonic error and returns `SearchError`. pub fn parse_grpc_error(grpc_error: &tonic::Status) -> SearchError { serde_json::from_str(grpc_error.message()) - .unwrap_or_else(|_| SearchError::InternalError(grpc_error.message().to_string())) + .unwrap_or_else(|_| SearchError::Internal(grpc_error.message().to_string())) } impl From for SearchError { - fn from(tantivy_err: TantivyError) -> Self { - SearchError::InternalError(format!("{tantivy_err}")) + fn from(tantivy_error: TantivyError) -> Self { + SearchError::Internal(format!("tantivy error: {tantivy_error}")) } } impl From for SearchError { fn from(error: postcard::Error) -> Self { - SearchError::InternalError(format!("Postcard error: {error}")) + SearchError::Internal(format!("Postcard error: {error}")) } } impl From for SearchError { fn from(serde_error: serde_json::Error) -> Self { - SearchError::InternalError(format!("Serde error: {serde_error}")) + SearchError::Internal(format!("Serde error: {serde_error}")) } } impl From for SearchError { - fn from(any_err: anyhow::Error) -> Self { - SearchError::InternalError(format!("{any_err}")) + fn from(any_error: anyhow::Error) -> Self { + SearchError::Internal(any_error.to_string()) } } impl From for SearchError { fn from(query_parser_error: QueryParserError) -> Self { - SearchError::InvalidQuery(format!("{query_parser_error}")) + SearchError::InvalidQuery(query_parser_error.to_string()) } } impl From for SearchError { fn from(metastore_error: MetastoreError) -> SearchError { match metastore_error { - MetastoreError::IndexesDoNotExist { index_ids } => SearchError::IndexesDoNotExist { - index_id_patterns: index_ids, - }, - _ => SearchError::InternalError(format!("{metastore_error}")), + MetastoreError::NotFound(EntityKind::Index { index_id }) => { + SearchError::IndexesNotFound { + index_id_patterns: vec![index_id], + } + } + MetastoreError::NotFound(EntityKind::Indexes { index_ids }) => { + SearchError::IndexesNotFound { + index_id_patterns: index_ids, + } + } + _ => SearchError::Internal(metastore_error.to_string()), } } } impl From for SearchError { fn from(join_error: JoinError) -> SearchError { - SearchError::InternalError(format!("Spawned task in root join failed: {join_error}")) + SearchError::Internal(format!("Spawned task in root join failed: {join_error}")) } } diff --git a/quickwit/quickwit-search/src/leaf.rs b/quickwit/quickwit-search/src/leaf.rs index 1a18318665e..910390738ca 100644 --- a/quickwit/quickwit-search/src/leaf.rs +++ b/quickwit/quickwit-search/src/leaf.rs @@ -379,7 +379,7 @@ async fn leaf_search_single_split( }) .await .map_err(|_| { - crate::SearchError::InternalError(format!("Leaf search panicked. split={split_id}")) + crate::SearchError::Internal(format!("Leaf search panicked. split={split_id}")) })??; searcher_context @@ -428,7 +428,7 @@ pub(crate) fn rewrite_start_end_time_bounds( /// `leaf` step of search. /// /// The leaf search collects all kind of information, and returns a set of -/// [PartialHit](quickwit_proto::PartialHit) candidates. The root will be in +/// [PartialHit](quickwit_proto::search::PartialHit) candidates. The root will be in /// charge to consolidate, identify the actual final top hits to display, and /// fetch the actual documents to convert the partial hits into actual Hits. pub async fn leaf_search( diff --git a/quickwit/quickwit-search/src/lib.rs b/quickwit/quickwit-search/src/lib.rs index 77d1b6a4a6b..a6371d9ff3d 100644 --- a/quickwit/quickwit-search/src/lib.rs +++ b/quickwit/quickwit-search/src/lib.rs @@ -135,19 +135,16 @@ async fn list_relevant_splits( if let Some(start_ts) = start_timestamp { query = query.with_time_range_start_gte(start_ts); } - if let Some(end_ts) = end_timestamp { query = query.with_time_range_end_lt(end_ts); } - if let Some(tags_filter) = tags_filter_opt { query = query.with_tags_filter(tags_filter); } - - let split_metas = metastore.list_splits(query).await?; - Ok(split_metas + let splits = metastore.list_splits(query).await?; + Ok(splits .into_iter() - .map(|metadata| metadata.split_metadata) + .map(|split| split.split_metadata) .collect::>()) } diff --git a/quickwit/quickwit-search/src/retry/mod.rs b/quickwit/quickwit-search/src/retry/mod.rs index 3fe6dc1f706..831e0ae2eb3 100644 --- a/quickwit/quickwit-search/src/retry/mod.rs +++ b/quickwit/quickwit-search/src/retry/mod.rs @@ -95,7 +95,7 @@ mod tests { #[test] fn test_should_retry_on_error() { let retry_policy = DefaultRetryPolicy {}; - let response_res = crate::Result::<()>::Err(SearchError::InternalError("test".to_string())); + let response_res = crate::Result::<()>::Err(SearchError::Internal("test".to_string())); retry_policy.retry_request((), &response_res).unwrap() } diff --git a/quickwit/quickwit-search/src/retry/search.rs b/quickwit/quickwit-search/src/retry/search.rs index c46d24371f9..8eb7b470957 100644 --- a/quickwit/quickwit-search/src/retry/search.rs +++ b/quickwit/quickwit-search/src/retry/search.rs @@ -97,9 +97,9 @@ mod tests { fn test_should_retry_on_error() { let retry_policy = LeafSearchRetryPolicy {}; let request = mock_leaf_search_request(); - let response_res = Result::::Err( - SearchError::InternalError("test".to_string()), - ); + let response_res = Result::::Err(SearchError::Internal( + "test".to_string(), + )); retry_policy.retry_request(request, &response_res).unwrap(); } diff --git a/quickwit/quickwit-search/src/root.rs b/quickwit/quickwit-search/src/root.rs index 2376333ffb7..45555f8b0ce 100644 --- a/quickwit/quickwit-search/src/root.rs +++ b/quickwit/quickwit-search/src/root.rs @@ -150,7 +150,7 @@ type TimestampFieldOpt = Option; /// Note: the requirements on timestamp fields and resolved query ASTs can be lifted /// but it adds complexity that does not seem needed right now. fn validate_request_and_build_metadatas( - indexes_metadatas: &[IndexMetadata], + indexes_metadata: &[IndexMetadata], search_request: &SearchRequest, ) -> crate::Result<(TimestampFieldOpt, QueryAst, IndexesMetasForLeafSearch)> { let mut metadatas_for_leaf: HashMap = HashMap::new(); @@ -159,13 +159,13 @@ fn validate_request_and_build_metadatas( let mut query_ast_resolved_opt: Option = None; let mut timestamp_field_opt: Option = None; - for index_metadata in indexes_metadatas.iter() { + for index_metadata in indexes_metadata { let doc_mapper = build_doc_mapper( &index_metadata.index_config.doc_mapping, &index_metadata.index_config.search_settings, ) .map_err(|err| { - SearchError::InternalError(format!("Failed to build doc mapper. Cause: {err}")) + SearchError::Internal(format!("Failed to build doc mapper. Cause: {err}")) })?; let query_ast_resolved_for_index = query_ast .clone() @@ -209,7 +209,7 @@ fn validate_request_and_build_metadatas( let index_metadata_for_leaf_search = IndexMetasForLeafSearch { index_uri: index_metadata.index_uri().clone(), doc_mapper_str: serde_json::to_string(&doc_mapper).map_err(|err| { - SearchError::InternalError(format!("Failed to serialize doc mapper. Cause: {err}")) + SearchError::Internal(format!("Failed to serialize doc mapper. Cause: {err}")) })?, }; metadatas_for_leaf.insert( @@ -219,7 +219,7 @@ fn validate_request_and_build_metadatas( } let query_ast_resolved = query_ast_resolved_opt.ok_or_else(|| { - SearchError::InternalError( + SearchError::Internal( "Resolved query AST must be present. This should never happen.".to_string(), ) })?; @@ -470,21 +470,14 @@ pub(crate) async fn search_partial_hits_phase( merge_collector.merge_fruits(leaf_search_responses) }) .await - .context("failed to merge fruits")? - .map_err(|merge_error: TantivyError| { - crate::SearchError::InternalError(format!("{merge_error}")) - })?; + .context("failed to merge leaf search responses")? + .map_err(|error: TantivyError| crate::SearchError::Internal(error.to_string()))?; debug!(leaf_search_response = ?leaf_search_response, "Merged leaf search response."); if !leaf_search_response.failed_splits.is_empty() { error!(failed_splits = ?leaf_search_response.failed_splits, "Leaf search response contains at least one failed split."); - let errors: String = leaf_search_response - .failed_splits - .iter() - .map(|splits| format!("{splits}")) - .collect::>() - .join(", "); - return Err(SearchError::InternalError(errors)); + let errors: String = leaf_search_response.failed_splits.iter().join(", "); + return Err(SearchError::Internal(errors)); } Ok(leaf_search_response) } @@ -686,23 +679,24 @@ pub async fn root_search( cluster_client: &ClusterClient, ) -> crate::Result { let start_instant = tokio::time::Instant::now(); - let indexes_metadatas = metastore + let indexes_metadata = metastore .list_indexes_metadatas(ListIndexesQuery::IndexIdPatterns( search_request.index_id_patterns.clone(), )) .await?; - if indexes_metadatas.is_empty() { - return Err(SearchError::IndexesDoNotExist { + if indexes_metadata.is_empty() { + return Err(SearchError::IndexesNotFound { index_id_patterns: search_request.index_id_patterns, }); } - let index_uids = indexes_metadatas + let index_uids = indexes_metadata .iter() .map(|index_metadata| index_metadata.index_uid.clone()) .collect_vec(); let (timestamp_field_opt, query_ast_resolved, indexes_metas_for_leaf_search) = - validate_request_and_build_metadatas(&indexes_metadatas, &search_request)?; + validate_request_and_build_metadatas(&indexes_metadata, &search_request)?; search_request.query_ast = serde_json::to_string(&query_ast_resolved)?; + if let Some(timestamp_field) = ×tamp_field_opt { refine_start_end_timestamp_from_ast( &query_ast_resolved, @@ -884,7 +878,7 @@ pub async fn root_list_terms( let doc_mapper = build_doc_mapper(&index_config.doc_mapping, &index_config.search_settings) .map_err(|err| { - SearchError::InternalError(format!("Failed to build doc mapper. Cause: {err}")) + SearchError::Internal(format!("Failed to build doc mapper. Cause: {err}")) })?; let schema = doc_mapper.schema(); @@ -952,7 +946,7 @@ pub async fn root_list_terms( .map(|splits| splits.to_string()) .collect::>() .join(", "); - return Err(SearchError::InternalError(errors)); + return Err(SearchError::Internal(errors)); } // Merging is a cpu-bound task, but probably fast enough to not require @@ -1013,7 +1007,7 @@ async fn assign_client_fetch_docs_jobs( let (index_uid, offsets) = index_uids_and_split_offsets_map .get(&split_id) .ok_or_else(|| { - crate::SearchError::InternalError(format!( + crate::SearchError::Internal(format!( "Received partial hit from an unknown split {split_id}" )) })? @@ -1052,7 +1046,7 @@ pub fn jobs_to_leaf_requests( // Group jobs by index uid. for (index_uid, job_group) in &jobs.into_iter().group_by(|job| job.index_uid.clone()) { let search_index_meta = search_indexes_metadatas.get(&index_uid).ok_or_else(|| { - SearchError::InternalError(format!( + SearchError::Internal(format!( "Received search job for an unknown index {index_uid}. It should never happen." )) })?; @@ -1079,7 +1073,7 @@ pub fn jobs_to_fetch_docs_requests( let index_meta = indexes_metas_for_leaf_search .get(&index_uid) .ok_or_else(|| { - SearchError::InternalError(format!( + SearchError::Internal(format!( "Received search job for an unknown index {index_uid}" )) })?; @@ -2248,7 +2242,7 @@ mod tests { ); mock_search_service.expect_fetch_docs().returning( |_fetch_docs_req: quickwit_proto::search::FetchDocsRequest| { - Err(SearchError::InternalError("mockerr docs".to_string())) + Err(SearchError::Internal("mockerr docs".to_string())) }, ); let searcher_pool = searcher_pool_for_test([("127.0.0.1:1001", mock_search_service)]); @@ -2325,7 +2319,7 @@ mod tests { ); mock_search_service_2.expect_fetch_docs().returning( |_fetch_docs_req: quickwit_proto::search::FetchDocsRequest| { - Err(SearchError::InternalError("mockerr docs".to_string())) + Err(SearchError::Internal("mockerr docs".to_string())) }, ); let searcher_pool = searcher_pool_for_test([ @@ -2392,12 +2386,12 @@ mod tests { let mut mock_search_service_2 = MockSearchService::new(); mock_search_service_2.expect_leaf_search().returning( move |_leaf_search_req: quickwit_proto::search::LeafSearchRequest| { - Err(SearchError::InternalError("mockerr search".to_string())) + Err(SearchError::Internal("mockerr search".to_string())) }, ); mock_search_service_2.expect_fetch_docs().returning( |_fetch_docs_req: quickwit_proto::search::FetchDocsRequest| { - Err(SearchError::InternalError("mockerr docs".to_string())) + Err(SearchError::Internal("mockerr docs".to_string())) }, ); let searcher_pool = searcher_pool_for_test([ diff --git a/quickwit/quickwit-search/src/search_response_rest.rs b/quickwit/quickwit-search/src/search_response_rest.rs index 20212ec9dbb..894bd1ddc71 100644 --- a/quickwit/quickwit-search/src/search_response_rest.rs +++ b/quickwit/quickwit-search/src/search_response_rest.rs @@ -57,7 +57,7 @@ impl TryFrom for SearchResponseRest { let mut snippets = Vec::new(); for hit in search_response.hits { let document: JsonValue = serde_json::from_str(&hit.json).map_err(|err| { - SearchError::InternalError(format!( + SearchError::Internal(format!( "Failed to serialize document `{}` to JSON: `{}`.", truncate_str(&hit.json, 100), err @@ -68,7 +68,7 @@ impl TryFrom for SearchResponseRest { if let Some(snippet_json) = hit.snippet { let snippet_opt: JsonValue = serde_json::from_str(&snippet_json).map_err(|err| { - SearchError::InternalError(format!( + SearchError::Internal(format!( "Failed to serialize snippet `{snippet_json}` to JSON: `{err}`." )) })?; @@ -84,7 +84,7 @@ impl TryFrom for SearchResponseRest { let aggregations_opt = if let Some(aggregation_json) = search_response.aggregation { let aggregation: JsonValue = serde_json::from_str(&aggregation_json) - .map_err(|err| SearchError::InternalError(err.to_string()))?; + .map_err(|err| SearchError::Internal(err.to_string()))?; Some(aggregation) } else { None diff --git a/quickwit/quickwit-search/src/search_stream/leaf.rs b/quickwit/quickwit-search/src/search_stream/leaf.rs index b5b778449c4..ccb441c99c4 100644 --- a/quickwit/quickwit-search/src/search_stream/leaf.rs +++ b/quickwit/quickwit-search/src/search_stream/leaf.rs @@ -140,14 +140,13 @@ async fn leaf_search_stream_single_split( doc_mapper.as_ref(), )?); - let output_format = OutputFormat::from_i32(stream_request.output_format).ok_or_else(|| { - SearchError::InternalError("Invalid output format specified.".to_string()) - })?; + let output_format = OutputFormat::from_i32(stream_request.output_format) + .ok_or_else(|| SearchError::Internal("Invalid output format specified.".to_string()))?; if request_fields.partition_by_fast_field.is_some() && output_format != OutputFormat::ClickHouseRowBinary { - return Err(SearchError::InternalError( + return Err(SearchError::Internal( "Invalid output format specified, only ClickHouseRowBinary is allowed when providing \ a partitioned-by field." .to_string(), @@ -205,9 +204,7 @@ async fn leaf_search_stream_single_split( )?; super::serialize::(&collected_values, &mut buffer, output_format).map_err( |_| { - SearchError::InternalError( - "Error when serializing i64 during export".to_owned(), - ) + SearchError::Internal("Error when serializing i64 during export".to_owned()) }, )?; } @@ -220,9 +217,7 @@ async fn leaf_search_stream_single_split( )?; super::serialize::(&collected_values, &mut buffer, output_format).map_err( |_| { - SearchError::InternalError( - "Error when serializing u64 during export".to_owned(), - ) + SearchError::Internal("Error when serializing u64 during export".to_owned()) }, )?; } @@ -243,9 +238,7 @@ async fn leaf_search_stream_single_split( // We serialize Date as i64 microseconds. super::serialize::(&collected_values_as_micros, &mut buffer, output_format) .map_err(|_| { - SearchError::InternalError( - "Error when serializing i64 during export".to_owned(), - ) + SearchError::Internal("Error when serializing i64 during export".to_owned()) })?; } (Type::I64, Some(Type::I64)) => { @@ -257,9 +250,7 @@ async fn leaf_search_stream_single_split( )?; super::serialize_partitions::(collected_values.as_slice(), &mut buffer) .map_err(|_| { - SearchError::InternalError( - "Error when serializing i64 during export".to_owned(), - ) + SearchError::Internal("Error when serializing i64 during export".to_owned()) })?; } (Type::U64, Some(Type::U64)) => { @@ -271,18 +262,16 @@ async fn leaf_search_stream_single_split( )?; super::serialize_partitions::(collected_values.as_slice(), &mut buffer) .map_err(|_| { - SearchError::InternalError( - "Error when serializing i64 during export".to_owned(), - ) + SearchError::Internal("Error when serializing i64 during export".to_owned()) })?; } (fast_field_type, None) => { - return Err(SearchError::InternalError(format!( + return Err(SearchError::Internal(format!( "Search stream does not support fast field of type `{fast_field_type:?}`." ))); } (fast_field_type, Some(partition_fast_field_type)) => { - return Err(SearchError::InternalError(format!( + return Err(SearchError::Internal(format!( "Search stream does not support the combination of fast field type \ `{fast_field_type:?}` and partition fast field type \ `{partition_fast_field_type:?}`." @@ -293,7 +282,7 @@ async fn leaf_search_stream_single_split( }); let buffer = collect_handle.await.map_err(|_| { error!(split_id = %split.split_id, request_fields=%request_fields, "Failed to collect fast field"); - SearchError::InternalError(format!("Error when collecting fast field values for split {}", split.split_id)) + SearchError::Internal(format!("Error when collecting fast field values for split {}", split.split_id)) })??; Ok(LeafSearchStreamResponse { data: buffer, diff --git a/quickwit/quickwit-search/src/search_stream/root.rs b/quickwit/quickwit-search/src/search_stream/root.rs index e22c2c411f9..2b7890e391b 100644 --- a/quickwit/quickwit-search/src/search_stream/root.rs +++ b/quickwit/quickwit-search/src/search_stream/root.rs @@ -52,7 +52,7 @@ pub async fn root_search_stream( let doc_mapper = build_doc_mapper(&index_config.doc_mapping, &index_config.search_settings) .map_err(|err| { - SearchError::InternalError(format!("Failed to build doc mapper. Cause: {err}")) + SearchError::Internal(format!("Failed to build doc mapper. Cause: {err}")) })?; let query_ast: QueryAst = serde_json::from_str(&search_stream_request.query_ast) @@ -84,7 +84,7 @@ pub async fn root_search_stream( .await?; let doc_mapper_str = serde_json::to_string(&doc_mapper).map_err(|err| { - SearchError::InternalError(format!("Failed to serialize doc mapper: Cause {err}")) + SearchError::Internal(format!("Failed to serialize doc mapper: Cause {err}")) })?; let index_uri: &Uri = &index_config.index_uri; @@ -270,7 +270,7 @@ mod tests { data: b"123".to_vec(), split_id: "split1".to_string(), }))?; - result_sender.send(Err(SearchError::InternalError("error".to_string())))?; + result_sender.send(Err(SearchError::Internal("error".to_string())))?; mock_search_service .expect_leaf_search_stream() .withf(|request| request.split_offsets.len() == 2) // First request. @@ -284,7 +284,7 @@ mod tests { .withf(|request| request.split_offsets.len() == 1) // Retry request on the failed split. .return_once( |_leaf_search_req: quickwit_proto::search::LeafSearchStreamRequest| { - Err(SearchError::InternalError("error".to_string())) + Err(SearchError::Internal("error".to_string())) }, ); // The test will hang on indefinitely if we don't drop the sender. diff --git a/quickwit/quickwit-search/src/service.rs b/quickwit/quickwit-search/src/service.rs index aa2b650f637..476f70e1972 100644 --- a/quickwit/quickwit-search/src/service.rs +++ b/quickwit/quickwit-search/src/service.rs @@ -153,7 +153,7 @@ impl SearchServiceImpl { fn deserialize_doc_mapper(doc_mapper_str: &str) -> crate::Result> { let doc_mapper = serde_json::from_str::>(doc_mapper_str).map_err(|err| { - SearchError::InternalError(format!("Failed to deserialize doc mapper: `{err}`")) + SearchError::Internal(format!("Failed to deserialize doc mapper: `{err}`")) })?; Ok(doc_mapper) } @@ -177,7 +177,7 @@ impl SearchService for SearchServiceImpl { ) -> crate::Result { let search_request = leaf_search_request .search_request - .ok_or_else(|| SearchError::InternalError("No search request.".to_string()))?; + .ok_or_else(|| SearchError::Internal("No search request.".to_string()))?; info!(index=?search_request.index_id_patterns, splits=?leaf_search_request.split_offsets, "leaf_search"); let storage = self .storage_resolver @@ -241,7 +241,7 @@ impl SearchService for SearchServiceImpl { ) -> crate::Result>> { let stream_request = leaf_stream_request .request - .ok_or_else(|| SearchError::InternalError("No search request.".to_string()))?; + .ok_or_else(|| SearchError::Internal("No search request.".to_string()))?; info!(index=?stream_request.index_id, splits=?leaf_stream_request.split_offsets, "leaf_search"); let storage = self .storage_resolver @@ -279,7 +279,7 @@ impl SearchService for SearchServiceImpl { ) -> crate::Result { let search_request = leaf_search_request .list_terms_request - .ok_or_else(|| SearchError::InternalError("No search request.".to_string()))?; + .ok_or_else(|| SearchError::Internal("No search request.".to_string()))?; info!(index=?search_request.index_id, splits=?leaf_search_request.split_offsets, "leaf_search"); let storage = self @@ -328,10 +328,10 @@ pub(crate) async fn scroll( let scroll_key: [u8; 16] = current_scroll.scroll_key(); let payload = cluster_client.get_kv(&scroll_key[..]).await; let payload = - payload.ok_or_else(|| SearchError::InternalError("scroll key not found.".to_string()))?; + payload.ok_or_else(|| SearchError::Internal("scroll key not found.".to_string()))?; let mut scroll_context = ScrollContext::load(&payload) - .map_err(|_| SearchError::InternalError("Corrupted scroll context.".to_string()))?; + .map_err(|_| SearchError::Internal("Corrupted scroll context.".to_string()))?; let end_doc: u64 = start_doc + scroll_context.max_hits_per_page; diff --git a/quickwit/quickwit-search/src/tests.rs b/quickwit/quickwit-search/src/tests.rs index 6eabc9c87bb..fd2886be5ac 100644 --- a/quickwit/quickwit-search/src/tests.rs +++ b/quickwit/quickwit-search/src/tests.rs @@ -1381,7 +1381,7 @@ async fn test_single_node_aggregation_missing_fast_field() { ) .await .unwrap_err(); - let SearchError::InternalError(error_msg) = single_node_error else { + let SearchError::Internal(error_msg) = single_node_error else { panic!(); }; assert!(error_msg.contains("Field \"color\" is not configured as fast field")); diff --git a/quickwit/quickwit-serve/src/delete_task_api/handler.rs b/quickwit/quickwit-serve/src/delete_task_api/handler.rs index 8b34f5a8960..2ca6ffab114 100644 --- a/quickwit/quickwit-serve/src/delete_task_api/handler.rs +++ b/quickwit/quickwit-serve/src/delete_task_api/handler.rs @@ -21,8 +21,8 @@ use std::sync::Arc; use quickwit_config::build_doc_mapper; use quickwit_janitor::error::JanitorError; -use quickwit_metastore::{Metastore, MetastoreError}; -use quickwit_proto::metastore::{DeleteQuery, DeleteTask}; +use quickwit_metastore::Metastore; +use quickwit_proto::metastore::{DeleteQuery, DeleteTask, MetastoreResult}; use quickwit_proto::search::SearchRequest; use quickwit_proto::IndexUid; use quickwit_query::query_ast::{query_ast_from_user_text, QueryAst}; @@ -95,7 +95,7 @@ pub fn get_delete_tasks_handler( pub async fn get_delete_tasks( index_id: String, metastore: Arc, -) -> Result, MetastoreError> { +) -> MetastoreResult> { let index_uid: IndexUid = metastore.index_metadata(&index_id).await?.index_uid; let delete_tasks = metastore.list_delete_tasks(index_uid, 0).await?; Ok(delete_tasks) @@ -140,7 +140,7 @@ pub async fn post_delete_request( .parse_user_query(&[]) .map_err(|err| JanitorError::InvalidDeleteQuery(err.to_string()))?; let query_ast_json = serde_json::to_string(&query_ast).map_err(|_err| { - JanitorError::InternalError("Failed to serialized delete query ast".to_string()) + JanitorError::Internal("Failed to serialized delete query ast".to_string()) })?; let delete_query = DeleteQuery { index_uid: index_uid.to_string(), @@ -151,7 +151,7 @@ pub async fn post_delete_request( let index_config = metadata.into_index_config(); // TODO should it be something else than a JanitorError? let doc_mapper = build_doc_mapper(&index_config.doc_mapping, &index_config.search_settings) - .map_err(|error| JanitorError::InternalError(error.to_string()))?; + .map_err(|error| JanitorError::Internal(error.to_string()))?; let delete_search_request = SearchRequest::try_from(delete_query.clone()) .map_err(|error| JanitorError::InvalidDeleteQuery(error.to_string()))?; diff --git a/quickwit/quickwit-serve/src/elastic_search_api/mod.rs b/quickwit/quickwit-serve/src/elastic_search_api/mod.rs index 1206310f1ec..648ee67493f 100644 --- a/quickwit/quickwit-serve/src/elastic_search_api/mod.rs +++ b/quickwit/quickwit-serve/src/elastic_search_api/mod.rs @@ -168,7 +168,7 @@ mod tests { { Ok(Default::default()) } else { - Err(quickwit_search::SearchError::InternalError( + Err(quickwit_search::SearchError::Internal( "something bad happened".to_string(), )) } @@ -326,7 +326,7 @@ mod tests { { Ok(Default::default()) } else { - Err(quickwit_search::SearchError::InternalError( + Err(quickwit_search::SearchError::Internal( "something bad happened".to_string(), )) } diff --git a/quickwit/quickwit-serve/src/index_api/rest_handler.rs b/quickwit/quickwit-serve/src/index_api/rest_handler.rs index 4f43e32b030..99417ab4a15 100644 --- a/quickwit/quickwit-serve/src/index_api/rest_handler.rs +++ b/quickwit/quickwit-serve/src/index_api/rest_handler.rs @@ -29,9 +29,9 @@ use quickwit_config::{ use quickwit_doc_mapper::{analyze_text, TokenizerConfig}; use quickwit_index_management::{IndexService, IndexServiceError}; use quickwit_metastore::{ - IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, MetastoreError, Split, SplitInfo, - SplitState, + IndexMetadata, ListIndexesQuery, ListSplitsQuery, Metastore, Split, SplitInfo, SplitState, }; +use quickwit_proto::metastore::{EntityKind, MetastoreError, MetastoreResult}; use quickwit_proto::IndexUid; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; @@ -133,7 +133,7 @@ fn get_index_metadata_handler( async fn get_index_metadata( index_id: String, metastore: Arc, -) -> Result { +) -> MetastoreResult { info!(index_id = %index_id, "get-index-metadata"); metastore.index_metadata(&index_id).await } @@ -180,7 +180,7 @@ struct IndexStats { async fn describe_index( index_id: String, metastore: Arc, -) -> Result { +) -> MetastoreResult { let index_metadata = metastore.index_metadata(&index_id).await?; let query = ListSplitsQuery::for_index(index_metadata.index_uid.clone()); let splits = metastore.list_splits(query).await?; @@ -281,7 +281,7 @@ async fn list_splits( index_id: String, list_split_query: ListSplitsQueryParams, metastore: Arc, -) -> Result, MetastoreError> { +) -> MetastoreResult> { let index_uid: IndexUid = metastore.index_metadata(&index_id).await?.index_uid; info!(index_id = %index_id, list_split_query = ?list_split_query, "get-splits"); let mut query = ListSplitsQuery::for_index(index_uid); @@ -335,7 +335,7 @@ async fn mark_splits_for_deletion( index_id: String, splits_for_deletion: SplitsForDeletion, metastore: Arc, -) -> Result<(), MetastoreError> { +) -> MetastoreResult<()> { let index_uid: IndexUid = metastore.index_metadata(&index_id).await?.index_uid; info!(index_id = %index_id, splits_ids = ?splits_for_deletion.split_ids, "mark-splits-for-deletion"); let split_ids: Vec<&str> = splits_for_deletion @@ -372,7 +372,7 @@ fn mark_splits_for_deletion_handler( /// Gets indexes metadata. async fn get_indexes_metadatas( metastore: Arc, -) -> Result, MetastoreError> { +) -> MetastoreResult> { info!("get-indexes-metadatas"); metastore .list_indexes_metadatas(ListIndexesQuery::All) @@ -580,15 +580,17 @@ async fn get_source( index_id: String, source_id: String, metastore: Arc, -) -> Result { +) -> MetastoreResult { info!(index_id = %index_id, source_id = %source_id, "get-source"); let source_config = metastore .index_metadata(&index_id) .await? .sources .get(&source_id) - .ok_or_else(|| MetastoreError::SourceDoesNotExist { - source_id: source_id.to_string(), + .ok_or_else(|| { + MetastoreError::NotFound(EntityKind::Source { + source_id: source_id.to_string(), + }) })? .clone(); Ok(source_config) @@ -622,7 +624,7 @@ async fn reset_source_checkpoint( index_id: String, source_id: String, metastore: Arc, -) -> Result<(), MetastoreError> { +) -> MetastoreResult<()> { let index_uid: IndexUid = metastore.index_metadata(&index_id).await?.index_uid; info!(index_id = %index_id, source_id = %source_id, "reset-checkpoint"); metastore @@ -772,11 +774,8 @@ mod tests { use quickwit_common::uri::Uri; use quickwit_config::{SourceParams, VecSourceParams}; use quickwit_indexing::{mock_split, MockSplitBuilder}; - use quickwit_metastore::{ - metastore_for_test, IndexMetadata, ListIndexesQuery, MetastoreError, MockMetastore, - }; + use quickwit_metastore::{metastore_for_test, IndexMetadata, ListIndexesQuery, MockMetastore}; use quickwit_storage::StorageResolver; - use serde::__private::from_utf8_lossy; use serde_json::Value as JsonValue; use super::*; @@ -856,7 +855,7 @@ mod tests { .with_index_uid(&index_uid) .build()]); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) @@ -925,7 +924,7 @@ mod tests { if list_split_query.index_uids.contains(&index_uid) { return Ok(vec![split_1, split_2]); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) @@ -979,7 +978,7 @@ mod tests { { return Ok(Vec::new()); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) @@ -1017,7 +1016,7 @@ mod tests { { return Ok(()); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) @@ -1473,7 +1472,7 @@ mod tests { .reply(&index_management_handler) .await; assert_eq!(resp.status(), 415); - let body = from_utf8_lossy(resp.body()); + let body = std::str::from_utf8(resp.body()).unwrap(); assert!(body.contains("Unsupported content-type header. Choices are")); } @@ -1498,7 +1497,7 @@ mod tests { .reply(&index_management_handler) .await; assert_eq!(resp.status(), 400); - let body = from_utf8_lossy(resp.body()); + let body = std::str::from_utf8(resp.body()).unwrap(); assert!(body.contains("Field `timestamp` has an unknown type")); Ok(()) } @@ -1522,7 +1521,7 @@ mod tests { .reply(&index_management_handler) .await; assert_eq!(resp.status(), 400); - let body = from_utf8_lossy(resp.body()); + let body = std::str::from_utf8(resp.body()).unwrap(); assert!(body.contains("invalid type: floating point `0.4`")); } { @@ -1535,7 +1534,7 @@ mod tests { .reply(&index_management_handler) .await; assert_eq!(resp.status(), 400); - let body = from_utf8_lossy(resp.body()); + let body = std::str::from_utf8(resp.body()).unwrap(); assert!(body.contains( "Quickwit currently supports multiple pipelines only for GCP PubSub or Kafka \ sources" @@ -1561,9 +1560,9 @@ mod tests { .expect_delete_source() .return_once(|index_uid, source_id| { assert_eq!(index_uid.index_id(), "quickwit-demo-index"); - Err(MetastoreError::SourceDoesNotExist { + Err(MetastoreError::NotFound(EntityKind::Source { source_id: source_id.to_string(), - }) + })) }); let index_service = IndexService::new(Arc::new(metastore), StorageResolver::unconfigured()); let index_management_handler = super::index_management_handlers( @@ -1597,7 +1596,7 @@ mod tests { if index_uid.index_id() == "quickwit-demo-index" && source_id == "source-to-reset" { return Ok(()); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) @@ -1644,7 +1643,7 @@ mod tests { { return Ok(()); } - Err(MetastoreError::InternalError { + Err(MetastoreError::Internal { message: "".to_string(), cause: "".to_string(), }) diff --git a/quickwit/quickwit-serve/src/lib.rs b/quickwit/quickwit-serve/src/lib.rs index 87e2f6f41a2..e8727ae0dfb 100644 --- a/quickwit/quickwit-serve/src/lib.rs +++ b/quickwit/quickwit-serve/src/lib.rs @@ -75,12 +75,13 @@ use quickwit_ingest::{ }; use quickwit_janitor::{start_janitor_service, JanitorService}; use quickwit_metastore::{ - ListIndexesQuery, Metastore, MetastoreError, MetastoreEvent, MetastoreEventPublisher, - MetastoreGrpcClient, MetastoreResolver, RetryingMetastore, + ListIndexesQuery, Metastore, MetastoreEvent, MetastoreEventPublisher, MetastoreGrpcClient, + MetastoreResolver, RetryingMetastore, }; use quickwit_opentelemetry::otlp::{OtlpGrpcLogsService, OtlpGrpcTracesService}; use quickwit_proto::control_plane::ControlPlaneServiceClient; use quickwit_proto::indexing::IndexingServiceClient; +use quickwit_proto::metastore::{EntityKind, MetastoreError}; use quickwit_search::{ create_search_client_from_channel, start_searcher_service, SearchJobPlacer, SearchService, SearchServiceClient, SearcherPool, @@ -224,9 +225,9 @@ pub async fn serve_quickwit( for index_config in [otel_logs_index_config, otel_traces_index_config] { match index_service.create_index(index_config, false).await { Ok(_) - | Err(IndexServiceError::MetastoreError( - MetastoreError::IndexAlreadyExists { .. }, - )) => Ok(()), + | Err(IndexServiceError::Metastore(MetastoreError::AlreadyExists( + EntityKind::Index { .. }, + ))) => Ok(()), Err(error) => Err(error), }?; } diff --git a/quickwit/quickwit-serve/src/search_api/mod.rs b/quickwit/quickwit-serve/src/search_api/mod.rs index 0fea7599b51..847d9b4a092 100644 --- a/quickwit/quickwit-serve/src/search_api/mod.rs +++ b/quickwit/quickwit-serve/src/search_api/mod.rs @@ -98,9 +98,7 @@ mod tests { data: b"123".to_vec(), split_id: "split_1".to_string(), }))?; - result_sender.send(Err(SearchError::InternalError( - "Error on `split2`".to_string(), - )))?; + result_sender.send(Err(SearchError::Internal("Error on `split2`".to_string())))?; mock_search_service .expect_leaf_search_stream() .withf(|request| request.split_offsets.len() == 2) // First request. @@ -114,9 +112,7 @@ mod tests { .withf(|request| request.split_offsets.len() == 1) // Retry request on the failing split. .return_once( |_leaf_search_req: quickwit_proto::search::LeafSearchStreamRequest| { - Err(SearchError::InternalError( - "Error again on `split2`".to_string(), - )) + Err(SearchError::Internal("Error again on `split2`".to_string())) }, ); // The test will hang on indefinitely if we don't drop the sender. diff --git a/quickwit/quickwit-serve/src/search_api/rest_handler.rs b/quickwit/quickwit-serve/src/search_api/rest_handler.rs index 68565b15a1e..0ee68579793 100644 --- a/quickwit/quickwit-serve/src/search_api/rest_handler.rs +++ b/quickwit/quickwit-serve/src/search_api/rest_handler.rs @@ -43,13 +43,13 @@ use crate::{with_arg, BodyFormat}; #[openapi( paths(search_get_handler, search_post_handler, search_stream_handler,), components(schemas( + BodyFormat, + OutputFormat, SearchRequestQueryString, SearchResponseRest, SortBy, SortField, SortOrder, - OutputFormat, - BodyFormat, ),) )] pub struct SearchApi; @@ -93,7 +93,7 @@ impl From for SortBy { }; sort_fields.push(sort_field); } - SortBy { sort_fields } + Self { sort_fields } } } @@ -132,11 +132,11 @@ fn default_max_hits() -> u64 { // I did not find a way to plug it to serde_qs. // Conclusion: the best way I found to reject a user query that contains an empty // string on an mandatory field is this serializer. -fn deserialize_not_empty_string<'de, D>(deserializer: D) -> Result +fn deserialize_non_empty_string<'de, D>(deserializer: D) -> Result where D: Deserializer<'de> { let value = String::deserialize(deserializer)?; if value.is_empty() { - return Err(de::Error::custom("Expected a non empty string field.")); + return Err(de::Error::custom("Expected a non-empty string field.")); } Ok(value) } @@ -373,7 +373,7 @@ struct SearchStreamRequestQueryString { /// If set, restricts search to documents with a `timestamp < end_timestamp``. pub end_timestamp: Option, /// The fast field to extract. - #[serde(deserialize_with = "deserialize_not_empty_string")] + #[serde(deserialize_with = "deserialize_non_empty_string")] pub fast_field: String, /// The requested output format. #[serde(default)] @@ -891,7 +891,7 @@ mod tests { async fn test_rest_search_api_with_index_does_not_exist() -> anyhow::Result<()> { let mut mock_search_service = MockSearchService::new(); mock_search_service.expect_root_search().returning(|_| { - Err(SearchError::IndexesDoNotExist { + Err(SearchError::IndexesNotFound { index_id_patterns: vec!["not-found-index".to_string()], }) }); @@ -912,7 +912,7 @@ mod tests { let mut mock_search_service = MockSearchService::new(); mock_search_service .expect_root_search() - .returning(|_| Err(SearchError::InternalError("ty".to_string()))); + .returning(|_| Err(SearchError::Internal("ty".to_string()))); let rest_search_api_handler = search_handler(mock_search_service); assert_eq!( warp::test::request() @@ -1045,7 +1045,7 @@ mod tests { let parse_error = rejection.find::().unwrap(); assert_eq!( parse_error.to_string(), - "Expected a non empty string field." + "Expected a non-empty string field." ); } diff --git a/quickwit/quickwit-storage/src/error.rs b/quickwit/quickwit-storage/src/error.rs index f5a0b6f7a95..615f64d07f8 100644 --- a/quickwit/quickwit-storage/src/error.rs +++ b/quickwit/quickwit-storage/src/error.rs @@ -36,7 +36,7 @@ pub enum StorageErrorKind { /// A third-party service forbids this operation, or is misconfigured. Service, /// Any generic internal error. - InternalError, + Internal, /// A timeout occured during the operation. Timeout, /// Io error. diff --git a/quickwit/quickwit-storage/src/local_file_storage.rs b/quickwit/quickwit-storage/src/local_file_storage.rs index 3b617e3a977..532a6832d31 100644 --- a/quickwit/quickwit-storage/src/local_file_storage.rs +++ b/quickwit/quickwit-storage/src/local_file_storage.rs @@ -184,7 +184,7 @@ impl Storage for LocalFileStorage { let full_path = self.full_path(path)?; let parent_dir = full_path.parent().ok_or_else(|| { let err = anyhow::anyhow!("No parent directory for {full_path:?}"); - StorageErrorKind::InternalError.with_error(err) + StorageErrorKind::Internal.with_error(err) })?; fs::create_dir_all(parent_dir).await?; @@ -227,7 +227,7 @@ impl Storage for LocalFileStorage { }) .await .map_err(|_| { - StorageErrorKind::InternalError.with_error(anyhow::anyhow!("reading file panicked")) + StorageErrorKind::Internal.with_error(anyhow::anyhow!("reading file panicked")) })? } diff --git a/quickwit/quickwit-storage/src/object_storage/azure_blob_storage.rs b/quickwit/quickwit-storage/src/object_storage/azure_blob_storage.rs index 5c62d91104e..289539bafbd 100644 --- a/quickwit/quickwit-storage/src/object_storage/azure_blob_storage.rs +++ b/quickwit/quickwit-storage/src/object_storage/azure_blob_storage.rs @@ -566,7 +566,7 @@ impl From for StorageError { }, ErrorKind::Io => StorageErrorKind::Io.with_error(err), ErrorKind::Credential => StorageErrorKind::Unauthorized.with_error(err), - _ => StorageErrorKind::InternalError.with_error(err), + _ => StorageErrorKind::Internal.with_error(err), } } } diff --git a/quickwit/quickwit-storage/src/object_storage/error.rs b/quickwit/quickwit-storage/src/object_storage/error.rs index 1f1feb918d9..14a11760f59 100644 --- a/quickwit/quickwit-storage/src/object_storage/error.rs +++ b/quickwit/quickwit-storage/src/object_storage/error.rs @@ -37,14 +37,14 @@ where E: Send + Sync + std::error::Error + 'static + ToStorageErrorKind + Retrya { fn from(error: SdkError) -> StorageError { let error_kind = match &error { - SdkError::ConstructionFailure(_) => StorageErrorKind::InternalError, + SdkError::ConstructionFailure(_) => StorageErrorKind::Internal, SdkError::DispatchFailure(failure) => { if failure.is_io() { StorageErrorKind::Io } else if failure.is_timeout() { StorageErrorKind::Timeout } else { - StorageErrorKind::InternalError + StorageErrorKind::Internal } } SdkError::ResponseError(response_error) => { @@ -52,12 +52,12 @@ where E: Send + Sync + std::error::Error + 'static + ToStorageErrorKind + Retrya match response.status() { StatusCode::NOT_FOUND => StorageErrorKind::NotFound, StatusCode::UNAUTHORIZED => StorageErrorKind::Unauthorized, - _ => StorageErrorKind::InternalError, + _ => StorageErrorKind::Internal, } } SdkError::ServiceError(service_error) => service_error.err().to_storage_error_kind(), SdkError::TimeoutError(_) => StorageErrorKind::Timeout, - _ => StorageErrorKind::InternalError, + _ => StorageErrorKind::Internal, }; let source = anyhow::anyhow!("{}", DisplayErrorContext(error)); error_kind.with_error(source) @@ -106,7 +106,7 @@ impl ToStorageErrorKind for CompleteMultipartUploadError { impl ToStorageErrorKind for AbortMultipartUploadError { fn to_storage_error_kind(&self) -> StorageErrorKind { match self { - AbortMultipartUploadError::NoSuchUpload(_) => StorageErrorKind::InternalError, + AbortMultipartUploadError::NoSuchUpload(_) => StorageErrorKind::Internal, AbortMultipartUploadError::Unhandled(_) => StorageErrorKind::Service, _ => StorageErrorKind::Service, } diff --git a/quickwit/quickwit-storage/src/object_storage/s3_compatible_storage.rs b/quickwit/quickwit-storage/src/object_storage/s3_compatible_storage.rs index e22ee01dfef..1fa7b916ac6 100644 --- a/quickwit/quickwit-storage/src/object_storage/s3_compatible_storage.rs +++ b/quickwit/quickwit-storage/src/object_storage/s3_compatible_storage.rs @@ -320,7 +320,7 @@ impl S3CompatibleObjectStorage { .await? .upload_id .ok_or_else(|| { - StorageErrorKind::InternalError + StorageErrorKind::Internal .with_error(anyhow!("The returned multipart upload id was null.")) })?; Ok(MultipartUploadId(upload_id))