diff --git a/config/templates/gh-archive.yaml b/config/templates/gh-archive.yaml new file mode 100644 index 00000000000..8abe764ddc9 --- /dev/null +++ b/config/templates/gh-archive.yaml @@ -0,0 +1,48 @@ +version: 0.7 + +template_id: gh-archive + +index_id_patterns: + - gh-archive* + +description: Index config template for the GH Archive dataset (gharchive.org) + +priority: 0 + +doc_mapping: + field_mappings: + - name: id + type: text + tokenizer: raw + - name: type + type: text + fast: true + tokenizer: raw + - name: public + type: bool + fast: true + - name: payload + type: json + tokenizer: default + - name: org + type: json + tokenizer: default + - name: repo + type: json + tokenizer: default + - name: actor + type: json + tokenizer: default + - name: other + type: json + tokenizer: default + - name: created_at + type: datetime + fast: true + input_formats: + - rfc3339 + fast_precision: seconds + timestamp_field: created_at + +indexing_settings: + commit_timeout_secs: 10 diff --git a/config/templates/stackoverflow.yaml b/config/templates/stackoverflow.yaml new file mode 100644 index 00000000000..72171dd4454 --- /dev/null +++ b/config/templates/stackoverflow.yaml @@ -0,0 +1,36 @@ +version: 0.7 + +template_id: stackoverflow + +index_id_patterns: + - stackoverflow* + +description: Index config template for the Stackoverflow tutorial (quickwit.io/docs/get-started/quickstart) + +priority: 0 + +doc_mapping: + field_mappings: + - name: title + type: text + tokenizer: default + record: position + stored: true + - name: body + type: text + tokenizer: default + record: position + stored: true + - name: creationDate + type: datetime + fast: true + input_formats: + - rfc3339 + fast_precision: seconds + timestamp_field: creationDate + +search_settings: + default_search_fields: [title, body] + +indexing_settings: + commit_timeout_secs: 10 diff --git a/distribution/lambda/Makefile b/distribution/lambda/Makefile index 39cd723271f..fc4445a034a 100644 --- a/distribution/lambda/Makefile +++ b/distribution/lambda/Makefile @@ -67,14 +67,17 @@ deploy-mock-data: package check-env # address https://github.com/aws/aws-cdk/issues/20060 before-destroy: + mkdir -p cdk.out touch $(INDEXER_PACKAGE_PATH) touch $(SEARCHER_PACKAGE_PATH) destroy-hdfs: before-destroy - cdk destroy -a cdk/app.py HdfsStack + python -c 'from cdk import cli; cli.empty_hdfs_bucket()' + cdk destroy --force -a cdk/app.py HdfsStack destroy-mock-data: before-destroy - cdk destroy -a cdk/app.py MockDataStack + python -c 'from cdk import cli; cli.empty_mock_data_buckets()' + cdk destroy --force -a cdk/app.py MockDataStack clean: rm -rf cdk.out diff --git a/distribution/lambda/cdk/cli.py b/distribution/lambda/cdk/cli.py index ce2fe4cf75d..bfffc5f846a 100644 --- a/distribution/lambda/cdk/cli.py +++ b/distribution/lambda/cdk/cli.py @@ -269,13 +269,16 @@ def get_logs( def download_logs_to_file(request_id: str, function_name: str, invoke_start: float): - with open(f"lambda.{request_id}.log", "w") as f: - for log in get_logs( - function_name, - request_id, - int(invoke_start * 1000), - ): - f.write(log) + try: + with open(f"lambda.{request_id}.log", "w") as f: + for log in get_logs( + function_name, + request_id, + int(invoke_start * 1000), + ): + f.write(log) + except Exception as e: + print(f"Failed to download logs: {e}") def invoke_mock_data_searcher(): @@ -288,11 +291,31 @@ def invoke_mock_data_searcher(): def _clean_s3_bucket(bucket_name: str, prefix: str = ""): + print(f"Cleaning up bucket {bucket_name}/{prefix}...") s3 = session.resource("s3") bucket = s3.Bucket(bucket_name) bucket.objects.filter(Prefix=prefix).delete() +def empty_hdfs_bucket(): + bucket_name = _get_cloudformation_output_value( + app.HDFS_STACK_NAME, hdfs_stack.INDEX_STORE_BUCKET_NAME_EXPORT_NAME + ) + + _clean_s3_bucket(bucket_name) + + +def empty_mock_data_buckets(): + bucket_name = _get_cloudformation_output_value( + app.MOCK_DATA_STACK_NAME, mock_data_stack.INDEX_STORE_BUCKET_NAME_EXPORT_NAME + ) + _clean_s3_bucket(bucket_name) + bucket_name = _get_cloudformation_output_value( + app.MOCK_DATA_STACK_NAME, mock_data_stack.SOURCE_BUCKET_NAME_EXPORT_NAME + ) + _clean_s3_bucket(bucket_name) + + @cache def _git_commit(): return subprocess.run( diff --git a/distribution/lambda/cdk/stacks/examples/mock_data_stack.py b/distribution/lambda/cdk/stacks/examples/mock_data_stack.py index a54018a6d8d..79aa1eb20ef 100644 --- a/distribution/lambda/cdk/stacks/examples/mock_data_stack.py +++ b/distribution/lambda/cdk/stacks/examples/mock_data_stack.py @@ -15,6 +15,8 @@ from ..services.quickwit_service import QuickwitService SEARCHER_FUNCTION_NAME_EXPORT_NAME = "mock-data-searcher-function-name" +INDEX_STORE_BUCKET_NAME_EXPORT_NAME = "mock-data-index-store-bucket-name" +SOURCE_BUCKET_NAME_EXPORT_NAME = "mock-data-source-bucket-name" class Source(Construct): @@ -66,6 +68,12 @@ def __init__( mock_data_bucket.add_object_created_notification( aws_s3_notifications.LambdaDestination(qw_svc.indexer.lambda_function) ) + aws_cdk.CfnOutput( + self, + "source-bucket-name", + value=mock_data_bucket.bucket_name, + export_name=SOURCE_BUCKET_NAME_EXPORT_NAME, + ) class SearchAPI(Construct): @@ -164,6 +172,12 @@ def __init__( api_key=search_api_key, ) + aws_cdk.CfnOutput( + self, + "index-store-bucket-name", + value=qw_svc.bucket.bucket_name, + export_name=INDEX_STORE_BUCKET_NAME_EXPORT_NAME, + ) aws_cdk.CfnOutput( self, "searcher-function-name", diff --git a/docs/guides/e2e-serverless-aws-lambda.md b/docs/guides/e2e-serverless-aws-lambda.md index 0b97481b979..0a46caec7c2 100644 --- a/docs/guides/e2e-serverless-aws-lambda.md +++ b/docs/guides/e2e-serverless-aws-lambda.md @@ -143,7 +143,9 @@ curl -d '{"query":"quantity:>5", "max_hits": 10}' \ --compressed ``` +:::note The index is not created until the first run of the Indexer, so you might need a few minutes before your first search request succeeds. The API Gateway key configuration also takes a minute or two to propagate, so the first requests might receive an authorization error response. +::: Because the JSON query responses are often quite verbose, the Searcher Lambda always compresses them before sending them on the wire. It is crucial to keep this size low, both to avoid hitting the Lambda payload size limit of 6MB and to avoid egress costs at around $0.10/GB. We do this regardless of the `accept-encoding` request header, this is why the `--compressed` flag needs to be set to `curl`. diff --git a/quickwit/Cargo.lock b/quickwit/Cargo.lock index 271e1b325f9..1db06dfbce3 100644 --- a/quickwit/Cargo.lock +++ b/quickwit/Cargo.lock @@ -155,9 +155,9 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7079075b41f533b8c61d2a4d073c4676e1f8b249ff94a393b0595db304e0dd87" +checksum = "2faccea4cc4ab4a667ce676a30e8ec13922a692c99bb8f5b11f1502c72e04220" [[package]] name = "anstyle-parse" @@ -1182,9 +1182,9 @@ dependencies = [ [[package]] name = "census" -version = "0.4.1" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0fafee10a5dd1cffcb5cc560e0d0df8803d7355a2b12272e3557dee57314cb6e" +checksum = "4f4c707c6a209cbe82d10abd08e1ea8995e9ea937d2550646e02798948992be0" [[package]] name = "cfb-mode" @@ -1249,7 +1249,7 @@ dependencies = [ "anyhow", "async-trait", "bytes", - "itertools 0.12.0", + "itertools 0.12.1", "rand 0.8.5", "serde", "tokio", @@ -1259,9 +1259,9 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.32" +version = "0.4.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41daef31d7a747c5c847246f36de49ced6f7403b4cdabc807a97b5cc184cda7a" +checksum = "9f13690e35a5e4ace198e7beea2895d29f3a9cc55015fcebe6336bd2010af9eb" dependencies = [ "android-tzdata", "iana-time-zone", @@ -1727,12 +1727,12 @@ dependencies = [ [[package]] name = "darling" -version = "0.20.3" +version = "0.20.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0209d94da627ab5605dcccf08bb18afa5009cfbef48d8a8b7d7bdbc79be25c5e" +checksum = "da01daa5f6d41c91358398e8db4dde38e292378da1f28300b59ef4732b879454" dependencies = [ - "darling_core 0.20.3", - "darling_macro 0.20.3", + "darling_core 0.20.4", + "darling_macro 0.20.4", ] [[package]] @@ -1751,9 +1751,9 @@ dependencies = [ [[package]] name = "darling_core" -version = "0.20.3" +version = "0.20.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "177e3443818124b357d8e76f53be906d60937f0d3a90773a664fa63fa253e621" +checksum = "f44f6238b948a3c6c3073cdf53bb0c2d5e024ee27e0f35bfe9d556a12395808a" dependencies = [ "fnv", "ident_case", @@ -1776,15 +1776,28 @@ dependencies = [ [[package]] name = "darling_macro" -version = "0.20.3" +version = "0.20.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "836a9bbc7ad63342d6d6e7b815ccab164bc77a2d95d84bc3117a8c0d5c98e2d5" +checksum = "0d2d88bd93979b1feb760a6b5c531ac5ba06bd63e74894c377af02faee07b9cd" dependencies = [ - "darling_core 0.20.3", + "darling_core 0.20.4", "quote", "syn 2.0.48", ] +[[package]] +name = "dashmap" +version = "5.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "978747c1d849a7d2ee5e8adc0159961c48fb7e5db2f06af6723b80123bb53856" +dependencies = [ + "cfg-if", + "hashbrown 0.14.3", + "lock_api", + "once_cell", + "parking_lot_core", +] + [[package]] name = "data-encoding" version = "2.5.0" @@ -3212,9 +3225,9 @@ dependencies = [ [[package]] name = "inventory" -version = "0.3.14" +version = "0.3.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c8573b2b1fb643a372c73b23f4da5f888677feef3305146d68a539250a9bccc7" +checksum = "f958d3d68f4167080a18141e10381e7634563984a537f2a49a30fd8e53ac5767" [[package]] name = "io-lifetimes" @@ -3273,9 +3286,9 @@ dependencies = [ [[package]] name = "itertools" -version = "0.12.0" +version = "0.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0" +checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" dependencies = [ "either", ] @@ -3508,9 +3521,9 @@ dependencies = [ [[package]] name = "libz-sys" -version = "1.1.14" +version = "1.1.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "295c17e837573c8c821dbaeb3cceb3d745ad082f7572191409e69cbc1b3fd050" +checksum = "037731f5d3aaa87a5675e895b63ddff1a87624bc29f77004ea829809654e48f6" dependencies = [ "cc", "libc", @@ -3795,9 +3808,9 @@ dependencies = [ [[package]] name = "lru" -version = "0.12.1" +version = "0.12.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2994eeba8ed550fd9b47a0b38f0242bc3344e496483c6180b69139cc2fa5d1d7" +checksum = "db2c024b41519440580066ba82aab04092b333e09066a5eb86c7c4890df31f22" dependencies = [ "hashbrown 0.14.3", ] @@ -3889,9 +3902,9 @@ checksum = "523dc4f511e55ab87b694dc30d0f820d60906ef06413f93d4d7a1385599cc149" [[package]] name = "memmap2" -version = "0.9.3" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "45fd3a57831bf88bc63f8cebc0cf956116276e97fef3966103e96416209f7c92" +checksum = "fe751422e4a8caa417e13c3ea66452215d7d63e19e604f4980461212f3ae1322" dependencies = [ "libc", ] @@ -4409,9 +4422,9 @@ checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf" [[package]] name = "openssl-src" -version = "300.2.1+3.2.0" +version = "300.2.2+3.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3fe476c29791a5ca0d1273c697e96085bbabbbea2ef7afd5617e78a4b40332d3" +checksum = "8bbfad0063610ac26ee79f7484739e2b07555a75c42453b89263830b5c8103bc" dependencies = [ "cc", ] @@ -4588,7 +4601,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b645dcde5f119c2c454a92d0dfa271a2a3b205da92e4292a68ead4bdbfde1f33" dependencies = [ "heck", - "itertools 0.12.0", + "itertools 0.12.1", "proc-macro2", "proc-macro2-diagnostics", "quote", @@ -4610,7 +4623,7 @@ checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" [[package]] name = "ownedbytes" version = "0.6.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "stable_deref_trait", ] @@ -4839,18 +4852,18 @@ dependencies = [ [[package]] name = "pin-project" -version = "1.1.3" +version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fda4ed1c6c173e3fc7a83629421152e01d7b1f9b7f65fb301e490e8cfc656422" +checksum = "0302c4a0442c456bd56f841aee5c3bfd17967563f6fadc9ceb9f9c23cf3807e0" dependencies = [ "pin-project-internal", ] [[package]] name = "pin-project-internal" -version = "1.1.3" +version = "1.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4359fd9c9171ec6e8c62926d6faaf553a8dc3f64e1507e76da7911b4f6a04405" +checksum = "266c042b60c9c76b8d53061e52b2e0d1116abc57cefc8c5cd671619a56ac3690" dependencies = [ "proc-macro2", "quote", @@ -5463,7 +5476,7 @@ dependencies = [ "futures", "humantime", "indicatif", - "itertools 0.12.0", + "itertools 0.12.1", "numfmt", "once_cell", "openssl-probe", @@ -5513,7 +5526,7 @@ dependencies = [ "async-trait", "chitchat", "futures", - "itertools 0.12.0", + "itertools 0.12.1", "quickwit-common", "quickwit-config", "quickwit-proto", @@ -5590,7 +5603,7 @@ dependencies = [ "hostname", "http 0.2.11", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "num_cpus", "once_cell", "pin-project", @@ -5623,7 +5636,7 @@ dependencies = [ "http 0.2.11", "http-serde 1.1.3", "humantime", - "itertools 0.12.0", + "itertools 0.12.1", "json_comments", "new_string_template", "num_cpus", @@ -5655,7 +5668,7 @@ dependencies = [ "futures", "http 0.2.11", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "mockall", "once_cell", "proptest", @@ -5688,7 +5701,7 @@ name = "quickwit-datetime" version = "0.7.1" dependencies = [ "anyhow", - "itertools 0.12.0", + "itertools 0.12.1", "ouroboros", "serde", "serde_json", @@ -5729,7 +5742,7 @@ dependencies = [ "fnv", "hex", "indexmap 2.1.0", - "itertools 0.12.0", + "itertools 0.12.1", "matches", "mockall", "nom", @@ -5761,7 +5774,7 @@ dependencies = [ "async-trait", "futures", "futures-util", - "itertools 0.12.0", + "itertools 0.12.1", "mockall", "quickwit-common", "quickwit-config", @@ -5809,7 +5822,7 @@ dependencies = [ "google-cloud-gax 0.15.0", "google-cloud-googleapis 0.10.0", "google-cloud-pubsub 0.18.0", - "itertools 0.12.0", + "itertools 0.12.1", "libz-sys", "mockall", "once_cell", @@ -5865,7 +5878,7 @@ dependencies = [ "futures", "http 0.2.11", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "mockall", "mrecordlog", "once_cell", @@ -5899,7 +5912,7 @@ dependencies = [ "chitchat", "futures-util", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "quickwit-actors", "quickwit-cluster", "quickwit-common", @@ -5930,7 +5943,7 @@ dependencies = [ "anyhow", "async-trait", "base64 0.21.7", - "itertools 0.12.0", + "itertools 0.12.1", "once_cell", "prost", "prost-types", @@ -5965,7 +5978,7 @@ dependencies = [ "async-trait", "chrono", "futures", - "itertools 0.12.0", + "itertools 0.12.1", "mockall", "once_cell", "quickwit-actors", @@ -6064,7 +6077,7 @@ dependencies = [ "dotenv", "futures", "http 0.2.11", - "itertools 0.12.0", + "itertools 0.12.1", "md5", "mockall", "once_cell", @@ -6077,11 +6090,13 @@ dependencies = [ "quickwit-storage", "rand 0.8.5", "regex", + "regex-syntax 0.8.2", "sea-query", "sea-query-binder", "serde", "serde_json", "serde_with 3.5.1", + "serial_test", "sqlx", "tempfile", "thiserror", @@ -6221,7 +6236,7 @@ dependencies = [ "futures", "http 0.2.11", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "lru", "mockall", "once_cell", @@ -6275,7 +6290,7 @@ dependencies = [ "http-serde 1.1.3", "humantime", "hyper 0.14.28", - "itertools 0.12.0", + "itertools 0.12.1", "mime_guess", "mockall", "num_cpus", @@ -6584,7 +6599,7 @@ checksum = "b62dbe01f0b06f9d8dc7d49e05a0785f153b00b2c227856282f671e0318c9b15" dependencies = [ "aho-corasick", "memchr", - "regex-automata 0.4.4", + "regex-automata 0.4.5", "regex-syntax 0.8.2", ] @@ -6599,9 +6614,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.4.4" +version = "0.4.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b7fa1134405e2ec9353fd416b17f8dacd46c473d7d3fd1cf202706a14eb792a" +checksum = "5bb987efffd3c6d0d8f5f89510bb458559eab11e4f869acb20bf845e016259cd" dependencies = [ "aho-corasick", "memchr", @@ -7141,9 +7156,9 @@ checksum = "b97ed7a9823b74f99c7742f5336af7be5ecd3eeafcb1507d1fa93347b1d589b0" [[package]] name = "serde" -version = "1.0.195" +version = "1.0.196" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "63261df402c67811e9ac6def069e4786148c4563f4b50fd4bf30aa370d626b02" +checksum = "870026e60fa08c69f064aa766c10f10b1d62db9ccd4d0abb206472bee0ce3b32" dependencies = [ "serde_derive", ] @@ -7160,9 +7175,9 @@ dependencies = [ [[package]] name = "serde_derive" -version = "1.0.195" +version = "1.0.196" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46fe8f8603d81ba86327b23a2e9cdf49e1255fb94a4c5f297f6ee0547178ea2c" +checksum = "33c85360c95e7d137454dc81d9a4ed2b8efd8fbe19cee57357b32b9771fccb67" dependencies = [ "proc-macro2", "quote", @@ -7181,9 +7196,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.111" +version = "1.0.113" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "176e46fa42316f18edd598015a5166857fc835ec732f5215eac6b7bdbf0a84f4" +checksum = "69801b70b1c3dac963ecb03a364ba0ceda9cf60c71cfe475e99864759c8b8a79" dependencies = [ "itoa", "ryu", @@ -7300,7 +7315,7 @@ version = "3.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dbff351eb4b33600a2e138dfa0b10b65a238ea8ff8fb2387c422c5022a3e8298" dependencies = [ - "darling 0.20.3", + "darling 0.20.4", "proc-macro2", "quote", "syn 2.0.48", @@ -7331,6 +7346,31 @@ dependencies = [ "unsafe-libyaml", ] +[[package]] +name = "serial_test" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "953ad9342b3aaca7cb43c45c097dd008d4907070394bd0751a0aa8817e5a018d" +dependencies = [ + "dashmap", + "futures", + "lazy_static", + "log", + "parking_lot", + "serial_test_derive", +] + +[[package]] +name = "serial_test_derive" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b93fb4adc70021ac1b47f7d45e8cc4169baaa7ea58483bc5b721d19a26202212" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.48", +] + [[package]] name = "sha-1" version = "0.10.1" @@ -7535,7 +7575,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ce81b7bd7c4493975347ef60d8c7e8b742d4694f4c49f93e0a12ea263938176c" dependencies = [ - "itertools 0.12.0", + "itertools 0.12.1", "nom", "unicode_categories", ] @@ -7901,7 +7941,7 @@ dependencies = [ [[package]] name = "tantivy" version = "0.22.0-dev" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "aho-corasick", "arc-swap", @@ -7917,7 +7957,7 @@ dependencies = [ "fs4", "futures-util", "htmlescape", - "itertools 0.12.0", + "itertools 0.12.1", "levenshtein_automata", "log", "lru", @@ -7954,7 +7994,7 @@ dependencies = [ [[package]] name = "tantivy-bitpacker" version = "0.5.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "bitpacking", ] @@ -7962,10 +8002,10 @@ dependencies = [ [[package]] name = "tantivy-columnar" version = "0.2.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "fastdivide", - "itertools 0.12.0", + "itertools 0.12.1", "serde", "tantivy-bitpacker", "tantivy-common", @@ -7976,7 +8016,7 @@ dependencies = [ [[package]] name = "tantivy-common" version = "0.6.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "async-trait", "byteorder", @@ -7999,7 +8039,7 @@ dependencies = [ [[package]] name = "tantivy-query-grammar" version = "0.21.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "nom", ] @@ -8007,7 +8047,7 @@ dependencies = [ [[package]] name = "tantivy-sstable" version = "0.2.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "tantivy-bitpacker", "tantivy-common", @@ -8018,16 +8058,17 @@ dependencies = [ [[package]] name = "tantivy-stacker" version = "0.2.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "murmurhash32", + "rand_distr", "tantivy-common", ] [[package]] name = "tantivy-tokenizer-api" version = "0.2.0" -source = "git+https://github.com/quickwit-oss/tantivy/?rev=108f30b#108f30ba235cf4c4154c62d115b3efc487fb0a8b" +source = "git+https://github.com/quickwit-oss/tantivy/?rev=1223a87#1223a87eb2382879ae011fb77dd91b06d81bfeb6" dependencies = [ "serde", ] @@ -8836,9 +8877,9 @@ checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" [[package]] name = "utoipa" -version = "3.5.0" +version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d82b1bc5417102a73e8464c686eef947bdfb99fcdfc0a4f228e81afa9526470a" +checksum = "272ebdfbc99111033031d2f10e018836056e4d2c8e2acda76450ec7974269fa7" dependencies = [ "indexmap 2.1.0", "serde", @@ -8848,9 +8889,9 @@ dependencies = [ [[package]] name = "utoipa-gen" -version = "3.5.0" +version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05d96dcd6fc96f3df9b3280ef480770af1b7c5d14bc55192baa9b067976d920c" +checksum = "d3c9f4d08338c1bfa70dde39412a040a884c6f318b3d09aaaf3437a1e52027fc" dependencies = [ "proc-macro-error", "proc-macro2", @@ -9473,9 +9514,9 @@ checksum = "dff9641d1cd4be8d1a070daf9e3773c5f67e78b4d9d42263020c057706765c04" [[package]] name = "winnow" -version = "0.5.34" +version = "0.5.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7cf47b659b318dccbd69cc4797a39ae128f533dce7902a1096044d1967b9c16" +checksum = "1931d78a9c73861da0134f453bb1f790ce49b2e30eba8410b4b79bac72b46a2d" dependencies = [ "memchr", ] diff --git a/quickwit/Cargo.toml b/quickwit/Cargo.toml index 58a75441488..5e3c10c05ed 100644 --- a/quickwit/Cargo.toml +++ b/quickwit/Cargo.toml @@ -164,6 +164,7 @@ rdkafka = { version = "0.33", default-features = false, features = [ "zstd", ] } regex = "1.10.0" +regex-syntax = "0.8" reqwest = { version = "0.11", default-features = false, features = [ "json", "rustls-tls", @@ -222,7 +223,7 @@ ttl_cache = "0.5" typetag = "0.2" ulid = "1.1" username = "0.2" -utoipa = "3.5.0" +utoipa = "4.2.0" uuid = { version = "1.7", features = ["v4", "serde"] } vrl = { version = "0.8.1", default-features = false, features = [ "compiler", @@ -288,7 +289,7 @@ quickwit-serve = { version = "0.7.1", path = "./quickwit-serve" } quickwit-storage = { version = "0.7.1", path = "./quickwit-storage" } quickwit-telemetry = { version = "0.7.1", path = "./quickwit-telemetry" } -tantivy = { git = "https://github.com/quickwit-oss/tantivy/", rev = "108f30b", default-features = false, features = [ +tantivy = { git = "https://github.com/quickwit-oss/tantivy/", rev = "1223a87", default-features = false, features = [ "lz4-compression", "mmap", "quickwit", diff --git a/quickwit/Makefile b/quickwit/Makefile index f462b176392..6ec296839f2 100644 --- a/quickwit/Makefile +++ b/quickwit/Makefile @@ -11,7 +11,7 @@ fmt: fix: @echo "Running cargo clippy --fix" - @cargo clippy --fix --all-features --allow-dirty --allow-staged + @cargo clippy --workspace --all-features --tests --fix --allow-dirty --allow-staged @$(MAKE) fmt # Usage: diff --git a/quickwit/quickwit-actors/src/mailbox.rs b/quickwit/quickwit-actors/src/mailbox.rs index c0999d82315..90f95e7b8b3 100644 --- a/quickwit/quickwit-actors/src/mailbox.rs +++ b/quickwit/quickwit-actors/src/mailbox.rs @@ -340,13 +340,20 @@ impl Inbox { self.rx.try_recv() } - pub async fn recv_typed_message(&self) -> Option { - while let Ok(mut envelope) = self.rx.recv().await { - if let Some(msg) = envelope.message_typed() { - return Some(msg); + #[cfg(any(test, feature = "testsuite"))] + pub async fn recv_typed_message(&self) -> Result { + loop { + match self.rx.recv().await { + Ok(mut envelope) => { + if let Some(msg) = envelope.message_typed() { + return Ok(msg); + } + } + Err(err) => { + return Err(err); + } } } - None } /// Destroys the inbox and returns the list of pending messages or commands diff --git a/quickwit/quickwit-actors/src/tests.rs b/quickwit/quickwit-actors/src/tests.rs index 6a916b364e1..4e006ce3984 100644 --- a/quickwit/quickwit-actors/src/tests.rs +++ b/quickwit/quickwit-actors/src/tests.rs @@ -23,6 +23,7 @@ use std::ops::Mul; use std::time::Duration; use async_trait::async_trait; +use quickwit_common::new_coolid; use serde::Serialize; use crate::observation::ObservationType; @@ -725,3 +726,49 @@ async fn test_unsync_actor_message() { universe.assert_quit().await; } + +struct FakeActorService { + // We use a cool id to make sure in the test that we get twice the same instance. + cool_id: String, +} + +#[derive(Debug)] +struct GetCoolId; + +impl Actor for FakeActorService { + type ObservableState = (); + + fn observable_state(&self) {} +} + +#[async_trait] +impl Handler for FakeActorService { + type Reply = String; + + async fn handle( + &mut self, + _: GetCoolId, + _ctx: &ActorContext, + ) -> Result { + Ok(self.cool_id.clone()) + } +} + +impl Default for FakeActorService { + fn default() -> Self { + FakeActorService { + cool_id: new_coolid("fake-actor"), + } + } +} + +#[tokio::test] +async fn test_get_or_spawn() { + let universe = Universe::new(); + let mailbox1: Mailbox = universe.get_or_spawn_one(); + let id1 = mailbox1.ask(GetCoolId).await.unwrap(); + let mailbox2: Mailbox = universe.get_or_spawn_one(); + let id2 = mailbox2.ask(GetCoolId).await.unwrap(); + assert_eq!(id1, id2); + universe.assert_quit().await; +} diff --git a/quickwit/quickwit-actors/src/universe.rs b/quickwit/quickwit-actors/src/universe.rs index 04869a351a8..e907feb29e8 100644 --- a/quickwit/quickwit-actors/src/universe.rs +++ b/quickwit/quickwit-actors/src/universe.rs @@ -89,6 +89,16 @@ impl Universe { self.spawn_ctx.registry.get_one::() } + pub fn get_or_spawn_one(&self) -> Mailbox { + if let Some(actor_mailbox) = self.spawn_ctx.registry.get_one::() { + actor_mailbox + } else { + let actor_default = A::default(); + let (mailbox, _handler) = self.spawn_builder().spawn(actor_default); + mailbox + } + } + pub async fn observe(&self, timeout: Duration) -> Vec { self.spawn_ctx.registry.observe(timeout).await } diff --git a/quickwit/quickwit-cli/src/tool.rs b/quickwit/quickwit-cli/src/tool.rs index 81c8c4b3aa4..408543d8aaa 100644 --- a/quickwit/quickwit-cli/src/tool.rs +++ b/quickwit/quickwit-cli/src/tool.rs @@ -29,7 +29,7 @@ use anyhow::{bail, Context}; use clap::{arg, ArgMatches, Command}; use colored::{ColoredString, Colorize}; use humantime::format_duration; -use quickwit_actors::{ActorExitStatus, ActorHandle, Universe}; +use quickwit_actors::{ActorExitStatus, ActorHandle, Mailbox, Universe}; use quickwit_cluster::{ChannelTransport, Cluster, ClusterMember, FailureDetectorConfig}; use quickwit_common::pubsub::EventBroker; use quickwit_common::runtimes::RuntimesConfig; @@ -37,10 +37,12 @@ use quickwit_common::uri::Uri; use quickwit_config::service::QuickwitService; use quickwit_config::{ IndexerConfig, NodeConfig, SourceConfig, SourceInputFormat, SourceParams, TransformConfig, - VecSourceParams, CLI_INGEST_SOURCE_ID, + VecSourceParams, CLI_SOURCE_ID, }; use quickwit_index_management::{clear_cache_directory, IndexService}; -use quickwit_indexing::actors::{IndexingService, MergePipeline, MergePipelineId}; +use quickwit_indexing::actors::{ + IndexingService, MergePipeline, MergePipelineId, MergeSchedulerService, +}; use quickwit_indexing::models::{ DetachIndexingPipeline, DetachMergePipeline, IndexingStatistics, SpawnPipeline, }; @@ -419,7 +421,7 @@ pub async fn local_ingest_docs_cli(args: LocalIngestDocsArgs) -> anyhow::Result< .vrl_script .map(|vrl_script| TransformConfig::new(vrl_script, None)); let source_config = SourceConfig { - source_id: CLI_INGEST_SOURCE_ID.to_string(), + source_id: CLI_SOURCE_ID.to_string(), max_num_pipelines_per_indexer: NonZeroUsize::new(1).expect("1 is always non-zero."), desired_num_pipelines: NonZeroUsize::new(1).expect("1 is always non-zero."), enabled: true, @@ -451,6 +453,8 @@ pub async fn local_ingest_docs_cli(args: LocalIngestDocsArgs) -> anyhow::Result< runtimes_config, &HashSet::from_iter([QuickwitService::Indexer]), )?; + let universe = Universe::new(); + let merge_scheduler_service_mailbox = universe.get_or_spawn_one(); let indexing_server = IndexingService::new( config.node_id.clone(), config.data_dir_path.clone(), @@ -459,12 +463,12 @@ pub async fn local_ingest_docs_cli(args: LocalIngestDocsArgs) -> anyhow::Result< cluster, metastore, None, + merge_scheduler_service_mailbox, IngesterPool::default(), storage_resolver, EventBroker::default(), ) .await?; - let universe = Universe::new(); let (indexing_server_mailbox, indexing_server_handle) = universe.spawn_builder().spawn(indexing_server); let pipeline_id = indexing_server_mailbox @@ -580,10 +584,9 @@ pub async fn merge_cli(args: MergeArgs) -> anyhow::Result<()> { runtimes_config, &HashSet::from_iter([QuickwitService::Indexer]), )?; + let indexer_config = IndexerConfig::default(); let universe = Universe::new(); - let indexer_config = IndexerConfig { - ..Default::default() - }; + let merge_scheduler_service: Mailbox = universe.get_or_spawn_one(); let indexing_server = IndexingService::new( config.node_id, config.data_dir_path, @@ -592,6 +595,7 @@ pub async fn merge_cli(args: MergeArgs) -> anyhow::Result<()> { cluster, metastore, None, + merge_scheduler_service, IngesterPool::default(), storage_resolver, EventBroker::default(), diff --git a/quickwit/quickwit-cli/tests/cli.rs b/quickwit/quickwit-cli/tests/cli.rs index a8dfd672b05..162736fe655 100644 --- a/quickwit/quickwit-cli/tests/cli.rs +++ b/quickwit/quickwit-cli/tests/cli.rs @@ -40,7 +40,7 @@ use quickwit_cli::ClientArgs; 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_config::{SourceInputFormat, CLI_SOURCE_ID}; use quickwit_metastore::{ ListSplitsRequestExt, MetastoreResolver, MetastoreServiceExt, MetastoreServiceStreamSplitsExt, SplitMetadata, SplitState, StageSplitsRequestExt, @@ -284,7 +284,7 @@ async fn test_ingest_docs_cli() { error.root_cause().downcast_ref::().unwrap(), ChecklistError { errors - } if errors.len() == 1 && errors[0].0 == CLI_INGEST_SOURCE_ID + } if errors.len() == 1 && errors[0].0 == CLI_SOURCE_ID )); } @@ -830,7 +830,7 @@ async fn test_garbage_collect_index_cli() { .unwrap(); metastore .stage_splits( - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata.clone()) + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata) .unwrap(), ) .await @@ -945,7 +945,7 @@ async fn test_all_local_index() { let metadata_file_exists = test_env .storage - .exists(&Path::new(&test_env.index_id).join("quickwit.json")) + .exists(&Path::new(&test_env.index_id).join("metastore.json")) .await .unwrap(); assert_eq!(metadata_file_exists, false); diff --git a/quickwit/quickwit-common/src/pubsub.rs b/quickwit/quickwit-common/src/pubsub.rs index c318d95fb93..0ead9318091 100644 --- a/quickwit/quickwit-common/src/pubsub.rs +++ b/quickwit/quickwit-common/src/pubsub.rs @@ -29,6 +29,8 @@ use tracing::warn; use crate::type_map::TypeMap; +const EVENT_SUBSCRIPTION_CALLBACK_TIMEOUT: Duration = Duration::from_secs(10); + pub trait Event: fmt::Debug + Clone + Send + Sync + 'static {} #[async_trait] @@ -138,14 +140,14 @@ impl EventBroker { let subscriber_name = subscription.subscriber_name; let subscriber_clone = subscription.subscriber.clone(); let handle_event_fut = async move { - if tokio::time::timeout(Duration::from_secs(1), async { + if tokio::time::timeout(EVENT_SUBSCRIPTION_CALLBACK_TIMEOUT, async { subscriber_clone.lock().await.handle_event(event).await }) .await .is_err() { let event_name = std::any::type_name::(); - warn!("{}'s handler for {event_name} timed out", subscriber_name); + warn!("{subscriber_name}'s handler for {event_name} timed out"); } }; tokio::spawn(handle_event_fut); diff --git a/quickwit/quickwit-common/src/uri.rs b/quickwit/quickwit-common/src/uri.rs index 78959afc674..97cdfec466e 100644 --- a/quickwit/quickwit-common/src/uri.rs +++ b/quickwit/quickwit-common/src/uri.rs @@ -151,7 +151,7 @@ impl Uri { DATABASE_URI_PATTERN .get_or_init(|| { Regex::new("(?P^.*://.*)(?P:.*@)(?P.*)") - .expect("the regular expression should compile") + .expect("regular expression should compile") }) .replace(&self.uri, "$before:***redacted***@$after") } else { diff --git a/quickwit/quickwit-config/resources/tests/config/quickwit.json b/quickwit/quickwit-config/resources/tests/node_config/quickwit.json similarity index 96% rename from quickwit/quickwit-config/resources/tests/config/quickwit.json rename to quickwit/quickwit-config/resources/tests/node_config/quickwit.json index 828ea718d59..fe4ed3050e5 100644 --- a/quickwit/quickwit-config/resources/tests/config/quickwit.json +++ b/quickwit/quickwit-config/resources/tests/node_config/quickwit.json @@ -48,7 +48,8 @@ "split_store_max_num_bytes": "1T", "split_store_max_num_splits": 10000, "max_concurrent_split_uploads": 8, - "max_merge_write_throughput": "100mb" + "max_merge_write_throughput": "100mb", + "merge_concurrency": 2 }, "ingest_api": { "replication_factor": 2 diff --git a/quickwit/quickwit-config/resources/tests/config/quickwit.toml b/quickwit/quickwit-config/resources/tests/node_config/quickwit.toml similarity index 98% rename from quickwit/quickwit-config/resources/tests/config/quickwit.toml rename to quickwit/quickwit-config/resources/tests/node_config/quickwit.toml index 49f582fc0e0..1c6cb0b57f8 100644 --- a/quickwit/quickwit-config/resources/tests/config/quickwit.toml +++ b/quickwit/quickwit-config/resources/tests/node_config/quickwit.toml @@ -39,6 +39,7 @@ split_store_max_num_bytes = "1T" split_store_max_num_splits = 10_000 max_concurrent_split_uploads = 8 max_merge_write_throughput = "100mb" +merge_concurrency = 2 [ingest_api] replication_factor = 2 diff --git a/quickwit/quickwit-config/resources/tests/config/quickwit.wrongkey.yaml b/quickwit/quickwit-config/resources/tests/node_config/quickwit.wrongkey.yaml similarity index 100% rename from quickwit/quickwit-config/resources/tests/config/quickwit.wrongkey.yaml rename to quickwit/quickwit-config/resources/tests/node_config/quickwit.wrongkey.yaml diff --git a/quickwit/quickwit-config/resources/tests/config/quickwit.yaml b/quickwit/quickwit-config/resources/tests/node_config/quickwit.yaml similarity index 98% rename from quickwit/quickwit-config/resources/tests/config/quickwit.yaml rename to quickwit/quickwit-config/resources/tests/node_config/quickwit.yaml index 4581efed515..9a829d2306f 100644 --- a/quickwit/quickwit-config/resources/tests/config/quickwit.yaml +++ b/quickwit/quickwit-config/resources/tests/node_config/quickwit.yaml @@ -43,6 +43,7 @@ indexer: split_store_max_num_splits: 10000 max_concurrent_split_uploads: 8 max_merge_write_throughput: 100mb + merge_concurrency: 2 ingest_api: replication_factor: 2 diff --git a/quickwit/quickwit-config/src/cluster_config/mod.rs b/quickwit/quickwit-config/src/cluster_config/mod.rs new file mode 100644 index 00000000000..34e7c1bb9a7 --- /dev/null +++ b/quickwit/quickwit-config/src/cluster_config/mod.rs @@ -0,0 +1,42 @@ +// Copyright (C) 2024 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::uri::Uri; + +/// An embryo of a cluster config. +// TODO: Version object. +#[derive(Debug, Clone)] +pub struct ClusterConfig { + pub cluster_id: String, + pub auto_create_indexes: bool, + pub default_index_root_uri: Uri, + pub replication_factor: usize, +} + +impl ClusterConfig { + #[cfg(any(test, feature = "testsuite"))] + pub fn for_test() -> Self { + ClusterConfig { + cluster_id: "test-cluster".to_string(), + auto_create_indexes: false, + default_index_root_uri: Uri::for_test("ram:///indexes"), + replication_factor: 1, + } + } +} diff --git a/quickwit/quickwit-config/src/index_config/mod.rs b/quickwit/quickwit-config/src/index_config/mod.rs index 61cf9fc1ec1..0bc9661ba44 100644 --- a/quickwit/quickwit-config/src/index_config/mod.rs +++ b/quickwit/quickwit-config/src/index_config/mod.rs @@ -25,7 +25,7 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use anyhow::Context; +use anyhow::{ensure, Context}; use bytesize::ByteSize; use chrono::Utc; use cron::Schedule; @@ -225,24 +225,17 @@ pub struct RetentionPolicy { /// Duration of time for which the splits should be retained, expressed in a human-friendly way /// (`1 hour`, `3 days`, `a week`, ...). #[serde(rename = "period")] - retention_period: String, + pub retention_period: String, /// Defines the frequency at which the retention policy is evaluated and applied, expressed in /// a human-friendly way (`hourly`, `daily`, ...) or as a cron expression (`0 0 * * * *`, /// `0 0 0 * * *`). #[serde(default = "RetentionPolicy::default_schedule")] #[serde(rename = "schedule")] - evaluation_schedule: String, + pub evaluation_schedule: String, } impl RetentionPolicy { - pub fn new(retention_period: String, evaluation_schedule: String) -> Self { - Self { - retention_period, - evaluation_schedule, - } - } - fn default_schedule() -> String { "hourly".to_string() } @@ -279,7 +272,7 @@ impl RetentionPolicy { Ok(duration) } - fn validate(&self) -> anyhow::Result<()> { + pub(super) fn validate(&self) -> anyhow::Result<()> { self.retention_period()?; self.evaluation_schedule()?; Ok(()) @@ -310,7 +303,7 @@ pub struct IndexConfig { pub doc_mapping: DocMapping, pub indexing_settings: IndexingSettings, pub search_settings: SearchSettings, - pub retention_policy: Option, + pub retention_policy_opt: Option, } impl IndexConfig { @@ -395,7 +388,7 @@ impl IndexConfig { doc_mapping, indexing_settings, search_settings, - retention_policy: Default::default(), + retention_policy_opt: Default::default(), } } } @@ -462,10 +455,10 @@ impl TestableForRegression for IndexConfig { timestamp_field: Some("timestamp".to_string()), tokenizers: vec![tokenizer], }; - let retention_policy = Some(RetentionPolicy::new( - "90 days".to_string(), - "daily".to_string(), - )); + let retention_policy = Some(RetentionPolicy { + retention_period: "90 days".to_string(), + evaluation_schedule: "daily".to_string(), + }); let stable_log_config = StableLogMergePolicyConfig { merge_factor: 9, max_merge_factor: 11, @@ -491,12 +484,12 @@ impl TestableForRegression for IndexConfig { index_uri: Uri::for_test("s3://quickwit-indexes/my-index"), doc_mapping, indexing_settings, - retention_policy, + retention_policy_opt: retention_policy, search_settings, } } - fn test_equality(&self, other: &Self) { + fn assert_equality(&self, other: &Self) { assert_eq!(self.index_id, other.index_id); assert_eq!(self.index_uri, other.index_uri); assert_eq!( @@ -542,6 +535,33 @@ pub fn build_doc_mapper( Ok(Arc::new(builder.try_build()?)) } +/// Validates the objects that make up an index configuration. This is a "free" function as opposed +/// to a method on `IndexConfig` so we can reuse it for validating index templates. +pub(super) fn validate_index_config( + doc_mapping: &DocMapping, + indexing_settings: &IndexingSettings, + search_settings: &SearchSettings, + retention_policy_opt: &Option, +) -> anyhow::Result<()> { + // Note: this needs a deep refactoring to separate the doc mapping configuration, + // and doc mapper implementations. + // TODO see if we should store the byproducton the IndexConfig. + build_doc_mapper(doc_mapping, search_settings)?; + + indexing_settings.merge_policy.validate()?; + indexing_settings.resources.validate()?; + + if let Some(retention_policy) = retention_policy_opt { + retention_policy.validate()?; + + ensure!( + doc_mapping.timestamp_field.is_some(), + "retention policy requires a timestamp field, but indexing settings do not declare one" + ); + } + Ok(()) +} + #[cfg(test)] mod tests { @@ -595,7 +615,7 @@ mod tests { evaluation_schedule: "daily".to_string(), }; assert_eq!( - index_config.retention_policy.unwrap(), + index_config.retention_policy_opt.unwrap(), expected_retention_policy ); assert!(index_config.doc_mapping.store_source); diff --git a/quickwit/quickwit-config/src/index_config/serialize.rs b/quickwit/quickwit-config/src/index_config/serialize.rs index a293d5bd01d..76d5c26ef9a 100644 --- a/quickwit/quickwit-config/src/index_config/serialize.rs +++ b/quickwit/quickwit-config/src/index_config/serialize.rs @@ -22,9 +22,10 @@ use quickwit_common::uri::Uri; use serde::{Deserialize, Serialize}; use tracing::info; +use super::validate_index_config; use crate::{ - build_doc_mapper, validate_identifier, ConfigFormat, DocMapping, IndexConfig, IndexingSettings, - RetentionPolicy, SearchSettings, + validate_identifier, ConfigFormat, DocMapping, IndexConfig, IndexingSettings, RetentionPolicy, + SearchSettings, }; /// Alias for the latest serialization format. @@ -58,7 +59,7 @@ pub fn load_index_config_from_user_config( ) -> anyhow::Result { let versioned_index_config: VersionedIndexConfig = config_format.parse(config_content)?; let index_config_for_serialization: IndexConfigForSerialization = versioned_index_config.into(); - index_config_for_serialization.validate_and_build(Some(default_index_root_uri)) + index_config_for_serialization.build_and_validate(Some(default_index_root_uri)) } impl IndexConfigForSerialization { @@ -66,55 +67,43 @@ impl IndexConfigForSerialization { &self, default_index_root_uri_opt: Option<&Uri>, ) -> anyhow::Result { - if let Some(index_uri) = self.index_uri.as_ref() { + if let Some(index_uri) = &self.index_uri { return Ok(index_uri.clone()); } let default_index_root_uri = default_index_root_uri_opt.context("missing `index_uri`")?; let index_uri: Uri = default_index_root_uri.join(&self.index_id) .context("failed to create default index URI. this should never happen! please, report on https://github.com/quickwit-oss/quickwit/issues")?; info!( - index_id = %self.index_id, - index_uri = %index_uri, - "Index config does not specify `index_uri`, falling back to default value.", + index_id=%self.index_id, + index_uri=%index_uri, + "index config does not specify `index_uri`, falling back to default value", ); Ok(index_uri) } - pub fn validate_and_build( + pub fn build_and_validate( self, default_index_root_uri: Option<&Uri>, ) -> anyhow::Result { - validate_identifier("Index ID", &self.index_id)?; + validate_identifier("index", &self.index_id)?; let index_uri = self.index_uri_or_fallback_to_default(default_index_root_uri)?; - if let Some(retention_policy) = &self.retention_policy { - retention_policy.validate()?; - - if self.doc_mapping.timestamp_field.is_none() { - anyhow::bail!( - "failed to validate index config. the retention policy requires a timestamp \ - field, but the indexing settings do not declare one" - ); - } - } - - // Note: this needs a deep refactoring to separate the doc mapping configuration, - // and doc mapper implementations. - // TODO see if we should store the byproducton the IndexConfig. - build_doc_mapper(&self.doc_mapping, &self.search_settings)?; - - self.indexing_settings.merge_policy.validate()?; - self.indexing_settings.resources.validate()?; - - Ok(IndexConfig { + let index_config = IndexConfig { index_id: self.index_id, index_uri, doc_mapping: self.doc_mapping, indexing_settings: self.indexing_settings, search_settings: self.search_settings, - retention_policy: self.retention_policy, - }) + retention_policy_opt: self.retention_policy_opt, + }; + validate_index_config( + &index_config.doc_mapping, + &index_config.indexing_settings, + &index_config.search_settings, + &index_config.retention_policy_opt, + )?; + Ok(index_config) } } @@ -129,7 +118,7 @@ impl TryFrom for IndexConfig { fn try_from(versioned_index_config: VersionedIndexConfig) -> anyhow::Result { match versioned_index_config { - VersionedIndexConfig::V0_7(v0_6) => v0_6.validate_and_build(None), + VersionedIndexConfig::V0_7(v0_6) => v0_6.build_and_validate(None), } } } @@ -148,7 +137,7 @@ pub struct IndexConfigV0_7 { pub search_settings: SearchSettings, #[serde(rename = "retention")] #[serde(default)] - pub retention_policy: Option, + pub retention_policy_opt: Option, } impl From for IndexConfigV0_7 { @@ -159,7 +148,7 @@ impl From for IndexConfigV0_7 { doc_mapping: index_config.doc_mapping, indexing_settings: index_config.indexing_settings, search_settings: index_config.search_settings, - retention_policy: index_config.retention_policy, + retention_policy_opt: index_config.retention_policy_opt, } } } @@ -201,7 +190,7 @@ mod test { invalid_index_config.indexing_settings.merge_policy = MergePolicyConfig::StableLog(stable_log_merge_policy_config); let validation_err = invalid_index_config - .validate_and_build(None) + .build_and_validate(None) .unwrap_err() .to_string(); assert_eq!( @@ -216,15 +205,15 @@ mod test { // Not yet invalid, but we modify it right after this. let mut invalid_index_config: IndexConfigForSerialization = minimal_index_config_for_serialization(); - invalid_index_config.retention_policy = Some(RetentionPolicy { + invalid_index_config.retention_policy_opt = Some(RetentionPolicy { retention_period: "90 days".to_string(), evaluation_schedule: "hourly".to_string(), }); let validation_err = invalid_index_config - .validate_and_build(None) + .build_and_validate(None) .unwrap_err() .to_string(); - assert!(validation_err.contains("the retention policy requires a timestamp field")); + assert!(validation_err.contains("retention policy requires a timestamp field")); } #[test] diff --git a/quickwit/quickwit-config/src/index_template/mod.rs b/quickwit/quickwit-config/src/index_template/mod.rs new file mode 100644 index 00000000000..70c00c1ebf9 --- /dev/null +++ b/quickwit/quickwit-config/src/index_template/mod.rs @@ -0,0 +1,290 @@ +// Copyright (C) 2024 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 . + +mod serialize; + +use anyhow::ensure; +use quickwit_common::uri::Uri; +use quickwit_proto::types::IndexId; +use serde::{Deserialize, Serialize}; +pub use serialize::VersionedIndexTemplate; + +use crate::index_config::validate_index_config; +use crate::{ + validate_identifier, validate_index_id_pattern, DocMapping, IndexConfig, IndexingSettings, + RetentionPolicy, SearchSettings, TestableForRegression, +}; + +pub type IndexTemplateId = String; +pub type IndexIdPattern = String; + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[serde(into = "VersionedIndexTemplate")] +#[serde(from = "VersionedIndexTemplate")] +pub struct IndexTemplate { + pub template_id: IndexTemplateId, + pub index_id_patterns: Vec, + #[serde(default)] + pub index_root_uri: Option, + #[serde(default)] + pub priority: usize, + #[serde(default)] + pub description: Option, + pub doc_mapping: DocMapping, + #[serde(default)] + pub indexing_settings: IndexingSettings, + #[serde(default)] + pub search_settings: SearchSettings, + #[serde(rename = "retention")] + #[serde(default)] + pub retention_policy_opt: Option, +} + +impl IndexTemplate { + pub fn apply_template( + &self, + index_id: IndexId, + default_index_root_uri: &Uri, + ) -> anyhow::Result { + let index_uri = self + .index_root_uri + .as_ref() + .unwrap_or(default_index_root_uri) + .join(&index_id)?; + + let index_config = IndexConfig { + index_id, + index_uri, + doc_mapping: self.doc_mapping.clone(), + indexing_settings: self.indexing_settings.clone(), + search_settings: self.search_settings.clone(), + retention_policy_opt: self.retention_policy_opt.clone(), + }; + Ok(index_config) + } + + pub fn validate(&self) -> anyhow::Result<()> { + validate_identifier("template", &self.template_id)?; + + ensure!( + !self.index_id_patterns.is_empty(), + "`index_id_patterns` must not be empty" + ); + for index_id_pattern in &self.index_id_patterns { + validate_index_id_pattern(index_id_pattern, true)?; + } + validate_index_config( + &self.doc_mapping, + &self.indexing_settings, + &self.search_settings, + &self.retention_policy_opt, + )?; + Ok(()) + } + + #[cfg(any(test, feature = "testsuite"))] + pub fn for_test(template_id: &str, index_id_patterns: &[&str], priority: usize) -> Self { + let index_id_patterns: Vec = index_id_patterns + .iter() + .map(|pattern| pattern.to_string()) + .collect(); + + let doc_mapping_json = r#"{ + "field_mappings": [ + { + "name": "ts", + "type": "datetime", + "fast": true + }, + { + "name": "message", + "type": "json" + } + ], + "timestamp_field": "ts" + }"#; + let doc_mapping: DocMapping = serde_json::from_str(doc_mapping_json).unwrap(); + + IndexTemplate { + template_id: template_id.to_string(), + index_root_uri: Some(Uri::for_test("ram:///indexes")), + index_id_patterns, + priority, + description: Some("Test description.".to_string()), + doc_mapping, + indexing_settings: IndexingSettings::default(), + search_settings: SearchSettings::default(), + retention_policy_opt: None, + } + } +} + +impl TestableForRegression for IndexTemplate { + fn sample_for_regression() -> Self { + let template_id = "test-template".to_string(); + let index_id_patterns = vec![ + "test-index-foo*".to_string(), + "-test-index-foobar".to_string(), + ]; + + let doc_mapping_json = r#"{ + "field_mappings": [ + { + "name": "ts", + "type": "datetime", + "fast": true + }, + { + "name": "message", + "type": "json" + } + ], + "timestamp_field": "ts" + }"#; + let doc_mapping: DocMapping = serde_json::from_str(doc_mapping_json).unwrap(); + + IndexTemplate { + template_id: template_id.to_string(), + index_root_uri: Some(Uri::for_test("ram:///indexes")), + index_id_patterns, + priority: 100, + description: Some("Test description.".to_string()), + doc_mapping, + indexing_settings: IndexingSettings::default(), + search_settings: SearchSettings::default(), + retention_policy_opt: Some(RetentionPolicy { + retention_period: "42 days".to_string(), + evaluation_schedule: "daily".to_string(), + }), + } + } + + fn assert_equality(&self, other: &Self) { + assert_eq!(self, other); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_index_template_serde() { + let index_template_yaml = r#" + version: 0.7 + + template_id: test-template + index_id_patterns: + - test-index-* + - -test-index-foo + description: Test description. + priority: 100 + doc_mapping: + field_mappings: + - name: ts + type: datetime + fast: true + - name: message + type: json + timestamp_field: ts + "#; + let index_template: IndexTemplate = serde_yaml::from_str(index_template_yaml).unwrap(); + assert_eq!(index_template.template_id, "test-template"); + assert_eq!(index_template.index_id_patterns.len(), 2); + assert_eq!( + index_template.index_id_patterns, + ["test-index-*", "-test-index-foo"] + ); + assert_eq!(index_template.priority, 100); + assert_eq!(index_template.description.unwrap(), "Test description."); + assert_eq!(index_template.doc_mapping.timestamp_field.unwrap(), "ts"); + } + + #[test] + fn test_index_template_apply() { + let mut index_template = IndexTemplate::for_test("test-template", &["test-index-*"], 0); + + index_template.indexing_settings = IndexingSettings { + commit_timeout_secs: 42, + ..Default::default() + }; + index_template.search_settings = SearchSettings { + default_search_fields: vec!["message".to_string()], + }; + index_template.retention_policy_opt = Some(RetentionPolicy { + retention_period: "42 days".to_string(), + evaluation_schedule: "hourly".to_string(), + }); + let default_index_root_uri = Uri::for_test("s3://test-bucket/indexes"); + + let index_config = index_template + .apply_template("test-index".to_string(), &default_index_root_uri) + .unwrap(); + + assert_eq!(index_config.index_id, "test-index"); + assert_eq!(index_config.index_uri, "ram:///indexes/test-index"); + + assert_eq!(index_config.doc_mapping.timestamp_field.unwrap(), "ts"); + assert_eq!(index_config.indexing_settings.commit_timeout_secs, 42); + assert_eq!( + index_config.search_settings.default_search_fields, + ["message"] + ); + let retention_policy = index_config.retention_policy_opt.unwrap(); + assert_eq!(retention_policy.retention_period, "42 days"); + assert_eq!(retention_policy.evaluation_schedule, "hourly"); + + index_template.index_root_uri = None; + + let index_config = index_template + .apply_template("test-index".to_string(), &default_index_root_uri) + .unwrap(); + + assert_eq!(index_config.index_id, "test-index"); + assert_eq!( + index_config.index_uri, + "s3://test-bucket/indexes/test-index" + ); + } + + #[test] + fn test_index_template_validate() { + let index_template = IndexTemplate::for_test("", &[], 0); + let error = index_template.validate().unwrap_err(); + assert!(error.to_string().contains("template ID `` is invalid")); + + let index_template = IndexTemplate::for_test("test-template", &[], 0); + let error = index_template.validate().unwrap_err(); + assert!(error.to_string().contains("empty")); + + let index_template = IndexTemplate::for_test("test-template", &[""], 0); + let error = index_template.validate().unwrap_err(); + assert!(error.to_string().contains("index ID pattern `` is invalid")); + + let mut index_template = IndexTemplate::for_test("test-template", &["test-index-*"], 0); + index_template.retention_policy_opt = Some(RetentionPolicy { + retention_period: "".to_string(), + evaluation_schedule: "".to_string(), + }); + let error = index_template.validate().unwrap_err(); + assert!(error + .to_string() + .contains("failed to parse retention period")); + } +} diff --git a/quickwit/quickwit-config/src/index_template/serialize.rs b/quickwit/quickwit-config/src/index_template/serialize.rs new file mode 100644 index 00000000000..c0ed491b4f9 --- /dev/null +++ b/quickwit/quickwit-config/src/index_template/serialize.rs @@ -0,0 +1,97 @@ +// Copyright (C) 2024 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::uri::Uri; +use serde::{Deserialize, Serialize}; + +use super::{IndexIdPattern, IndexTemplate, IndexTemplateId}; +use crate::{DocMapping, IndexingSettings, RetentionPolicy, SearchSettings}; + +#[derive(Clone, Debug, Serialize, Deserialize, utoipa::ToSchema)] +#[serde(tag = "version")] +pub enum VersionedIndexTemplate { + #[serde(rename = "0.7")] + V0_7(IndexTemplateV0_7), +} + +#[derive(Clone, Debug, Serialize, Deserialize, utoipa::ToSchema)] +#[serde(deny_unknown_fields)] +pub struct IndexTemplateV0_7 { + pub template_id: IndexTemplateId, + pub index_id_patterns: Vec, + #[serde(default)] + pub index_root_uri: Option, + #[serde(default)] + pub priority: usize, + #[serde(default)] + pub description: Option, + pub doc_mapping: DocMapping, + #[serde(default)] + pub indexing_settings: IndexingSettings, + #[serde(default)] + pub search_settings: SearchSettings, + #[serde(default)] + pub retention: Option, +} + +impl From for IndexTemplate { + fn from(versioned_index_template: VersionedIndexTemplate) -> Self { + match versioned_index_template { + VersionedIndexTemplate::V0_7(v0_7) => v0_7.into(), + } + } +} + +impl From for VersionedIndexTemplate { + fn from(index_template: IndexTemplate) -> Self { + VersionedIndexTemplate::V0_7(index_template.into()) + } +} + +impl From for IndexTemplate { + fn from(index_template_v0_7: IndexTemplateV0_7) -> Self { + IndexTemplate { + template_id: index_template_v0_7.template_id, + index_id_patterns: index_template_v0_7.index_id_patterns, + index_root_uri: index_template_v0_7.index_root_uri, + priority: index_template_v0_7.priority, + description: index_template_v0_7.description, + doc_mapping: index_template_v0_7.doc_mapping, + indexing_settings: index_template_v0_7.indexing_settings, + search_settings: index_template_v0_7.search_settings, + retention_policy_opt: index_template_v0_7.retention, + } + } +} + +impl From for IndexTemplateV0_7 { + fn from(index_template: IndexTemplate) -> Self { + IndexTemplateV0_7 { + template_id: index_template.template_id, + index_id_patterns: index_template.index_id_patterns, + index_root_uri: index_template.index_root_uri, + priority: index_template.priority, + description: index_template.description, + doc_mapping: index_template.doc_mapping, + indexing_settings: index_template.indexing_settings, + search_settings: index_template.search_settings, + retention: index_template.retention_policy_opt, + } + } +} diff --git a/quickwit/quickwit-config/src/lib.rs b/quickwit/quickwit-config/src/lib.rs index 7e74fc617b2..84278f71064 100644 --- a/quickwit/quickwit-config/src/lib.rs +++ b/quickwit/quickwit-config/src/lib.rs @@ -21,15 +21,17 @@ use std::str::FromStr; -use anyhow::{bail, Context}; +use anyhow::{bail, ensure, Context}; use json_comments::StripComments; use once_cell::sync::Lazy; use quickwit_common::net::is_valid_hostname; use quickwit_common::uri::Uri; use regex::Regex; +mod cluster_config; mod config_value; mod index_config; +mod index_template; pub mod merge_policy_config; mod metastore_config; mod node_config; @@ -39,6 +41,7 @@ mod source_config; mod storage_config; mod templating; +pub use cluster_config::ClusterConfig; // We export that one for backward compatibility. // See #2048 use index_config::serialize::{IndexConfigV0_7, VersionedIndexConfig}; @@ -53,10 +56,11 @@ pub use source_config::{ load_source_config_from_user_config, FileSourceParams, GcpPubSubSourceParams, KafkaSourceParams, KinesisSourceParams, PulsarSourceAuth, PulsarSourceParams, RegionOrEndpoint, SourceConfig, SourceInputFormat, SourceParams, TransformConfig, VecSourceParams, - VoidSourceParams, CLI_INGEST_SOURCE_ID, INGEST_API_SOURCE_ID, INGEST_V2_SOURCE_ID, + VoidSourceParams, CLI_SOURCE_ID, INGEST_API_SOURCE_ID, INGEST_V2_SOURCE_ID, }; use tracing::warn; +pub use crate::index_template::{IndexTemplate, IndexTemplateId, VersionedIndexTemplate}; use crate::merge_policy_config::{ ConstWriteAmplificationMergePolicyConfig, MergePolicyConfig, StableLogMergePolicyConfig, }; @@ -103,18 +107,17 @@ pub use crate::storage_config::{ /// Schema used for the OpenAPI generation which are apart of this crate. pub struct ConfigApiSchemas; -/// Checks whether an identifier conforms to Quickwit object naming conventions. +/// Checks whether an identifier conforms to Quickwit naming conventions. pub fn validate_identifier(label: &str, value: &str) -> anyhow::Result<()> { static IDENTIFIER_REGEX: Lazy = Lazy::new(|| { Regex::new(r"^[a-zA-Z][a-zA-Z0-9-_\.]{2,254}$").expect("regular expression should compile") }); - if IDENTIFIER_REGEX.is_match(value) { - return Ok(()); - } - bail!( - "{label} identifier `{value}` is invalid. identifiers must match the following regular \ + ensure!( + IDENTIFIER_REGEX.is_match(value), + "{label} ID `{value}` is invalid: identifiers must match the following regular \ expression: `^[a-zA-Z][a-zA-Z0-9-_\\.]{{2,254}}$`" ); + Ok(()) } /// Checks whether an index ID pattern conforms to Quickwit conventions. @@ -188,7 +191,7 @@ impl ConfigFormat { pub fn sniff_from_uri(uri: &Uri) -> anyhow::Result { let extension_str: &str = uri.extension().with_context(|| { format!( - "failed to read config file `{uri}`: file extension is missing. supported file \ + "failed to parse config file `{uri}`: file extension is missing. supported file \ formats and extensions are JSON (.json), TOML (.toml), and YAML (.yaml or .yml)" ) })?; @@ -207,26 +210,26 @@ impl ConfigFormat { warn!(version_value=?version_value, "`version` is supposed to be a string"); *version_value = JsonValue::String(version_number.to_string()); } - serde_json::from_value(json_value).context("failed to read JSON file") + serde_json::from_value(json_value).context("failed to parse JSON file") } ConfigFormat::Toml => { let payload_str = std::str::from_utf8(payload) .context("configuration file contains invalid UTF-8 characters")?; let mut toml_value: toml::Value = - toml::from_str(payload_str).context("failed to read TOML file")?; + toml::from_str(payload_str).context("failed to parse TOML file")?; let version_value = toml_value.get_mut("version").context("missing version")?; if let Some(version_number) = version_value.as_integer() { warn!(version_value=?version_value, "`version` is supposed to be a string"); *version_value = toml::Value::String(version_number.to_string()); let reserialized = toml::to_string(version_value) .context("failed to reserialize toml config")?; - toml::from_str(&reserialized).context("failed to read TOML file") + toml::from_str(&reserialized).context("failed to parse TOML file") } else { - toml::from_str(payload_str).context("failed to read TOML file") + toml::from_str(payload_str).context("failed to parse TOML file") } } ConfigFormat::Yaml => { - serde_yaml::from_slice(payload).context("failed to read YAML file") + serde_yaml::from_slice(payload).context("failed to parse YAML file") } } } @@ -249,8 +252,12 @@ impl FromStr for ConfigFormat { } pub trait TestableForRegression: Serialize + DeserializeOwned { + /// Produces an instance of `Self` whose serialization output will be tested against future + /// versions of the format for backward compatibility. fn sample_for_regression() -> Self; - fn test_equality(&self, other: &Self); + + /// Asserts that `self` and `other` are equal. It must panic if they are not. + fn assert_equality(&self, other: &Self); } #[cfg(test)] @@ -260,22 +267,22 @@ mod tests { #[test] fn test_validate_identifier() { - validate_identifier("Cluster ID", "").unwrap_err(); - validate_identifier("Cluster ID", "-").unwrap_err(); - validate_identifier("Cluster ID", "_").unwrap_err(); - validate_identifier("Cluster ID", "f").unwrap_err(); - validate_identifier("Cluster ID", "fo").unwrap_err(); - validate_identifier("Cluster ID", "_fo").unwrap_err(); - validate_identifier("Cluster ID", "_foo").unwrap_err(); - validate_identifier("Cluster ID", ".foo.bar").unwrap_err(); - validate_identifier("Cluster ID", "foo").unwrap(); - validate_identifier("Cluster ID", "f-_").unwrap(); - validate_identifier("Index ID", "foo.bar").unwrap(); + validate_identifier("cluster", "").unwrap_err(); + validate_identifier("cluster", "-").unwrap_err(); + validate_identifier("cluster", "_").unwrap_err(); + validate_identifier("cluster", "f").unwrap_err(); + validate_identifier("cluster", "fo").unwrap_err(); + validate_identifier("cluster", "_fo").unwrap_err(); + validate_identifier("cluster", "_foo").unwrap_err(); + validate_identifier("cluster", ".foo.bar").unwrap_err(); + validate_identifier("cluster", "foo").unwrap(); + validate_identifier("cluster", "f-_").unwrap(); + validate_identifier("index", "foo.bar").unwrap(); - assert!(validate_identifier("Cluster ID", "foo!") + assert!(validate_identifier("cluster", "foo!") .unwrap_err() .to_string() - .contains("Cluster ID identifier `foo!` is invalid.")); + .contains("cluster ID `foo!` is invalid")); } #[test] diff --git a/quickwit/quickwit-config/src/node_config/mod.rs b/quickwit/quickwit-config/src/node_config/mod.rs index 8f2ed5d5c0e..4f788ea7c48 100644 --- a/quickwit/quickwit-config/src/node_config/mod.rs +++ b/quickwit/quickwit-config/src/node_config/mod.rs @@ -96,6 +96,10 @@ pub struct IndexerConfig { /// does not starve indexing itself (as it is a latency sensitive operation). #[serde(default)] pub max_merge_write_throughput: Option, + /// Maximum number of merge or delete operation that can be executed concurrently. + /// (defaults to num_cpu / 2). + #[serde(default = "IndexerConfig::default_merge_concurrency")] + pub merge_concurrency: NonZeroUsize, /// Enables the OpenTelemetry exporter endpoint to ingest logs and traces via the OpenTelemetry /// Protocol (OTLP). #[serde(default = "IndexerConfig::default_enable_otlp_endpoint")] @@ -134,6 +138,10 @@ impl IndexerConfig { 1_000 } + pub fn default_merge_concurrency() -> NonZeroUsize { + NonZeroUsize::new(num_cpus::get() / 2).unwrap_or(NonZeroUsize::new(1).unwrap()) + } + fn default_cpu_capacity() -> CpuCapacity { CpuCapacity::one_cpu_thread() * (num_cpus::get() as u32) } @@ -149,6 +157,7 @@ impl IndexerConfig { max_concurrent_split_uploads: 4, cpu_capacity: PIPELINE_FULL_CAPACITY * 4u32, max_merge_write_throughput: None, + merge_concurrency: NonZeroUsize::new(3).unwrap(), }; Ok(indexer_config) } @@ -163,6 +172,7 @@ impl Default for IndexerConfig { split_store_max_num_splits: Self::default_split_store_max_num_splits(), max_concurrent_split_uploads: Self::default_max_concurrent_split_uploads(), cpu_capacity: Self::default_cpu_capacity(), + merge_concurrency: Self::default_merge_concurrency(), max_merge_write_throughput: None, } } @@ -476,6 +486,23 @@ mod tests { "1500m" ); } + { + let indexer_config: IndexerConfig = + serde_yaml::from_str(r#"merge_concurrency: 5"#).unwrap(); + assert_eq!( + indexer_config.merge_concurrency, + NonZeroUsize::new(5).unwrap() + ); + let indexer_config_json = serde_json::to_value(&indexer_config).unwrap(); + assert_eq!( + indexer_config_json + .get("merge_concurrency") + .unwrap() + .as_u64() + .unwrap(), + 5 + ); + } { let indexer_config: IndexerConfig = serde_yaml::from_str(r#"cpu_capacity: 1500m"#).unwrap(); diff --git a/quickwit/quickwit-config/src/node_config/serialize.rs b/quickwit/quickwit-config/src/node_config/serialize.rs index 240ba88ef03..b315d949a51 100644 --- a/quickwit/quickwit-config/src/node_config/serialize.rs +++ b/quickwit/quickwit-config/src/node_config/serialize.rs @@ -317,17 +317,14 @@ impl NodeConfigBuilder { } fn validate(node_config: &NodeConfig) -> anyhow::Result<()> { - validate_identifier("Cluster ID", &node_config.cluster_id)?; + validate_identifier("cluster", &node_config.cluster_id)?; validate_node_id(&node_config.node_id)?; if node_config.cluster_id == DEFAULT_CLUSTER_ID { - warn!( - "cluster ID is not set, falling back to default value: `{}`", - DEFAULT_CLUSTER_ID - ); + warn!("cluster ID is not set, falling back to default value `{DEFAULT_CLUSTER_ID}`",); } if node_config.peer_seeds.is_empty() { - warn!("peer seed list is empty"); + warn!("peer seeds are empty"); } Ok(()) } @@ -455,7 +452,7 @@ pub fn node_config_for_test() -> NodeConfig { mod tests { use std::env; use std::net::Ipv4Addr; - use std::num::NonZeroU64; + use std::num::{NonZeroU64, NonZeroUsize}; use std::path::Path; use bytesize::ByteSize; @@ -466,7 +463,7 @@ mod tests { fn get_config_filepath(config_filename: &str) -> String { format!( - "{}/resources/tests/config/{}", + "{}/resources/tests/node_config/{}", env!("CARGO_MANIFEST_DIR"), config_filename ) @@ -554,6 +551,7 @@ mod tests { split_store_max_num_bytes: ByteSize::tb(1), split_store_max_num_splits: 10_000, max_concurrent_split_uploads: 8, + merge_concurrency: NonZeroUsize::new(2).unwrap(), cpu_capacity: IndexerConfig::default_cpu_capacity(), enable_cooperative_indexing: false, max_merge_write_throughput: Some(ByteSize::mb(100)), diff --git a/quickwit/quickwit-config/src/source_config/mod.rs b/quickwit/quickwit-config/src/source_config/mod.rs index e55e83a05b9..5b83dbd66e6 100644 --- a/quickwit/quickwit-config/src/source_config/mod.rs +++ b/quickwit/quickwit-config/src/source_config/mod.rs @@ -37,7 +37,7 @@ use serialize::VersionedSourceConfig; use crate::{enable_ingest_v2, TestableForRegression}; /// Reserved source ID for the `quickwit index ingest` CLI command. -pub const CLI_INGEST_SOURCE_ID: &str = "_ingest-cli-source"; +pub const CLI_SOURCE_ID: &str = "_ingest-cli-source"; /// Reserved source ID used for Quickwit ingest API. pub const INGEST_API_SOURCE_ID: &str = "_ingest-api-source"; @@ -46,11 +46,8 @@ pub const INGEST_API_SOURCE_ID: &str = "_ingest-api-source"; /// (this is for ingest v2) pub const INGEST_V2_SOURCE_ID: &str = "_ingest-source"; -pub const RESERVED_SOURCE_IDS: &[&str] = &[ - CLI_INGEST_SOURCE_ID, - INGEST_API_SOURCE_ID, - INGEST_V2_SOURCE_ID, -]; +pub const RESERVED_SOURCE_IDS: &[&str] = + &[CLI_SOURCE_ID, INGEST_API_SOURCE_ID, INGEST_V2_SOURCE_ID]; #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)] #[serde(into = "VersionedSourceConfig")] @@ -125,40 +122,40 @@ impl SourceConfig { .expect("`SourceParams` should be JSON serializable") } - /// Creates an ingest source v2. - pub fn ingest_v2_default() -> Self { + /// Creates the default CLI source config. The CLI source ingests data from stdin. + pub fn cli() -> Self { Self { - source_id: INGEST_V2_SOURCE_ID.to_string(), + source_id: CLI_SOURCE_ID.to_string(), max_num_pipelines_per_indexer: NonZeroUsize::new(1).expect("1 should be non-zero"), desired_num_pipelines: NonZeroUsize::new(1).expect("1 should be non-zero"), - enabled: enable_ingest_v2(), - source_params: SourceParams::Ingest, + enabled: true, + source_params: SourceParams::IngestCli, transform_config: None, input_format: SourceInputFormat::Json, } } - /// Creates the default ingest-api source config. - pub fn ingest_api_default() -> Self { + /// Creates a native Quickwit ingest source. The ingest source ingests data from an ingester. + pub fn ingest_v2() -> Self { Self { - source_id: INGEST_API_SOURCE_ID.to_string(), + source_id: INGEST_V2_SOURCE_ID.to_string(), max_num_pipelines_per_indexer: NonZeroUsize::new(1).expect("1 should be non-zero"), desired_num_pipelines: NonZeroUsize::new(1).expect("1 should be non-zero"), - enabled: true, - source_params: SourceParams::IngestApi, + enabled: enable_ingest_v2(), + source_params: SourceParams::Ingest, transform_config: None, input_format: SourceInputFormat::Json, } } - /// Creates the default cli-ingest source config. - pub fn cli_ingest_source() -> Self { + /// Creates the default ingest-api source config. + pub fn ingest_api_default() -> Self { Self { - source_id: CLI_INGEST_SOURCE_ID.to_string(), + source_id: INGEST_API_SOURCE_ID.to_string(), max_num_pipelines_per_indexer: NonZeroUsize::new(1).expect("1 should be non-zero"), desired_num_pipelines: NonZeroUsize::new(1).expect("1 should be non-zero"), enabled: true, - source_params: SourceParams::IngestCli, + source_params: SourceParams::IngestApi, transform_config: None, input_format: SourceInputFormat::Json, } @@ -199,7 +196,7 @@ impl TestableForRegression for SourceConfig { } } - fn test_equality(&self, other: &Self) { + fn assert_equality(&self, other: &Self) { assert_eq!(self, other); } } diff --git a/quickwit/quickwit-config/src/templating.rs b/quickwit/quickwit-config/src/templating.rs index c346c22635d..83443df9270 100644 --- a/quickwit/quickwit-config/src/templating.rs +++ b/quickwit/quickwit-config/src/templating.rs @@ -26,12 +26,12 @@ use once_cell::sync::Lazy; use regex::Regex; use tracing::debug; -// Matches ${value} if value is in format of: -// ENV_VAR or ENV_VAR:DEFAULT +// Matches `${value}` if value is formatted as: +// `ENV_VAR` or `ENV_VAR:DEFAULT` // Ignores whitespaces in curly braces static TEMPLATE_ENV_VAR_CAPTURE: Lazy = Lazy::new(|| { Regex::new(r"\$\{\s*([A-Za-z0-9_]+)\s*(?::\-\s*([\S]+)\s*)?}") - .expect("The regular expression should compile.") + .expect("regular expression should compile") }); pub fn render_config(config_content: &[u8]) -> Result { @@ -40,31 +40,31 @@ pub fn render_config(config_content: &[u8]) -> Result { let mut values = HashMap::new(); - for (line_no, line_res) in config_content.lines().enumerate() { - let line = line_res?; + for (line_no, line_result) in config_content.lines().enumerate() { + let line = line_result?; for captures in TEMPLATE_ENV_VAR_CAPTURE.captures_iter(&line) { let env_var_key = captures .get(1) - .expect("Captures should always have at least one match.") + .expect("captures should always have at least one match") .as_str(); let substitution_value = { if line.trim_start().starts_with('#') { debug!( env_var_name=%env_var_key, - "Config file line #{line_no} is commented out, skipping." + "config file line #{line_no} is commented out, skipping" ); // This line is commented out, return the line as is. captures .get(0) - .expect("The 0th capture should aways be set.") + .expect("0th capture should aways be set") .as_str() .to_string() } else if let Ok(env_var_value) = std::env::var(env_var_key) { debug!( env_var_name=%env_var_key, env_var_value=%env_var_value, - "Environment variable is set, substituting with environment variable value." + "environment variable is set, substituting with environment variable value" ); env_var_value } else if let Some(default_match) = captures.get(2) { @@ -72,7 +72,7 @@ pub fn render_config(config_content: &[u8]) -> Result { debug!( env_var_name=%env_var_key, default_value=%default_value, - "Environment variable is not set, substituting with default value." + "environment variable is not set, substituting with default value" ); default_value } else { diff --git a/quickwit/quickwit-control-plane/Cargo.toml b/quickwit/quickwit-control-plane/Cargo.toml index 7b486862377..03f76f79ef6 100644 --- a/quickwit/quickwit-control-plane/Cargo.toml +++ b/quickwit/quickwit-control-plane/Cargo.toml @@ -14,6 +14,7 @@ anyhow = { workspace = true } async-trait = { workspace = true } dyn-clone = { workspace = true } fnv = { workspace = true } +futures = { workspace = true } http = { workspace = true } hyper = { workspace = true } itertools = { workspace = true } diff --git a/quickwit/quickwit-control-plane/src/control_plane.rs b/quickwit/quickwit-control-plane/src/control_plane.rs index eebf521f3c2..8c65ba46bce 100644 --- a/quickwit/quickwit-control-plane/src/control_plane.rs +++ b/quickwit/quickwit-control-plane/src/control_plane.rs @@ -25,25 +25,29 @@ use std::time::Duration; use anyhow::Context; use async_trait::async_trait; use fnv::FnvHashSet; +use futures::stream::FuturesUnordered; +use futures::StreamExt; use quickwit_actors::{ Actor, ActorContext, ActorExitStatus, ActorHandle, Handler, Mailbox, Supervisor, Universe, WeakMailbox, }; use quickwit_common::pubsub::EventSubscriber; -use quickwit_config::SourceConfig; +use quickwit_common::uri::Uri; +use quickwit_common::Progress; +use quickwit_config::{ClusterConfig, IndexConfig, IndexTemplate, SourceConfig}; use quickwit_ingest::{IngesterPool, LocalShardsUpdate}; use quickwit_metastore::IndexMetadata; use quickwit_proto::control_plane::{ ControlPlaneError, ControlPlaneResult, GetDebugStateRequest, GetDebugStateResponse, - GetOrCreateOpenShardsRequest, GetOrCreateOpenShardsResponse, PhysicalIndexingPlanEntry, - ShardTableEntry, + GetOrCreateOpenShardsRequest, GetOrCreateOpenShardsResponse, GetOrCreateOpenShardsSubrequest, + PhysicalIndexingPlanEntry, ShardTableEntry, }; use quickwit_proto::indexing::ShardPositionsUpdate; use quickwit_proto::metastore::{ - serde_utils as metastore_serde_utils, AddSourceRequest, CreateIndexRequest, - CreateIndexResponse, DeleteIndexRequest, DeleteShardsRequest, DeleteShardsSubrequest, - DeleteSourceRequest, EmptyResponse, MetastoreError, MetastoreService, MetastoreServiceClient, - ToggleSourceRequest, + serde_utils, AddSourceRequest, CreateIndexRequest, CreateIndexResponse, DeleteIndexRequest, + DeleteShardsRequest, DeleteShardsSubrequest, DeleteSourceRequest, EmptyResponse, + FindIndexTemplateMatchesRequest, IndexTemplateMatch, MetastoreError, MetastoreResult, + MetastoreService, MetastoreServiceClient, ToggleSourceRequest, }; use quickwit_proto::types::{IndexUid, NodeId, ShardId, SourceUid}; use serde::Serialize; @@ -72,8 +76,7 @@ struct ControlPlanLoop; struct RebuildPlan; pub struct ControlPlane { - metastore: MetastoreServiceClient, - model: ControlPlaneModel, + cluster_config: ClusterConfig, // The control plane state is split into to independent functions, that we naturally isolated // code wise and state wise. // @@ -83,6 +86,8 @@ pub struct ControlPlane { // the different ingesters. indexing_scheduler: IndexingScheduler, ingest_controller: IngestController, + metastore: MetastoreServiceClient, + model: ControlPlaneModel, rebuild_plan_debouncer: Debouncer, } @@ -95,26 +100,29 @@ impl fmt::Debug for ControlPlane { impl ControlPlane { pub fn spawn( universe: &Universe, - cluster_id: String, + cluster_config: ClusterConfig, self_node_id: NodeId, indexer_pool: IndexerPool, ingester_pool: IngesterPool, metastore: MetastoreServiceClient, - replication_factor: usize, ) -> (Mailbox, ActorHandle>) { universe.spawn_builder().supervise_fn(move || { let indexing_scheduler = IndexingScheduler::new( - cluster_id.clone(), + cluster_config.cluster_id.clone(), self_node_id.clone(), indexer_pool.clone(), ); - let ingest_controller = - IngestController::new(metastore.clone(), ingester_pool.clone(), replication_factor); + let ingest_controller = IngestController::new( + metastore.clone(), + ingester_pool.clone(), + cluster_config.replication_factor, + ); ControlPlane { - model: Default::default(), - metastore: metastore.clone(), + cluster_config: cluster_config.clone(), indexing_scheduler, ingest_controller, + metastore: metastore.clone(), + model: Default::default(), rebuild_plan_debouncer: Debouncer::new(REBUILD_PLAN_COOLDOWN_PERIOD), } }) @@ -160,12 +168,70 @@ impl Actor for ControlPlane { } impl ControlPlane { - /// Rebuilds the indexing plan. - /// - /// This method includes debouncing logic. Every call will be followed by a cooldown period. - fn rebuild_plan_debounced(&mut self, ctx: &ActorContext) { - self.rebuild_plan_debouncer - .self_send_with_cooldown::(ctx); + async fn auto_create_indexes( + &mut self, + subrequests: &[GetOrCreateOpenShardsSubrequest], + progress: &Progress, + ) -> ControlPlaneResult<()> { + if !self.cluster_config.auto_create_indexes { + return Ok(()); + } + let mut index_ids = Vec::new(); + + for subrequest in subrequests { + if self.model.index_uid(&subrequest.index_id).is_none() { + index_ids.push(subrequest.index_id.clone()); + } + } + if index_ids.is_empty() { + return Ok(()); + } + let find_index_template_matches_request = FindIndexTemplateMatchesRequest { index_ids }; + let find_index_template_matches_response = progress + .protect_future( + self.metastore + .find_index_template_matches(find_index_template_matches_request), + ) + .await?; + + let mut create_index_futures = FuturesUnordered::new(); + + for index_template_match in find_index_template_matches_response.matches { + // TODO: It's a bit brutal to fail the entire operation if applying a single index + // template fails. We should return a partial failure instead for the subrequest. I + // want to do so in an upcoming refactor where the `GetOrCreateOpenShardsRequest` will + // be processed in multiple steps in a dedicated workbench. + let index_config = apply_index_template_match( + index_template_match, + &self.cluster_config.default_index_root_uri, + )?; + let index_config_json = serde_utils::to_json_str(&index_config)?; + + let source_configs_json = vec![ + serde_utils::to_json_str(&SourceConfig::ingest_api_default())?, + serde_utils::to_json_str(&SourceConfig::ingest_v2())?, + serde_utils::to_json_str(&SourceConfig::cli())?, + ]; + let create_index_request = CreateIndexRequest { + index_config_json, + source_configs_json, + }; + let create_index_future = { + let mut metastore = self.metastore.clone(); + async move { metastore.create_index(create_index_request).await } + }; + create_index_futures.push(create_index_future); + } + while let Some(create_index_response_result) = + progress.protect_future(create_index_futures.next()).await + { + // Same here. + let create_index_response = create_index_response_result?; + let index_metadata: IndexMetadata = + serde_utils::from_json_str(&create_index_response.index_metadata_json)?; + self.model.add_index(index_metadata); + } + Ok(()) } /// Deletes a set of shards from the metastore and the control plane model. @@ -175,7 +241,7 @@ impl ControlPlane { &mut self, source_uid: &SourceUid, shards: &[ShardId], - ctx: &ActorContext, + progress: &Progress, ) -> anyhow::Result<()> { let delete_shards_subrequest = DeleteShardsSubrequest { index_uid: source_uid.index_uid.to_string(), @@ -192,12 +258,11 @@ impl ControlPlane { // This is because deleting shards is done in reaction to an event // and we do not really have the freedom to return an error to a caller like for other // calls: there is no caller. - self.metastore - .delete_shards(delete_shards_request) + progress + .protect_future(self.metastore.delete_shards(delete_shards_request)) .await - .context("failed to delete shards in metastore")?; + .context("failed to delete shards from metastore")?; self.model.delete_shards(source_uid, shards); - self.rebuild_plan_debounced(ctx); Ok(()) } @@ -231,6 +296,15 @@ impl ControlPlane { physical_index_plan, } } + + /// Rebuilds the indexing plan. + /// + /// This method includes some debouncing logic. Every call will be followed by a cooldown + /// period. + fn rebuild_plan_debounced(&mut self, ctx: &ActorContext) { + self.rebuild_plan_debouncer + .self_send_with_cooldown::(ctx); + } } #[async_trait] @@ -277,8 +351,13 @@ impl Handler for ControlPlane { if shard_ids_to_close.is_empty() { return Ok(()); } - self.delete_shards(&shard_positions_update.source_uid, &shard_ids_to_close, ctx) - .await?; + self.delete_shards( + &shard_positions_update.source_uid, + &shard_ids_to_close, + ctx.progress(), + ) + .await?; + self.rebuild_plan_debounced(ctx); Ok(()) } } @@ -343,7 +422,7 @@ fn convert_metastore_error( } else { // If the metastore transaction may have been executed, we need to restart the control plane // so that it gets resynced with the metastore state. - error!(err=?metastore_error, transaction_outcome="maybe-executed", "metastore error"); + error!(error=?metastore_error, transaction_outcome="maybe-executed", "metastore error"); crate::metrics::CONTROL_PLANE_METRICS .metastore_error_maybe_executed .inc(); @@ -362,28 +441,26 @@ impl Handler for ControlPlane { request: CreateIndexRequest, ctx: &ActorContext, ) -> Result { - let index_config = match metastore_serde_utils::from_json_str(&request.index_config_json) { - Ok(index_config) => index_config, - Err(error) => { - return Ok(Err(ControlPlaneError::from(error))); - } - }; - let index_uid: IndexUid = match self.metastore.create_index(request).await { - Ok(response) => response.index_uid.into(), + let response = match self.metastore.create_index(request).await { + Ok(response) => response, Err(metastore_error) => return convert_metastore_error(metastore_error), }; - let index_metadata: IndexMetadata = - IndexMetadata::new_with_index_uid(index_uid.clone(), index_config); - + match serde_utils::from_json_str(&response.index_metadata_json) { + Ok(index_metadata) => index_metadata, + Err(serde_error) => { + error!(error=?serde_error, "failed to deserialize index metadata"); + return Err(ActorExitStatus::from(anyhow::anyhow!(serde_error))); + } + }; + // Now, create index can also add sources to support creating indexes automatically from + // index and source config templates. + let should_rebuild_plan = !index_metadata.sources.is_empty(); self.model.add_index(index_metadata); - self.rebuild_plan_debounced(ctx); - - let response = CreateIndexResponse { - index_uid: index_uid.into(), - }; - // We do not need to inform the indexing scheduler as there are no shards at this point. + if should_rebuild_plan { + self.rebuild_plan_debounced(ctx); + } Ok(Ok(response)) } } @@ -408,7 +485,7 @@ impl Handler for ControlPlane { let ingester_needing_resync: BTreeSet = self .model .list_shards_for_index(&index_uid) - .flat_map(|shard_entry| shard_entry.ingester_nodes()) + .flat_map(|shard_entry| shard_entry.ingesters()) .collect(); self.model.delete_index(&index_uid); @@ -438,7 +515,7 @@ impl Handler for ControlPlane { ) -> Result { let index_uid: IndexUid = request.index_uid.clone().into(); let source_config: SourceConfig = - match metastore_serde_utils::from_json_str(&request.source_config_json) { + match serde_utils::from_json_str(&request.source_config_json) { Ok(source_config) => source_config, Err(error) => { return Ok(Err(ControlPlaneError::from(error))); @@ -479,13 +556,11 @@ impl Handler for ControlPlane { if let Err(error) = self.metastore.toggle_source(request).await { return Ok(Err(ControlPlaneError::from(error))); }; + let mutation_occured = self.model.toggle_source(&index_uid, &source_id, enable)?; - let has_changed = self.model.toggle_source(&index_uid, &source_id, enable)?; - - if has_changed { + if mutation_occured { self.rebuild_plan_debounced(ctx); } - Ok(Ok(EmptyResponse {})) } } @@ -520,7 +595,7 @@ impl Handler for ControlPlane { let ingester_needing_resync: BTreeSet = if let Some(shards) = self.model.list_shards_for_source(&source_uid) { shards - .flat_map(|shard_entry| shard_entry.ingester_nodes()) + .flat_map(|shard_entry| shard_entry.ingesters()) .collect() } else { BTreeSet::new() @@ -548,6 +623,12 @@ impl Handler for ControlPlane { request: GetOrCreateOpenShardsRequest, ctx: &ActorContext, ) -> Result { + if let Err(control_plane_error) = self + .auto_create_indexes(&request.subrequests, ctx.progress()) + .await + { + return Ok(Err(control_plane_error)); + } let response = match self .ingest_controller .get_or_create_open_shards(request, &mut self.model, ctx.progress()) @@ -633,11 +714,28 @@ impl EventSubscriber for ControlPlaneEventSubscriber { } } +fn apply_index_template_match( + index_template_match: IndexTemplateMatch, + default_index_root_uri: &Uri, +) -> MetastoreResult { + let index_template: IndexTemplate = + serde_utils::from_json_str(&index_template_match.index_template_json)?; + let index_config = index_template + .apply_template(index_template_match.index_id, default_index_root_uri) + .map_err(|error| MetastoreError::Internal { + message: "failed to apply index template".to_string(), + cause: error.to_string(), + })?; + Ok(index_config) +} + #[cfg(test)] mod tests { use mockall::Sequence; use quickwit_actors::{AskError, Observe, SupervisorMetrics}; - use quickwit_config::{IndexConfig, SourceParams, INGEST_V2_SOURCE_ID}; + use quickwit_config::{ + IndexConfig, SourceParams, CLI_SOURCE_ID, INGEST_API_SOURCE_ID, INGEST_V2_SOURCE_ID, + }; use quickwit_indexing::IndexingService; use quickwit_metastore::{ CreateIndexRequestExt, IndexMetadata, ListIndexesMetadataResponseExt, @@ -647,8 +745,9 @@ mod tests { use quickwit_proto::ingest::ingester::{IngesterServiceClient, RetainShardsResponse}; use quickwit_proto::ingest::{Shard, ShardState}; use quickwit_proto::metastore::{ - DeleteShardsResponse, EntityKind, ListIndexesMetadataRequest, ListIndexesMetadataResponse, - ListShardsRequest, ListShardsResponse, ListShardsSubresponse, MetastoreError, SourceType, + DeleteShardsResponse, EntityKind, FindIndexTemplateMatchesResponse, + ListIndexesMetadataRequest, ListIndexesMetadataResponse, ListShardsRequest, + ListShardsResponse, ListShardsSubresponse, MetastoreError, SourceType, }; use quickwit_proto::types::Position; @@ -658,8 +757,6 @@ mod tests { #[tokio::test] async fn test_control_plane_create_index() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); let ingester_pool = IngesterPool::default(); @@ -669,36 +766,37 @@ mod tests { .expect_create_index() .withf(|create_index_request| { let index_config: IndexConfig = - serde_json::from_str(&create_index_request.index_config_json).unwrap(); + create_index_request.deserialize_index_config().unwrap(); assert_eq!(index_config.index_id, "test-index"); assert_eq!(index_config.index_uri, "ram:///test-index"); true }) .returning(|_| { - Ok(CreateIndexResponse { - index_uid: "test-index:0".to_string(), - }) + let index_metadata = IndexMetadata::for_test("test-index", "ram:///test-index"); + let index_metadata_json = serde_json::to_string(&index_metadata).unwrap(); + let response = CreateIndexResponse { + index_uid: index_metadata.index_uid.into(), + index_metadata_json, + }; + Ok(response) }); mock_metastore .expect_list_indexes_metadata() .returning(|_| { Ok(ListIndexesMetadataResponse::try_from_indexes_metadata(Vec::new()).unwrap()) }); - let replication_factor = 1; - + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); let index_config = IndexConfig::for_test("test-index", "ram:///test-index"); - let create_index_request = CreateIndexRequest { - index_config_json: serde_json::to_string(&index_config).unwrap(), - }; + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let create_index_response = control_plane_mailbox .ask_for_res(create_index_request) .await @@ -713,8 +811,6 @@ mod tests { #[tokio::test] async fn test_control_plane_delete_index() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); let ingester_pool = IngesterPool::default(); @@ -729,16 +825,15 @@ mod tests { .returning(|_| { Ok(ListIndexesMetadataResponse::try_from_indexes_metadata(Vec::new()).unwrap()) }); - let replication_factor = 1; + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); let delete_index_request = DeleteIndexRequest { index_uid: "test-index:0".to_string(), @@ -756,8 +851,6 @@ mod tests { #[tokio::test] async fn test_control_plane_add_source() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); let ingester_pool = IngesterPool::default(); @@ -784,17 +877,16 @@ mod tests { ]) .unwrap()) }); - let replication_factor = 1; + + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); - let source_config = SourceConfig::for_test("test-source", SourceParams::void()); let add_source_request = AddSourceRequest { index_uid: index_uid.to_string(), @@ -813,8 +905,6 @@ mod tests { #[tokio::test] async fn test_control_plane_toggle_source() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); let ingester_pool = IngesterPool::default(); @@ -850,35 +940,32 @@ mod tests { Ok(EmptyResponse {}) }); - let replication_factor = 1; - + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); - - let enabling_source_req = ToggleSourceRequest { + let enable_source_request = ToggleSourceRequest { index_uid: "test-index:0".to_string(), source_id: "test-source".to_string(), enable: true, }; control_plane_mailbox - .ask_for_res(enabling_source_req) + .ask_for_res(enable_source_request) .await .unwrap(); - let disabling_source_req = ToggleSourceRequest { + let disable_source_request = ToggleSourceRequest { index_uid: "test-index:0".to_string(), source_id: "test-source".to_string(), enable: false, }; control_plane_mailbox - .ask_for_res(disabling_source_req) + .ask_for_res(disable_source_request) .await .unwrap(); @@ -888,8 +975,6 @@ mod tests { #[tokio::test] async fn test_control_plane_delete_source() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); let ingester_pool = IngesterPool::default(); @@ -908,16 +993,15 @@ mod tests { .returning(|_| { Ok(ListIndexesMetadataResponse::try_from_indexes_metadata(Vec::new()).unwrap()) }); - let replication_factor = 1; + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); let delete_source_request = DeleteSourceRequest { index_uid: "test-index:0".to_string(), @@ -936,8 +1020,6 @@ mod tests { #[tokio::test] async fn test_control_plane_get_or_create_open_shards() { let universe = Universe::with_accelerated_time(); - - let cluster_id = "test-cluster".to_string(); let self_node_id: NodeId = "test-node".into(); let indexer_pool = IndexerPool::default(); @@ -948,7 +1030,7 @@ mod tests { .expect_list_indexes_metadata() .returning(|_| { let mut index_metadata = IndexMetadata::for_test("test-index", "ram:///test-index"); - let mut source_config = SourceConfig::ingest_v2_default(); + let mut source_config = SourceConfig::ingest_v2(); source_config.enabled = true; index_metadata.add_source(source_config).unwrap(); Ok( @@ -977,16 +1059,15 @@ mod tests { let response = ListShardsResponse { subresponses }; Ok(response) }); - let replication_factor = 1; + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - cluster_id, + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - replication_factor, ); let get_open_shards_request = GetOrCreateOpenShardsRequest { subrequests: vec![GetOrCreateOpenShardsSubrequest { @@ -1022,7 +1103,7 @@ mod tests { let mut mock_metastore = MetastoreServiceClient::mock(); let mut index_0 = IndexMetadata::for_test("test-index-0", "ram:///test-index-0"); - let source = SourceConfig::ingest_v2_default(); + let source = SourceConfig::ingest_v2(); index_0.add_source(source.clone()).unwrap(); mock_metastore @@ -1040,12 +1121,14 @@ mod tests { Ok(list_shards_resp) }, ); - let index_uid = IndexUid::new_with_random_ulid("test-index"); - let index_uid_string = index_uid.to_string(); + let index_metadata = IndexMetadata::for_test("test-index", "ram:///test-index"); + let index_metadata_json = serde_json::to_string(&index_metadata).unwrap(); + mock_metastore.expect_create_index().times(1).return_once( |_create_index_request: CreateIndexRequest| { Ok(CreateIndexResponse { - index_uid: index_uid_string, + index_uid: index_metadata.index_uid.into(), + index_metadata_json, }) }, ); @@ -1067,26 +1150,24 @@ mod tests { }, ); + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, control_plane_handle) = ControlPlane::spawn( &universe, - "cluster".to_string(), + cluster_config, node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - 1, ); - let index_config = IndexConfig::for_test("test-index", "ram:///test-index"); - let create_index_request = CreateIndexRequest { - index_config_json: serde_json::to_string(&index_config).unwrap(), - }; + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); // A happy path: we simply create the index. - assert!(control_plane_mailbox + control_plane_mailbox .ask_for_res(create_index_request.clone()) .await - .is_ok()); + .unwrap(); // Now let's see what happens if we attempt to create the same index a second time. let control_plane_error: ControlPlaneError = control_plane_mailbox @@ -1157,7 +1238,7 @@ mod tests { let mut mock_metastore = MetastoreServiceClient::mock(); let mut index_0 = IndexMetadata::for_test("test-index-0", "ram:///test-index-0"); - let mut source = SourceConfig::ingest_v2_default(); + let mut source = SourceConfig::ingest_v2(); source.enabled = true; index_0.add_source(source.clone()).unwrap(); @@ -1207,14 +1288,14 @@ mod tests { }, ); + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - "cluster".to_string(), + cluster_config, node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - 1, ); let source_uid = SourceUid { index_uid: index_0.index_uid.clone(), @@ -1292,7 +1373,7 @@ mod tests { let mut mock_metastore = MetastoreServiceClient::mock(); let mut index_0 = IndexMetadata::for_test("test-index-0", "ram:///test-index-0"); - let mut source = SourceConfig::ingest_v2_default(); + let mut source = SourceConfig::ingest_v2(); source.enabled = true; index_0.add_source(source.clone()).unwrap(); @@ -1333,14 +1414,14 @@ mod tests { }, ); + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - "cluster".to_string(), + cluster_config, node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - 1, ); let source_uid = SourceUid { index_uid: index_0.index_uid.clone(), @@ -1371,7 +1452,7 @@ mod tests { let mut seq = Sequence::new(); let mut index_0 = IndexMetadata::for_test("test-index-0", "ram:///test-index-0"); - let mut source = SourceConfig::ingest_v2_default(); + let mut source = SourceConfig::ingest_v2(); source.enabled = true; index_0.add_source(source.clone()).unwrap(); @@ -1448,14 +1529,14 @@ mod tests { }); ingester_pool.insert("node1".into(), ingester_mock.into()); + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - "cluster".to_string(), + cluster_config, node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - 1, ); // This update should not trigger anything in the control plane. control_plane_mailbox @@ -1468,6 +1549,7 @@ mod tests { universe.assert_quit().await; } + #[tokio::test] async fn test_delete_source() { quickwit_common::setup_logging_for_tests(); @@ -1502,7 +1584,7 @@ mod tests { }, ); - let mut source = SourceConfig::ingest_v2_default(); + let mut source = SourceConfig::ingest_v2(); source.enabled = true; index_0.add_source(source.clone()).unwrap(); @@ -1539,15 +1621,14 @@ mod tests { Ok(list_shards_resp) }, ); - + let cluster_config = ClusterConfig::for_test(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( &universe, - "cluster".to_string(), + cluster_config, node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(mock_metastore), - 1, ); // This update should not trigger anything in the control plane. control_plane_mailbox @@ -1561,4 +1642,97 @@ mod tests { universe.assert_quit().await; } + + #[tokio::test] + async fn test_auto_create_indexes_on_get_or_create_open_shards_request() { + let universe = Universe::default(); + + let mut cluster_config = ClusterConfig::for_test(); + cluster_config.auto_create_indexes = true; + + let node_id = NodeId::from("test-node"); + let indexer_pool = IndexerPool::default(); + let ingester_pool = IngesterPool::default(); + + let mut mock_metastore = MetastoreServiceClient::mock(); + + mock_metastore + .expect_list_indexes_metadata() + .return_once(|_| { + Ok(ListIndexesMetadataResponse::try_from_indexes_metadata(Vec::new()).unwrap()) + }); + + mock_metastore + .expect_find_index_template_matches() + .return_once(|request| { + assert_eq!(request.index_ids, ["test-index-foo"]); + + let index_template = + IndexTemplate::for_test("test-template-foo", &["test-index-foo*"], 100); + let index_template_json = serde_json::to_string(&index_template).unwrap(); + + Ok(FindIndexTemplateMatchesResponse { + matches: vec![IndexTemplateMatch { + template_id: "test-template-foo".to_string(), + index_id: "test-index-foo".to_string(), + index_template_json, + }], + }) + }); + + mock_metastore.expect_create_index().return_once(|request| { + let index_config = request.deserialize_index_config().unwrap(); + assert_eq!(index_config.index_id, "test-index-foo"); + assert_eq!(index_config.index_uri, "ram:///indexes/test-index-foo"); + + let source_configs = request.deserialize_source_configs().unwrap(); + assert_eq!(source_configs.len(), 3); + assert_eq!(source_configs[0].source_id, INGEST_API_SOURCE_ID); + assert_eq!(source_configs[1].source_id, INGEST_V2_SOURCE_ID); + assert_eq!(source_configs[2].source_id, CLI_SOURCE_ID); + + let index_uid = IndexUid::new_2("test-index-foo", 0); + let mut index_metadata = IndexMetadata::new_with_index_uid(index_uid, index_config); + + for source_config in source_configs { + index_metadata.add_source(source_config).unwrap(); + } + let index_metadata_json = serde_json::to_string(&index_metadata).unwrap(); + + Ok(CreateIndexResponse { + index_uid: index_metadata.index_uid.into(), + index_metadata_json, + }) + }); + + let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( + &universe, + cluster_config, + node_id, + indexer_pool, + ingester_pool, + MetastoreServiceClient::from(mock_metastore), + ); + + let error = control_plane_mailbox + .ask(GetOrCreateOpenShardsRequest { + subrequests: vec![GetOrCreateOpenShardsSubrequest { + subrequest_id: 0, + index_id: "test-index-foo".to_string(), + source_id: INGEST_V2_SOURCE_ID.to_string(), + }], + closed_shards: Vec::new(), + unavailable_leaders: Vec::new(), + }) + .await + .unwrap() + .unwrap_err(); + assert!(matches!(error, ControlPlaneError::Unavailable { .. })); + + let control_plane_state = control_plane_mailbox.ask(Observe).await.unwrap(); + assert_eq!(control_plane_state.model_metrics.num_indexes, 1); + assert_eq!(control_plane_state.model_metrics.num_sources, 1); + + universe.assert_quit().await; + } } diff --git a/quickwit/quickwit-control-plane/src/indexing_scheduler/mod.rs b/quickwit/quickwit-control-plane/src/indexing_scheduler/mod.rs index a241cbbede4..39d791528ae 100644 --- a/quickwit/quickwit-control-plane/src/indexing_scheduler/mod.rs +++ b/quickwit/quickwit-control-plane/src/indexing_scheduler/mod.rs @@ -33,7 +33,7 @@ use quickwit_proto::metastore::SourceType; use quickwit_proto::types::{NodeId, ShardId}; use scheduling::{SourceToSchedule, SourceToScheduleType}; use serde::Serialize; -use tracing::{debug, error, info, warn}; +use tracing::{debug, info, warn}; use crate::indexing_plan::PhysicalIndexingPlan; use crate::indexing_scheduler::scheduling::build_physical_indexing_plan; @@ -118,7 +118,7 @@ impl fmt::Debug for IndexingScheduler { fn get_sources_to_schedule(model: &ControlPlaneModel) -> Vec { let mut sources = Vec::new(); - for (source_uid, source_config) in model.get_source_configs() { + for (source_uid, source_config) in model.source_configs() { if !source_config.enabled { continue; } @@ -318,7 +318,7 @@ impl IndexingScheduler { .apply_indexing_plan(ApplyIndexingPlanRequest { indexing_tasks }) .await { - error!(indexer_node_id=%indexer.0, err=?error, "error occurred when applying indexing plan to indexer"); + warn!(error=%error, node_id=indexer.0, "failed to apply indexing plan to indexer"); } } }); @@ -757,7 +757,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - model.insert_newly_opened_shards(&index_uid, &"ingest_v2".to_string(), vec![shard]); + model.insert_shards(&index_uid, &"ingest_v2".to_string(), vec![shard]); let shards: Vec = get_sources_to_schedule(&model); assert_eq!(shards.len(), 3); } diff --git a/quickwit/quickwit-control-plane/src/ingest/ingest_controller.rs b/quickwit/quickwit-control-plane/src/ingest/ingest_controller.rs index 76d47e67f9e..b0b73a8738b 100644 --- a/quickwit/quickwit-control-plane/src/ingest/ingest_controller.rs +++ b/quickwit/quickwit-control-plane/src/ingest/ingest_controller.rs @@ -440,7 +440,7 @@ impl IngestController { for open_shards_subresponse in open_shards_response.subresponses { let index_uid: IndexUid = open_shards_subresponse.index_uid.clone().into(); let source_id = open_shards_subresponse.source_id.clone(); - model.insert_newly_opened_shards( + model.insert_shards( &index_uid, &source_id, open_shards_subresponse.opened_shards, @@ -567,7 +567,7 @@ impl IngestController { let index_uid: IndexUid = open_shards_subresponse.index_uid.into(); let source_id = open_shards_subresponse.source_id; - model.insert_newly_opened_shards( + model.insert_shards( &index_uid, &source_id, open_shards_subresponse.opened_shards, @@ -686,7 +686,7 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; - use quickwit_config::{SourceConfig, SourceParams, INGEST_V2_SOURCE_ID}; + use quickwit_config::{SourceConfig, INGEST_V2_SOURCE_ID}; use quickwit_ingest::{RateMibPerSec, ShardInfo}; use quickwit_metastore::IndexMetadata; use quickwit_proto::control_plane::GetOrCreateOpenShardsSubrequest; @@ -977,11 +977,12 @@ mod tests { ); let mut model = ControlPlaneModel::default(); - - let source_config = SourceConfig::for_test(source_id, SourceParams::stdin()); - model.add_index(index_metadata_0.clone()); model.add_index(index_metadata_1.clone()); + + let mut source_config = SourceConfig::ingest_v2(); + source_config.source_id = source_id.to_string(); + model .add_source(&index_uid_0, source_config.clone()) .unwrap(); @@ -1006,7 +1007,7 @@ mod tests { }, ]; - model.insert_newly_opened_shards(&index_uid_0, &source_id.into(), shards); + model.insert_shards(&index_uid_0, &source_id.into(), shards); let request = GetOrCreateOpenShardsRequest { subrequests: Vec::new(), @@ -1117,7 +1118,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let request = GetOrCreateOpenShardsRequest { subrequests: Vec::new(), @@ -1185,7 +1186,7 @@ mod tests { ..Default::default() }, ]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let request = GetOrCreateOpenShardsRequest { subrequests: Vec::new(), @@ -1245,7 +1246,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let shard_entries: Vec = model.all_shards().cloned().collect(); assert_eq!(shard_entries.len(), 1); @@ -1279,7 +1280,7 @@ mod tests { leader_id: "test-ingester".to_string(), ..Default::default() }]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let shard_entries: Vec = model.all_shards().cloned().collect(); assert_eq!(shard_entries.len(), 2); @@ -1412,7 +1413,7 @@ mod tests { IndexMetadata::for_test(index_uid.index_id(), "ram://indexes/test-index:0"); model.add_index(index_metadata); - let souce_config = SourceConfig::ingest_v2_default(); + let souce_config = SourceConfig::ingest_v2(); model.add_source(&index_uid, souce_config).unwrap(); let progress = Progress::default(); @@ -1510,7 +1511,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); // Test ingester is unavailable. ingest_controller @@ -1565,7 +1566,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); // Test rate limited. ingest_controller @@ -1637,7 +1638,7 @@ mod tests { ..Default::default() }, ]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let shard_infos = BTreeSet::from_iter([ ShardInfo { @@ -1719,7 +1720,7 @@ mod tests { ..Default::default() }, ]; - model.insert_newly_opened_shards(&index_uid, &source_id, shards); + model.insert_shards(&index_uid, &source_id, shards); let mut ingester_mock1 = IngesterServiceClient::mock(); let ingester_mock2 = IngesterServiceClient::mock(); diff --git a/quickwit/quickwit-control-plane/src/ingest/wait_handle.rs b/quickwit/quickwit-control-plane/src/ingest/wait_handle.rs index 566dcf43b88..fc1905457a0 100644 --- a/quickwit/quickwit-control-plane/src/ingest/wait_handle.rs +++ b/quickwit/quickwit-control-plane/src/ingest/wait_handle.rs @@ -41,6 +41,7 @@ pub struct WaitDropGuard(oneshot::Sender<()>); #[cfg(test)] mod tests { use tokio::sync::oneshot::error::TryRecvError; + #[tokio::test] async fn test_wait_handle_simple() { let (wait_drop_handle, mut wait_handle) = super::WaitHandle::new(); diff --git a/quickwit/quickwit-control-plane/src/model/mod.rs b/quickwit/quickwit-control-plane/src/model/mod.rs index 7007ef4bdc4..d321aee80ea 100644 --- a/quickwit/quickwit-control-plane/src/model/mod.rs +++ b/quickwit/quickwit-control-plane/src/model/mod.rs @@ -44,7 +44,7 @@ use tracing::{info, instrument, warn}; /// The control plane maintains a model in sync with the metastore. /// /// The model stays consistent with the metastore, because all -/// of the mutations go through the control plane. +/// the mutations (create/delete index, add/delete source, etc.) go through the control plane. /// /// If a mutation yields an error, the control plane is killed /// and restarted. @@ -59,6 +59,8 @@ pub(crate) struct ControlPlaneModel { #[derive(Clone, Copy, Debug, Default, Serialize)] pub struct ControlPlaneModelMetrics { + pub num_indexes: usize, + pub num_sources: usize, pub num_shards: usize, } @@ -70,6 +72,8 @@ impl ControlPlaneModel { pub fn observable_state(&self) -> ControlPlaneModelMetrics { ControlPlaneModelMetrics { + num_indexes: self.index_table.len(), + num_sources: self.shard_table.num_sources(), num_shards: self.shard_table.num_shards(), } } @@ -104,7 +108,7 @@ impl ControlPlaneModel { for source_config in index_metadata.sources.values() { num_sources += 1; - if source_config.source_type() != SourceType::IngestV2 || !source_config.enabled { + if source_config.source_type() != SourceType::IngestV2 { continue; } let request = ListShardsSubrequest { @@ -123,30 +127,26 @@ impl ControlPlaneModel { for list_shards_subresponse in list_shard_response.subresponses { num_shards += list_shards_subresponse.shards.len(); + let ListShardsSubresponse { index_uid, source_id, shards, } = list_shards_subresponse; - let source_uid = SourceUid { - index_uid: IndexUid::parse(&index_uid).map_err(|invalid_index_uri| { - ControlPlaneError::Internal(format!( - "invalid index uid received from the metastore: {invalid_index_uri:?}" - )) - })?, - source_id, - }; + let index_uid = IndexUid::parse(&index_uid).map_err(|invalid_index_uri| { + ControlPlaneError::Internal(format!( + "invalid index uid received from the metastore: {invalid_index_uri:?}" + )) + })?; self.shard_table - .initialize_source_shards(source_uid, shards); + .insert_shards(&index_uid, &source_id, shards); } } + let elapsed_secs = now.elapsed().as_secs(); + info!( - "synced internal state with metastore in {} seconds ({} indexes, {} sources, {} \ - shards)", - now.elapsed().as_secs(), - num_indexes, - num_sources, - num_shards, + "synced control plane model with metastore in {elapsed_secs} seconds ({num_indexes} \ + indexes, {num_sources} sources, {num_shards} shards)", ); Ok(()) } @@ -155,9 +155,7 @@ impl ControlPlaneModel { self.index_uid_table.get(index_id).cloned() } - pub(crate) fn get_source_configs( - &self, - ) -> impl Iterator + '_ { + pub(crate) fn source_configs(&self) -> impl Iterator + '_ { self.index_table.values().flat_map(|index_metadata| { index_metadata .sources @@ -176,18 +174,25 @@ impl ControlPlaneModel { pub(crate) fn add_index(&mut self, index_metadata: IndexMetadata) { let index_uid = index_metadata.index_uid.clone(); - self.index_uid_table - .insert(index_metadata.index_id().to_string(), index_uid.clone()); + let index_id = index_uid.index_id().to_string(); + + self.index_uid_table.insert(index_id, index_uid.clone()); + + for (source_id, source_config) in &index_metadata.sources { + if source_config.source_type() == SourceType::IngestV2 { + self.shard_table.add_source(&index_uid, source_id); + } + } self.index_table.insert(index_uid, index_metadata); } pub(crate) fn delete_index(&mut self, index_uid: &IndexUid) { - // TODO: We need to let the routers and ingesters know. self.index_table.remove(index_uid); + self.index_uid_table.remove(index_uid.index_id()); self.shard_table.delete_index(index_uid.index_id()); } - /// Adds a source to a given index. Returns an error if a source with the same source_id already + /// Adds a source to a given index. Returns an error if the source already /// exists. pub(crate) fn add_source( &mut self, @@ -199,9 +204,12 @@ impl ControlPlaneModel { index_id: index_uid.to_string(), }) })?; - let source_id = source_config.source_id.clone(); - index_metadata.add_source(source_config)?; - self.shard_table.add_source(index_uid, &source_id); + index_metadata.add_source(source_config.clone())?; + + if source_config.source_type() == SourceType::IngestV2 { + self.shard_table + .add_source(index_uid, &source_config.source_id); + } Ok(()) } @@ -209,12 +217,16 @@ impl ControlPlaneModel { // Removing shards from shard table. self.shard_table .delete_source(&source_uid.index_uid, &source_uid.source_id); - // Remove source from index config. - let Some(index_model) = self.index_table.get_mut(&source_uid.index_uid) else { + // Remove source from index metadata. + let Some(index_metadata) = self.index_table.get_mut(&source_uid.index_uid) else { warn!(index_uid=%source_uid.index_uid, source_id=%source_uid.source_id, "delete source: index not found"); return; }; - if index_model.sources.remove(&source_uid.source_id).is_none() { + if index_metadata + .sources + .remove(&source_uid.source_id) + .is_none() + { warn!(index_uid=%source_uid.index_uid, source_id=%source_uid.source_id, "delete source: source not found"); }; } @@ -280,14 +292,14 @@ impl ControlPlaneModel { } /// Inserts the shards that have just been opened by calling `open_shards` on the metastore. - pub fn insert_newly_opened_shards( + pub fn insert_shards( &mut self, index_uid: &IndexUid, source_id: &SourceId, opened_shards: Vec, ) { self.shard_table - .insert_newly_opened_shards(index_uid, source_id, opened_shards); + .insert_shards(index_uid, source_id, opened_shards); } /// Finds open shards for a given index and source and whose leaders are not in the set of @@ -363,16 +375,16 @@ mod tests { assert_eq!(request, ListIndexesMetadataRequest::all()); let mut index_0 = IndexMetadata::for_test("test-index-0", "ram:///test-index-0"); - let mut source_config = SourceConfig::ingest_v2_default(); + let mut source_config = SourceConfig::ingest_v2(); source_config.enabled = true; index_0.add_source(source_config.clone()).unwrap(); let mut index_1 = IndexMetadata::for_test("test-index-1", "ram:///test-index-1"); - index_1.add_source(source_config.clone()).unwrap(); + source_config.enabled = false; + index_1.add_source(source_config).unwrap(); let mut index_2 = IndexMetadata::for_test("test-index-2", "ram:///test-index-2"); - source_config.enabled = false; - index_2.add_source(source_config.clone()).unwrap(); + index_2.add_source(SourceConfig::cli()).unwrap(); let indexes = vec![index_0, index_1, index_2]; Ok(ListIndexesMetadataResponse::try_from_indexes_metadata(indexes).unwrap()) @@ -382,17 +394,11 @@ mod tests { assert_eq!(request.subrequests[0].index_uid, "test-index-0:0"); assert_eq!(request.subrequests[0].source_id, INGEST_V2_SOURCE_ID); - assert_eq!( - request.subrequests[0].shard_state(), - ShardState::Unspecified - ); + assert!(request.subrequests[0].shard_state.is_none()); assert_eq!(request.subrequests[1].index_uid, "test-index-1:0"); assert_eq!(request.subrequests[1].source_id, INGEST_V2_SOURCE_ID); - assert_eq!( - request.subrequests[1].shard_state(), - ShardState::Unspecified - ); + assert!(request.subrequests[1].shard_state.is_none()); let subresponses = vec![ metastore::ListShardsSubresponse { @@ -417,25 +423,16 @@ mod tests { Ok(response) }); let mut model = ControlPlaneModel::default(); - let mut metastore_client = MetastoreServiceClient::from(mock_metastore); + let mut metastore = MetastoreServiceClient::from(mock_metastore); model - .load_from_metastore(&mut metastore_client, &progress) + .load_from_metastore(&mut metastore, &progress) .await .unwrap(); assert_eq!(model.index_table.len(), 3); - assert_eq!( - model.index_uid("test-index-0").unwrap().as_str(), - "test-index-0:0" - ); - assert_eq!( - model.index_uid("test-index-1").unwrap().as_str(), - "test-index-1:0" - ); - assert_eq!( - model.index_uid("test-index-2").unwrap().as_str(), - "test-index-2:0" - ); + assert_eq!(model.index_uid("test-index-0").unwrap(), "test-index-0:0"); + assert_eq!(model.index_uid("test-index-1").unwrap(), "test-index-1:0"); + assert_eq!(model.index_uid("test-index-2").unwrap(), "test-index-2:0"); assert_eq!(model.shard_table.num_shards(), 1); @@ -463,10 +460,73 @@ mod tests { assert_eq!(shards.len(), 0); } + #[test] + fn test_control_plane_model_add_index() { + let mut model = ControlPlaneModel::default(); + let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes"); + let index_uid = index_metadata.index_uid.clone(); + model.add_index(index_metadata.clone()); + + assert_eq!(model.index_table.len(), 1); + assert_eq!(model.index_table.get(&index_uid).unwrap(), &index_metadata); + + assert_eq!(model.index_uid_table.len(), 1); + assert_eq!(model.index_uid("test-index").unwrap(), "test-index:0"); + } + + #[test] + fn test_control_plane_model_add_index_with_sources() { + let mut model = ControlPlaneModel::default(); + let mut index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes"); + index_metadata.add_source(SourceConfig::cli()).unwrap(); + index_metadata + .add_source(SourceConfig::ingest_v2()) + .unwrap(); + let index_uid = index_metadata.index_uid.clone(); + model.add_index(index_metadata.clone()); + + assert_eq!(model.index_table.len(), 1); + assert_eq!(model.index_table.get(&index_uid).unwrap(), &index_metadata); + + assert_eq!(model.index_uid_table.len(), 1); + assert_eq!(model.index_uid("test-index").unwrap(), "test-index:0"); + + assert_eq!(model.shard_table.num_sources(), 1); + + let source_uid = SourceUid { + index_uid: index_uid.clone(), + source_id: INGEST_V2_SOURCE_ID.to_string(), + }; + assert_eq!( + model.shard_table.list_shards(&source_uid).unwrap().count(), + 0 + ); + } + + #[test] + fn test_control_plane_model_delete_index() { + let mut model = ControlPlaneModel::default(); + + let mut index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes"); + let index_uid = index_metadata.index_uid.clone(); + model.delete_index(&index_uid); + + index_metadata + .add_source(SourceConfig::ingest_v2()) + .unwrap(); + model.add_index(index_metadata); + + model.delete_index(&index_uid); + + assert!(model.index_table.is_empty()); + assert!(model.index_uid_table.is_empty()); + assert_eq!(model.shard_table.num_sources(), 0); + } + #[test] fn test_control_plane_model_toggle_source() { let mut model = ControlPlaneModel::default(); - let index_metadata = IndexMetadata::for_test("test-index", "ram://"); + let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes"); let index_uid = index_metadata.index_uid.clone(); model.add_index(index_metadata); let source_config = SourceConfig::for_test("test-source", SourceParams::void()); diff --git a/quickwit/quickwit-control-plane/src/model/shard_table.rs b/quickwit/quickwit-control-plane/src/model/shard_table.rs index 14e2b522945..222479f29c5 100644 --- a/quickwit/quickwit-control-plane/src/model/shard_table.rs +++ b/quickwit/quickwit-control-plane/src/model/shard_table.rs @@ -99,21 +99,6 @@ impl Default for ShardTableEntry { } impl ShardTableEntry { - pub fn from_shards(shards: Vec) -> Self { - let shard_entries = shards - .into_iter() - .filter(|shard| { - let shard_state = shard.shard_state(); - shard_state == ShardState::Open || shard_state == ShardState::Closed - }) - .map(|shard| (shard.shard_id().clone(), shard.into())) - .collect(); - Self { - shard_entries, - ..Default::default() - } - } - fn is_empty(&self) -> bool { self.shard_entries.is_empty() } @@ -137,7 +122,7 @@ fn remove_shard_from_ingesters_internal( shard: &Shard, ingester_shards: &mut FnvHashMap>>, ) { - for node in shard.ingester_nodes() { + for node in shard.ingesters() { let ingester_shards = ingester_shards .get_mut(&node) .expect("shard table reached inconsistent state"); @@ -181,7 +166,7 @@ impl ShardTable { for (shard_id, shard_entry) in &shard_table_entry.shard_entries { debug_assert_eq!(shard_id, shard_entry.shard.shard_id()); debug_assert_eq!(source_uid.index_uid.as_str(), &shard_entry.shard.index_uid); - for node in shard_entry.shard.ingester_nodes() { + for node in shard_entry.shard.ingesters() { shard_sets_in_shard_table.insert((node, source_uid, shard_id)); } } @@ -220,6 +205,10 @@ impl ShardTable { .flat_map(|(_, shard_table_entry)| shard_table_entry.shard_entries.values()) } + pub fn num_sources(&self) -> usize { + self.table_entries.len() + } + pub fn num_shards(&self) -> usize { self.table_entries .values() @@ -295,8 +284,8 @@ impl ShardTable { .map(|table_entry| table_entry.shard_entries.values()) } - /// Updates the shard table. - pub fn insert_newly_opened_shards( + /// Inserts the shards into the shard table. + pub fn insert_shards( &mut self, index_uid: &IndexUid, source_id: &SourceId, @@ -317,7 +306,7 @@ impl ShardTable { } } for shard in &opened_shards { - for node in shard.ingester_nodes() { + for node in shard.ingesters() { let ingester_shards = self.ingester_shards.entry(node).or_default(); let shard_ids = ingester_shards.entry(source_uid.clone()).or_default(); shard_ids.insert(shard.shard_id().clone()); @@ -462,22 +451,6 @@ impl ShardTable { self.check_invariant(); } - /// Set the shards for a given source. - /// This function panics if an entry was previously associated to the source uid. - pub(crate) fn initialize_source_shards(&mut self, source_uid: SourceUid, shards: Vec) { - for shard in &shards { - for node in shard.ingester_nodes() { - let ingester_shards = self.ingester_shards.entry(node).or_default(); - let shard_ids = ingester_shards.entry(source_uid.clone()).or_default(); - shard_ids.insert(shard.shard_id().clone()); - } - } - let table_entry = ShardTableEntry::from_shards(shards); - let previous_entry = self.table_entries.insert(source_uid, table_entry); - assert!(previous_entry.is_none()); - self.check_invariant(); - } - pub fn acquire_scaling_permits( &mut self, source_uid: &SourceUid, @@ -615,7 +588,7 @@ mod tests { shard_state: ShardState::Closed as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards(&index_uid, &source_id, vec![shard_01]); + shard_table.insert_shards(&index_uid, &source_id, vec![shard_01]); let shards = shard_table.list_shards(&source_uid).unwrap(); assert_eq!(shards.count(), 1); @@ -636,7 +609,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards(&index_uid_0, &source_id, vec![shard_01.clone()]); + shard_table.insert_shards(&index_uid_0, &source_id, vec![shard_01.clone()]); assert_eq!(shard_table.table_entries.len(), 1); @@ -667,7 +640,7 @@ mod tests { ..Default::default() }; - shard_table.insert_newly_opened_shards( + shard_table.insert_shards( &index_uid_0, &source_id, vec![shard_01.clone(), shard_02.clone()], @@ -733,7 +706,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards( + shard_table.insert_shards( &index_uid, &source_id, vec![shard_01, shard_02, shard_03.clone(), shard_04.clone()], @@ -789,7 +762,7 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards( + shard_table.insert_shards( &index_uid, &source_id, vec![shard_01, shard_02, shard_03, shard_04], @@ -889,8 +862,8 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards(&index_uid_0, &source_id, vec![shard_01, shard_02]); - shard_table.insert_newly_opened_shards(&index_uid_1, &source_id, vec![shard_11]); + shard_table.insert_shards(&index_uid_0, &source_id, vec![shard_01, shard_02]); + shard_table.insert_shards(&index_uid_1, &source_id, vec![shard_11]); let source_uid_0 = SourceUid { index_uid: index_uid_0, @@ -939,12 +912,8 @@ mod tests { shard_state: ShardState::Open as i32, ..Default::default() }; - shard_table.insert_newly_opened_shards( - &index_uid_0, - &source_id, - vec![shard_01.clone(), shard_02], - ); - shard_table.insert_newly_opened_shards(&index_uid_1, &source_id, vec![shard_11]); + shard_table.insert_shards(&index_uid_0, &source_id, vec![shard_01.clone(), shard_02]); + shard_table.insert_shards(&index_uid_1, &source_id, vec![shard_11]); let source_uid_0 = SourceUid { index_uid: index_uid_0.clone(), diff --git a/quickwit/quickwit-control-plane/src/tests.rs b/quickwit/quickwit-control-plane/src/tests.rs index 20aba71910d..904cbdbb55f 100644 --- a/quickwit/quickwit-control-plane/src/tests.rs +++ b/quickwit/quickwit-control-plane/src/tests.rs @@ -27,7 +27,9 @@ use quickwit_cluster::{create_cluster_for_test, ChannelTransport, Cluster, Clust use quickwit_common::test_utils::wait_until_predicate; use quickwit_common::tower::{Change, Pool}; use quickwit_config::service::QuickwitService; -use quickwit_config::{KafkaSourceParams, SourceConfig, SourceInputFormat, SourceParams}; +use quickwit_config::{ + ClusterConfig, KafkaSourceParams, SourceConfig, SourceInputFormat, SourceParams, +}; use quickwit_indexing::IndexingService; use quickwit_metastore::{IndexMetadata, ListIndexesMetadataResponseExt}; use quickwit_proto::indexing::{ApplyIndexingPlanRequest, CpuCapacity, IndexingServiceClient}; @@ -140,15 +142,17 @@ async fn start_control_plane( let indexer_change_stream = test_indexer_change_stream(change_stream, indexing_clients); indexer_pool.listen_for_changes(indexer_change_stream); + let mut cluster_config = ClusterConfig::for_test(); + cluster_config.cluster_id = cluster.cluster_id().to_string(); + let self_node_id: NodeId = cluster.self_node_id().to_string().into(); let (control_plane_mailbox, _control_plane_handle) = ControlPlane::spawn( universe, - cluster.cluster_id().to_string(), + cluster_config, self_node_id, indexer_pool, ingester_pool, MetastoreServiceClient::from(metastore), - 1, ); (indexer_inboxes, control_plane_mailbox) diff --git a/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs b/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs index 4d6a5816366..d073b8709bf 100644 --- a/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs +++ b/quickwit/quickwit-doc-mapper/src/default_doc_mapper/default_mapper.rs @@ -220,7 +220,7 @@ impl TryFrom for DefaultDocMapper { let (default_search_field, _json_path) = schema .find_field_with_default(default_search_field_name, dynamic_field) .with_context(|| { - format!("Unknown default search field: `{default_search_field_name}`") + format!("unknown default search field `{default_search_field_name}`") })?; if !schema.get_field_entry(default_search_field).is_indexed() { bail!("default search field `{default_search_field_name}` is not indexed",); @@ -824,10 +824,7 @@ mod tests { let error = result.unwrap_err(); assert_eq!( error, - DocParsingError::ValueError( - "body".to_owned(), - "expected JSON string, got `1`".to_owned() - ) + DocParsingError::ValueError("body".to_owned(), "expected string, got `1`".to_owned()) ); Ok(()) } diff --git a/quickwit/quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs b/quickwit/quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs index efbe800d1d8..40b8fbf70b5 100644 --- a/quickwit/quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs +++ b/quickwit/quickwit-doc-mapper/src/default_doc_mapper/mapping_tree.rs @@ -61,7 +61,7 @@ impl LeafType { if let JsonValue::String(text) = json_val { Ok(TantivyValue::Str(text)) } else { - Err(format!("expected JSON string, got `{json_val}`")) + Err(format!("expected string, got `{json_val}`")) } } LeafType::I64(numeric_options) => i64::from_json(json_val, numeric_options.coerce), @@ -71,7 +71,7 @@ impl LeafType { if let JsonValue::Bool(val) = json_val { Ok(TantivyValue::Bool(val)) } else { - Err(format!("expected bool value, got `{json_val}`")) + Err(format!("expected boolean, got `{json_val}`")) } } LeafType::IpAddr(_) => { @@ -81,7 +81,7 @@ impl LeafType { .into_ipv6_addr(); Ok(TantivyValue::IpAddr(ipv6_value)) } else { - Err(format!("expected string value, got `{json_val}`")) + Err(format!("expected string, got `{json_val}`")) } } LeafType::DateTime(date_time_options) => date_time_options.parse_json(json_val), @@ -95,7 +95,7 @@ impl LeafType { .collect(), )) } else { - Err(format!("expected JSON object got `{json_val}`")) + Err(format!("expected object, got `{json_val}`")) } } } @@ -1021,7 +1021,7 @@ mod tests { assert!(err.contains("failed to parse IP address `foo`")); let err = typ.value_from_json(json!(1200)).err().unwrap(); - assert!(err.contains("expected string value, got `1200`")); + assert!(err.contains("expected string, got `1200`")); } #[test] @@ -1120,7 +1120,7 @@ mod tests { fn test_parse_text_number_should_error() { let typ = LeafType::Text(QuickwitTextOptions::default()); let err = typ.value_from_json(json!(2u64)).err().unwrap(); - assert_eq!(err, "expected JSON string, got `2`"); + assert_eq!(err, "expected string, got `2`"); } #[test] diff --git a/quickwit/quickwit-index-management/src/garbage_collection.rs b/quickwit/quickwit-index-management/src/garbage_collection.rs index 9a7e7f10c2e..e6004969f15 100644 --- a/quickwit/quickwit-index-management/src/garbage_collection.rs +++ b/quickwit/quickwit-index-management/src/garbage_collection.rs @@ -100,7 +100,7 @@ pub async fn run_garbage_collect( .with_split_state(SplitState::Staged) .with_update_timestamp_lte(grace_period_timestamp); - let list_deletable_staged_request = ListSplitsRequest::try_from_list_splits_query(query)?; + let list_deletable_staged_request = ListSplitsRequest::try_from_list_splits_query(&query)?; let deletable_staged_splits: Vec = protect_future( progress_opt, metastore.list_splits(list_deletable_staged_request), @@ -113,7 +113,7 @@ pub async fn run_garbage_collect( let marked_for_deletion_query = ListSplitsQuery::for_index(index_uid.clone()) .with_split_state(SplitState::MarkedForDeletion); let marked_for_deletion_request = - ListSplitsRequest::try_from_list_splits_query(marked_for_deletion_query)?; + ListSplitsRequest::try_from_list_splits_query(&marked_for_deletion_query)?; let mut splits_marked_for_deletion: Vec = protect_future( progress_opt, metastore.list_splits(marked_for_deletion_request), @@ -186,7 +186,7 @@ async fn delete_splits_marked_for_deletion( .with_update_timestamp_lte(updated_before_timestamp) .with_limit(DELETE_SPLITS_BATCH_SIZE); - let list_splits_request = match ListSplitsRequest::try_from_list_splits_query(query) { + let list_splits_request = match ListSplitsRequest::try_from_list_splits_query(&query) { Ok(request) => request, Err(error) => { error!(error = ?error, "failed to build list splits request"); @@ -374,7 +374,8 @@ mod tests { let index_id = "test-run-gc--index"; let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -389,12 +390,13 @@ mod tests { ..Default::default() }; let stage_splits_request = - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata).unwrap(); + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata) + .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let query = ListSplitsQuery::for_index(index_uid.clone()).with_split_state(SplitState::Staged); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -422,7 +424,7 @@ mod tests { let query = ListSplitsQuery::for_index(index_uid.clone()).with_split_state(SplitState::Staged); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -450,7 +452,7 @@ mod tests { let query = ListSplitsQuery::for_index(index_uid).with_split_state(SplitState::MarkedForDeletion); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -472,7 +474,8 @@ mod tests { let index_id = "test-run-gc--index"; let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -487,7 +490,8 @@ mod tests { ..Default::default() }; let stage_splits_request = - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata).unwrap(); + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata) + .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let mark_splits_for_deletion_request = MarkSplitsForDeletionRequest::new(index_uid.clone(), vec![split_id.to_string()]); @@ -498,7 +502,7 @@ mod tests { let query = ListSplitsQuery::for_index(index_uid.clone()) .with_split_state(SplitState::MarkedForDeletion); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -526,7 +530,7 @@ mod tests { let query = ListSplitsQuery::for_index(index_uid.clone()) .with_split_state(SplitState::MarkedForDeletion); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -553,7 +557,7 @@ mod tests { .unwrap(); let query = ListSplitsQuery::for_index(index_uid); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); assert_eq!( metastore .list_splits(list_splits_request) @@ -597,7 +601,8 @@ mod tests { let index_id = "test-delete-splits-happy--index"; let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -612,7 +617,7 @@ mod tests { ..Default::default() }; let stage_splits_request = - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata.clone()) + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata.clone()) .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let mark_splits_for_deletion = @@ -699,7 +704,8 @@ mod tests { let index_id = "test-delete-splits-storage-error--index"; let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await diff --git a/quickwit/quickwit-index-management/src/index.rs b/quickwit/quickwit-index-management/src/index.rs index 013cb26f446..61885a1f689 100644 --- a/quickwit/quickwit-index-management/src/index.rs +++ b/quickwit/quickwit-index-management/src/index.rs @@ -24,14 +24,14 @@ 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::{ - AddSourceRequestExt, CreateIndexRequestExt, IndexMetadata, IndexMetadataResponseExt, + AddSourceRequestExt, CreateIndexResponseExt, IndexMetadata, IndexMetadataResponseExt, ListSplitsQuery, ListSplitsRequestExt, MetastoreServiceStreamSplitsExt, SplitInfo, SplitMetadata, SplitState, }; use quickwit_proto::metastore::{ - AddSourceRequest, CreateIndexRequest, DeleteIndexRequest, EntityKind, IndexMetadataRequest, - ListSplitsRequest, MarkSplitsForDeletionRequest, MetastoreError, MetastoreService, - MetastoreServiceClient, ResetSourceCheckpointRequest, + serde_utils, AddSourceRequest, CreateIndexRequest, DeleteIndexRequest, EntityKind, + IndexMetadataRequest, ListSplitsRequest, MarkSplitsForDeletionRequest, MetastoreError, + MetastoreService, MetastoreServiceClient, ResetSourceCheckpointRequest, }; use quickwit_proto::types::{IndexUid, SplitId}; use quickwit_proto::{ServiceError, ServiceErrorCode}; @@ -120,34 +120,22 @@ impl IndexService { } } } - let mut metastore = self.metastore.clone(); - // Add default ingest-api & cli-ingest sources config. - let index_id = index_config.index_id.clone(); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config)?; + let index_config_json = serde_utils::to_json_str(&index_config)?; + + // Add default sources. + let source_configs_json = vec![ + serde_utils::to_json_str(&SourceConfig::ingest_api_default())?, + serde_utils::to_json_str(&SourceConfig::ingest_v2())?, + serde_utils::to_json_str(&SourceConfig::cli())?, + ]; + let create_index_request = CreateIndexRequest { + index_config_json, + source_configs_json, + }; let create_index_response = metastore.create_index(create_index_request).await?; - let index_uid: IndexUid = create_index_response.index_uid.into(); - let add_ingest_api_source_request = AddSourceRequest::try_from_source_config( - index_uid.clone(), - SourceConfig::ingest_api_default(), - )?; - metastore.add_source(add_ingest_api_source_request).await?; - let add_ingest_source_request = AddSourceRequest::try_from_source_config( - index_uid.clone(), - SourceConfig::ingest_v2_default(), - )?; - metastore.add_source(add_ingest_source_request).await?; - let add_ingest_cli_source_request = AddSourceRequest::try_from_source_config( - index_uid.clone(), - SourceConfig::cli_ingest_source(), - )?; - metastore.add_source(add_ingest_cli_source_request).await?; - let index_metadata_request = IndexMetadataRequest::for_index_id(index_id); - let index_metadata = metastore - .index_metadata(index_metadata_request) - .await? - .deserialize_index_metadata()?; + let index_metadata = create_index_response.deserialize_index_metadata()?; Ok(index_metadata) } @@ -188,7 +176,7 @@ impl IndexService { // Schedule staged and published splits for deletion. let query = ListSplitsQuery::for_index(index_uid.clone()) .with_split_states([SplitState::Staged, SplitState::Published]); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query)?; + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query)?; let split_ids: Vec = self .metastore .list_splits(list_splits_request) @@ -204,7 +192,7 @@ impl IndexService { // Select splits to delete let query = ListSplitsQuery::for_index(index_uid.clone()) .with_split_state(SplitState::MarkedForDeletion); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query)?; + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query)?; let splits_metadata_to_delete: Vec = self .metastore .list_splits(list_splits_request) @@ -329,8 +317,8 @@ impl IndexService { Ok(()) } - /// Creates a source config for index `index_id`. - pub async fn create_source( + /// Adds a source to an index identified by its UID. + pub async fn add_source( &mut self, index_uid: IndexUid, source_config: SourceConfig, @@ -339,14 +327,14 @@ impl IndexService { // This is a bit redundant, as SourceConfig deserialization also checks // that the identifier is valid. However it authorizes the special // private names internal to quickwit, so we do an extra check. - validate_identifier("Source ID", &source_id).map_err(|_| { + validate_identifier("source", &source_id).map_err(|_| { IndexServiceError::InvalidIdentifier(format!("invalid source ID: `{source_id}`")) })?; check_source_connectivity(&self.storage_resolver, &source_config) .await .map_err(IndexServiceError::InvalidConfig)?; let add_source_request = - AddSourceRequest::try_from_source_config(index_uid.clone(), source_config.clone())?; + AddSourceRequest::try_from_source_config(index_uid.clone(), &source_config)?; self.metastore.add_source(add_source_request).await?; info!( "source `{}` successfully created for index `{}`", @@ -420,7 +408,7 @@ pub async fn validate_storage_uri( mod tests { use quickwit_common::uri::Uri; - use quickwit_config::IndexConfig; + use quickwit_config::{IndexConfig, CLI_SOURCE_ID, INGEST_API_SOURCE_ID, INGEST_V2_SOURCE_ID}; use quickwit_metastore::{ metastore_for_test, MetastoreServiceExt, SplitMetadata, StageSplitsRequestExt, }; @@ -443,6 +431,12 @@ mod tests { .unwrap(); assert_eq!(index_metadata_0.index_id(), index_id); assert_eq!(index_metadata_0.index_uri(), &index_uri); + + assert_eq!(index_metadata_0.sources.len(), 3); + assert!(index_metadata_0.sources.contains_key(CLI_SOURCE_ID)); + assert!(index_metadata_0.sources.contains_key(INGEST_API_SOURCE_ID)); + assert!(index_metadata_0.sources.contains_key(INGEST_V2_SOURCE_ID)); + assert!(metastore .index_metadata(IndexMetadataRequest::for_index_id(index_id.to_string())) .await diff --git a/quickwit/quickwit-indexing/failpoints/mod.rs b/quickwit/quickwit-indexing/failpoints/mod.rs index 10dade36a90..fcca994712f 100644 --- a/quickwit/quickwit-indexing/failpoints/mod.rs +++ b/quickwit/quickwit-indexing/failpoints/mod.rs @@ -46,7 +46,7 @@ use quickwit_common::rand::append_random_suffix; use quickwit_common::split_file; use quickwit_common::temp_dir::TempDirectory; use quickwit_indexing::actors::MergeExecutor; -use quickwit_indexing::merge_policy::MergeOperation; +use quickwit_indexing::merge_policy::{MergeOperation, MergeTask}; use quickwit_indexing::models::MergeScratch; use quickwit_indexing::{get_tantivy_directory_from_split_bundle, TestSandbox}; use quickwit_metastore::{ @@ -57,7 +57,7 @@ use quickwit_proto::indexing::IndexingPipelineId; use quickwit_proto::metastore::{ListSplitsRequest, MetastoreService}; use quickwit_proto::types::{IndexUid, PipelineUid}; use serde_json::Value as JsonValue; -use tantivy::{Directory, Inventory}; +use tantivy::Directory; #[tokio::test] async fn test_failpoint_no_failure() -> anyhow::Result<()> { @@ -188,7 +188,7 @@ async fn aux_test_failpoints() -> anyhow::Result<()> { test_index_builder.add_documents(batch_2).await?; let query = ListSplitsQuery::for_index(test_index_builder.index_uid()) .with_split_state(SplitState::Published); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query).unwrap(); + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query).unwrap(); let mut splits = test_index_builder .metastore() .list_splits(list_splits_request) @@ -290,11 +290,10 @@ async fn test_merge_executor_controlled_directory_kill_switch() -> anyhow::Resul tantivy_dirs.push(get_tantivy_directory_from_split_bundle(&dest_filepath).unwrap()); } - let merge_ops_inventory = Inventory::new(); - let merge_operation = - merge_ops_inventory.track(MergeOperation::new_merge_operation(split_metadatas)); + let merge_operation = MergeOperation::new_merge_operation(split_metadatas); + let merge_task = MergeTask::from_merge_operation_for_test(merge_operation); let merge_scratch = MergeScratch { - merge_operation, + merge_task, merge_scratch_directory, downloaded_splits_directory, tantivy_dirs, diff --git a/quickwit/quickwit-indexing/src/actors/index_serializer.rs b/quickwit/quickwit-indexing/src/actors/index_serializer.rs index 47960384a45..d9a28cac8a4 100644 --- a/quickwit/quickwit-indexing/src/actors/index_serializer.rs +++ b/quickwit/quickwit-indexing/src/actors/index_serializer.rs @@ -95,7 +95,7 @@ impl Handler for IndexSerializer { checkpoint_delta_opt: batch_builder.checkpoint_delta_opt, publish_lock: batch_builder.publish_lock, publish_token_opt: batch_builder.publish_token_opt, - merge_operation_opt: None, + merge_task_opt: None, batch_parent_span: batch_builder.batch_parent_span, }; ctx.send_message(&self.packager_mailbox, indexed_split_batch) diff --git a/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs b/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs index 769d8c4e946..ca303e04fc1 100644 --- a/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs +++ b/quickwit/quickwit-indexing/src/actors/indexing_pipeline.rs @@ -650,7 +650,7 @@ mod tests { let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes/test-index"); return Ok( - IndexMetadataResponse::try_from_index_metadata(index_metadata).unwrap(), + IndexMetadataResponse::try_from_index_metadata(&index_metadata).unwrap(), ); } num_fails -= 1; @@ -763,7 +763,7 @@ mod tests { .returning(|_| { let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes/test-index"); - Ok(IndexMetadataResponse::try_from_index_metadata(index_metadata).unwrap()) + Ok(IndexMetadataResponse::try_from_index_metadata(&index_metadata).unwrap()) }); metastore .expect_last_delete_opstamp() @@ -863,7 +863,7 @@ mod tests { .returning(|_| { let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes/test-index"); - Ok(IndexMetadataResponse::try_from_index_metadata(index_metadata).unwrap()) + Ok(IndexMetadataResponse::try_from_index_metadata(&index_metadata).unwrap()) }); mock_metastore .expect_list_splits() @@ -898,6 +898,7 @@ mod tests { merge_policy: default_merge_policy(), max_concurrent_split_uploads: 2, merge_io_throughput_limiter_opt: None, + merge_scheduler_service: universe.get_or_spawn_one(), event_broker: Default::default(), }; let merge_pipeline = MergePipeline::new(merge_pipeline_params, universe.spawn_ctx()); @@ -959,7 +960,7 @@ mod tests { .returning(|_| { let index_metadata = IndexMetadata::for_test("test-index", "ram:///indexes/test-index"); - Ok(IndexMetadataResponse::try_from_index_metadata(index_metadata).unwrap()) + Ok(IndexMetadataResponse::try_from_index_metadata(&index_metadata).unwrap()) }); metastore .expect_last_delete_opstamp() diff --git a/quickwit/quickwit-indexing/src/actors/indexing_service.rs b/quickwit/quickwit-indexing/src/actors/indexing_service.rs index 81d2553ea44..290710450d7 100644 --- a/quickwit/quickwit-indexing/src/actors/indexing_service.rs +++ b/quickwit/quickwit-indexing/src/actors/indexing_service.rs @@ -57,7 +57,7 @@ use tokio::sync::Semaphore; use tracing::{debug, error, info, warn}; use super::merge_pipeline::{MergePipeline, MergePipelineParams}; -use super::MergePlanner; +use super::{MergePlanner, MergeSchedulerService}; use crate::models::{DetachIndexingPipeline, DetachMergePipeline, ObservePipeline, SpawnPipeline}; use crate::source::{AssignShards, Assignment}; use crate::split_store::{LocalSplitStore, SplitStoreQuota}; @@ -121,6 +121,7 @@ pub struct IndexingService { cluster: Cluster, metastore: MetastoreServiceClient, ingest_api_service_opt: Option>, + merge_scheduler_service: Mailbox, ingester_pool: IngesterPool, storage_resolver: StorageResolver, indexing_pipelines: HashMap, @@ -154,6 +155,7 @@ impl IndexingService { cluster: Cluster, metastore: MetastoreServiceClient, ingest_api_service_opt: Option>, + merge_scheduler_service: Mailbox, ingester_pool: IngesterPool, storage_resolver: StorageResolver, event_broker: EventBroker, @@ -182,6 +184,7 @@ impl IndexingService { cluster, metastore, ingest_api_service_opt, + merge_scheduler_service, ingester_pool, storage_resolver, local_split_store: Arc::new(local_split_store), @@ -297,6 +300,7 @@ impl IndexingService { indexing_directory: indexing_directory.clone(), metastore: self.metastore.clone(), split_store: split_store.clone(), + merge_scheduler_service: self.merge_scheduler_service.clone(), merge_policy: merge_policy.clone(), merge_io_throughput_limiter_opt: self.merge_io_throughput_limiter_opt.clone(), max_concurrent_split_uploads: self.max_concurrent_split_uploads, @@ -893,6 +897,7 @@ mod tests { init_ingest_api(universe, &queues_dir_path, &IngestApiConfig::default()) .await .unwrap(); + let merge_scheduler_mailbox: Mailbox = universe.get_or_spawn_one(); let indexing_server = IndexingService::new( "test-node".to_string(), data_dir_path.to_path_buf(), @@ -901,6 +906,7 @@ mod tests { cluster, metastore, Some(ingest_api_service), + merge_scheduler_mailbox, IngesterPool::default(), storage_resolver.clone(), EventBroker::default(), @@ -923,7 +929,8 @@ mod tests { let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(&index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -932,10 +939,11 @@ mod tests { .into(); let create_source_request = AddSourceRequest::try_from_source_config( index_uid.clone(), - SourceConfig::ingest_api_default(), + &SourceConfig::ingest_api_default(), ) .unwrap(); metastore.add_source(create_source_request).await.unwrap(); + let universe = Universe::with_accelerated_time(); let temp_dir = tempfile::tempdir().unwrap(); let (indexing_service, indexing_service_handle) = @@ -1024,7 +1032,8 @@ mod tests { let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(&index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); metastore.create_index(create_index_request).await.unwrap(); let universe = Universe::new(); @@ -1079,7 +1088,8 @@ mod tests { let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(&index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -1088,7 +1098,7 @@ mod tests { .into(); let add_source_request = AddSourceRequest::try_from_source_config( index_uid.clone(), - SourceConfig::ingest_api_default(), + &SourceConfig::ingest_api_default(), ) .unwrap(); metastore.add_source(add_source_request).await.unwrap(); @@ -1113,8 +1123,7 @@ mod tests { input_format: SourceInputFormat::Json, }; let add_source_request = - AddSourceRequest::try_from_source_config(index_uid.clone(), source_config_1.clone()) - .unwrap(); + AddSourceRequest::try_from_source_config(index_uid.clone(), &source_config_1).unwrap(); metastore.add_source(add_source_request).await.unwrap(); let metadata = metastore .index_metadata(IndexMetadataRequest::for_index_id(index_id.clone())) @@ -1164,8 +1173,7 @@ mod tests { input_format: SourceInputFormat::Json, }; let add_source_request_2 = - AddSourceRequest::try_from_source_config(index_uid.clone(), source_config_2.clone()) - .unwrap(); + AddSourceRequest::try_from_source_config(index_uid.clone(), &source_config_2).unwrap(); metastore.add_source(add_source_request_2).await.unwrap(); let indexing_tasks = vec![ @@ -1321,7 +1329,8 @@ mod tests { transform_config: None, input_format: SourceInputFormat::Json, }; - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -1329,8 +1338,7 @@ mod tests { .index_uid .into(); let add_source_request = - AddSourceRequest::try_from_source_config(index_uid.clone(), source_config.clone()) - .unwrap(); + AddSourceRequest::try_from_source_config(index_uid.clone(), &source_config).unwrap(); metastore.add_source(add_source_request).await.unwrap(); // Test `IndexingService::new`. @@ -1345,6 +1353,7 @@ mod tests { init_ingest_api(&universe, &queues_dir_path, &IngestApiConfig::default()) .await .unwrap(); + let merge_scheduler_service = universe.get_or_spawn_one(); let indexing_server = IndexingService::new( "test-node".to_string(), data_dir_path, @@ -1353,6 +1362,7 @@ mod tests { cluster.clone(), metastore.clone(), Some(ingest_api_service), + merge_scheduler_service, IngesterPool::default(), storage_resolver.clone(), EventBroker::default(), @@ -1465,7 +1475,7 @@ mod tests { Ok(list_indexes_metadatas_response) }); metastore.expect_index_metadata().returning(move |_| { - Ok(IndexMetadataResponse::try_from_index_metadata(index_metadata.clone()).unwrap()) + Ok(IndexMetadataResponse::try_from_index_metadata(&index_metadata).unwrap()) }); metastore .expect_list_splits() @@ -1519,7 +1529,8 @@ mod tests { .await .unwrap(); let mut metastore = metastore_for_test(); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -1548,6 +1559,7 @@ mod tests { let indexer_config = IndexerConfig::for_test().unwrap(); let num_blocking_threads = 1; let storage_resolver = StorageResolver::unconfigured(); + let merge_scheduler_service: Mailbox = universe.get_or_spawn_one(); let mut indexing_server = IndexingService::new( "test-ingest-api-gc-node".to_string(), data_dir_path, @@ -1556,6 +1568,7 @@ mod tests { cluster.clone(), metastore.clone(), Some(ingest_api_service.clone()), + merge_scheduler_service, IngesterPool::default(), storage_resolver.clone(), EventBroker::default(), diff --git a/quickwit/quickwit-indexing/src/actors/merge_executor.rs b/quickwit/quickwit-indexing/src/actors/merge_executor.rs index 030180d4a6f..f6579955745 100644 --- a/quickwit/quickwit-indexing/src/actors/merge_executor.rs +++ b/quickwit/quickwit-indexing/src/actors/merge_executor.rs @@ -85,19 +85,19 @@ impl Actor for MergeExecutor { impl Handler for MergeExecutor { type Reply = (); - #[instrument(level = "info", name = "merge_executor", parent = merge_scratch.merge_operation.merge_parent_span.id(), skip_all)] + #[instrument(level = "info", name = "merge_executor", parent = merge_scratch.merge_task.merge_parent_span.id(), skip_all)] async fn handle( &mut self, merge_scratch: MergeScratch, ctx: &ActorContext, ) -> Result<(), ActorExitStatus> { let start = Instant::now(); - let merge_op = merge_scratch.merge_operation; - let indexed_split_opt: Option = match merge_op.operation_type { + let merge_task = merge_scratch.merge_task; + let indexed_split_opt: Option = match merge_task.operation_type { MergeOperationType::Merge => Some( self.process_merge( - merge_op.merge_split_id.clone(), - merge_op.splits.clone(), + merge_task.merge_split_id.clone(), + merge_task.splits.clone(), merge_scratch.tantivy_dirs, merge_scratch.merge_scratch_directory, ctx, @@ -106,14 +106,14 @@ impl Handler for MergeExecutor { ), MergeOperationType::DeleteAndMerge => { assert_eq!( - merge_op.splits.len(), + merge_task.splits.len(), 1, "Delete tasks can be applied only on one split." ); assert_eq!(merge_scratch.tantivy_dirs.len(), 1); - let split_with_docs_to_delete = merge_op.splits[0].clone(); + let split_with_docs_to_delete = merge_task.splits[0].clone(); self.process_delete_and_merge( - merge_op.merge_split_id.clone(), + merge_task.merge_split_id.clone(), split_with_docs_to_delete, merge_scratch.tantivy_dirs, merge_scratch.merge_scratch_directory, @@ -126,7 +126,7 @@ impl Handler for MergeExecutor { info!( merged_num_docs = %indexed_split.split_attrs.num_docs, elapsed_secs = %start.elapsed().as_secs_f32(), - operation_type = %merge_op.operation_type, + operation_type = %merge_task.operation_type, "merge-operation-success" ); ctx.send_message( @@ -136,8 +136,8 @@ impl Handler for MergeExecutor { checkpoint_delta_opt: Default::default(), publish_lock: PublishLock::default(), publish_token_opt: None, - batch_parent_span: merge_op.merge_parent_span.clone(), - merge_operation_opt: Some(merge_op), + batch_parent_span: merge_task.merge_parent_span.clone(), + merge_task_opt: Some(merge_task), }, ) .await?; @@ -566,10 +566,10 @@ mod tests { DeleteQuery, ListSplitsRequest, PublishSplitsRequest, StageSplitsRequest, }; use serde_json::Value as JsonValue; - use tantivy::{Document, Inventory, ReloadPolicy, TantivyDocument}; + use tantivy::{Document, ReloadPolicy, TantivyDocument}; use super::*; - use crate::merge_policy::MergeOperation; + use crate::merge_policy::{MergeOperation, MergeTask}; use crate::{get_tantivy_directory_from_split_bundle, new_split_id, TestSandbox}; #[tokio::test] @@ -623,11 +623,10 @@ mod tests { .await?; tantivy_dirs.push(get_tantivy_directory_from_split_bundle(&dest_filepath).unwrap()) } - let merge_ops_inventory = Inventory::new(); - let merge_operation = - merge_ops_inventory.track(MergeOperation::new_merge_operation(split_metas)); + let merge_operation = MergeOperation::new_merge_operation(split_metas); + let merge_task = MergeTask::from_merge_operation_for_test(merge_operation); let merge_scratch = MergeScratch { - merge_operation, + merge_task, tantivy_dirs, merge_scratch_directory, downloaded_splits_directory, @@ -742,11 +741,9 @@ mod tests { let mut new_split_metadata = splits[0].split_metadata.clone(); new_split_metadata.split_id = new_split_id(); new_split_metadata.num_merge_ops = 1; - let stage_splits_request = StageSplitsRequest::try_from_split_metadata( - index_uid.clone(), - new_split_metadata.clone(), - ) - .unwrap(); + let stage_splits_request = + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &new_split_metadata) + .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let publish_splits_request = PublishSplitsRequest { index_uid: index_uid.to_string(), @@ -772,12 +769,10 @@ mod tests { .copy_to_file(Path::new(&split_filename), &dest_filepath) .await?; let tantivy_dir = get_tantivy_directory_from_split_bundle(&dest_filepath).unwrap(); - let merge_ops_inventory = Inventory::new(); - let merge_operation = merge_ops_inventory.track( - MergeOperation::new_delete_and_merge_operation(new_split_metadata), - ); + let merge_operation = MergeOperation::new_delete_and_merge_operation(new_split_metadata); + let merge_task = MergeTask::from_merge_operation_for_test(merge_operation); let merge_scratch = MergeScratch { - merge_operation, + merge_task, tantivy_dirs: vec![tantivy_dir], merge_scratch_directory, downloaded_splits_directory, diff --git a/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs b/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs index 04c28aaba3c..1d99d0adb77 100644 --- a/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs +++ b/quickwit/quickwit-indexing/src/actors/merge_pipeline.rs @@ -40,6 +40,7 @@ use quickwit_proto::metastore::{ use time::OffsetDateTime; use tracing::{debug, error, info, instrument}; +use super::MergeSchedulerService; use crate::actors::indexing_pipeline::wait_duration_before_retry; use crate::actors::merge_split_downloader::MergeSplitDownloader; use crate::actors::publisher::PublisherType; @@ -221,14 +222,13 @@ impl MergePipeline { let query = ListSplitsQuery::for_index(self.params.pipeline_id.index_uid.clone()) .with_split_state(SplitState::Published) .retain_immature(OffsetDateTime::now_utc()); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query)?; + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query)?; let published_splits_stream = ctx .protect_future(self.params.metastore.list_splits(list_splits_request)) .await?; let published_splits_metadata = ctx .protect_future(published_splits_stream.collect_splits_metadata()) .await?; - info!( num_splits = published_splits_metadata.len(), "loaded list of published splits" @@ -244,6 +244,11 @@ impl MergePipeline { let (merge_publisher_mailbox, merge_publisher_handler) = ctx .spawn_actor() .set_kill_switch(self.kill_switch.clone()) + .set_backpressure_micros_counter( + crate::metrics::INDEXER_METRICS + .backpressure_micros + .with_label_values(["merge_publisher"]), + ) .spawn(merge_publisher); // Merge uploader @@ -288,6 +293,11 @@ impl MergePipeline { let (merge_executor_mailbox, merge_executor_handler) = ctx .spawn_actor() .set_kill_switch(self.kill_switch.clone()) + .set_backpressure_micros_counter( + crate::metrics::INDEXER_METRICS + .backpressure_micros + .with_label_values(["merge_executor"]), + ) .spawn(merge_executor); let merge_split_downloader = MergeSplitDownloader { @@ -302,7 +312,7 @@ impl MergePipeline { .set_backpressure_micros_counter( crate::metrics::INDEXER_METRICS .backpressure_micros - .with_label_values(["MergeSplitDownloader"]), + .with_label_values(["merge_split_downloader"]), ) .spawn(merge_split_downloader); @@ -312,6 +322,7 @@ impl MergePipeline { published_splits_metadata, self.params.merge_policy.clone(), merge_split_downloader_mailbox, + self.params.merge_scheduler_service.clone(), ); let (_, merge_planner_handler) = ctx .spawn_actor() @@ -357,6 +368,9 @@ impl MergePipeline { handles.merge_planner.refresh_observe(); handles.merge_uploader.refresh_observe(); handles.merge_publisher.refresh_observe(); + let num_ongoing_merges = crate::metrics::INDEXER_METRICS + .ongoing_merge_operations + .get(); self.statistics = self .previous_generations_statistics .clone() @@ -366,13 +380,7 @@ impl MergePipeline { ) .set_generation(self.statistics.generation) .set_num_spawn_attempts(self.statistics.num_spawn_attempts) - .set_ongoing_merges( - handles - .merge_planner - .last_observation() - .ongoing_merge_operations - .len(), - ); + .set_ongoing_merges(usize::try_from(num_ongoing_merges).unwrap_or(0)); } async fn perform_health_check( @@ -455,6 +463,7 @@ pub struct MergePipelineParams { pub doc_mapper: Arc, pub indexing_directory: TempDirectory, pub metastore: MetastoreServiceClient, + pub merge_scheduler_service: Mailbox, pub split_store: IndexingSplitStore, pub merge_policy: Arc, pub max_concurrent_split_uploads: usize, //< TODO share with the indexing pipeline. @@ -515,6 +524,7 @@ mod tests { doc_mapper: Arc::new(default_doc_mapper_for_test()), indexing_directory: TempDirectory::for_test(), metastore: MetastoreServiceClient::from(metastore), + merge_scheduler_service: universe.get_or_spawn_one(), split_store, merge_policy: default_merge_policy(), max_concurrent_split_uploads: 2, diff --git a/quickwit/quickwit-indexing/src/actors/merge_planner.rs b/quickwit/quickwit-indexing/src/actors/merge_planner.rs index 79f57afc3aa..5fcc84f73fe 100644 --- a/quickwit/quickwit-indexing/src/actors/merge_planner.rs +++ b/quickwit/quickwit-indexing/src/actors/merge_planner.rs @@ -17,14 +17,11 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -use std::cmp::Reverse; use std::collections::{HashMap, HashSet}; use std::sync::Arc; use std::time::Instant; use async_trait::async_trait; -use itertools::Itertools; -use quickwit_actors::channel_with_priority::TrySendError; use quickwit_actors::{Actor, ActorContext, ActorExitStatus, Handler, Mailbox, QueueCapacity}; use quickwit_metastore::SplitMetadata; use quickwit_proto::indexing::IndexingPipelineId; @@ -33,9 +30,10 @@ use tantivy::Inventory; use time::OffsetDateTime; use tracing::{info, warn}; +use super::MergeSchedulerService; +use crate::actors::merge_scheduler_service::schedule_merge; use crate::actors::MergeSplitDownloader; use crate::merge_policy::MergeOperation; -use crate::metrics::INDEXER_METRICS; use crate::models::NewSplits; use crate::MergePolicy; @@ -61,13 +59,16 @@ pub struct MergePlanner { /// We incrementally build this set, by adding new splits to it. /// When it becomes too large, we entirely rebuild it. known_split_ids: HashSet, + known_split_ids_recompute_attempt_id: usize, merge_policy: Arc, merge_split_downloader_mailbox: Mailbox, + merge_scheduler_service: Mailbox, /// Inventory of ongoing merge operations. If everything goes well, /// a merge operation is dropped after the publish of the merged split. - /// Used for observability. + /// + /// It is used to GC the known_split_ids set. ongoing_merge_operations_inventory: Inventory, /// We use the actor start_time as a way to identify incarnations. @@ -75,27 +76,14 @@ pub struct MergePlanner { /// Since we recycle the mailbox of the merge planner, this incarnation /// makes it possible to ignore messages that where emitted from the previous /// instantiation. - /// - /// In particular, it is necessary to avoid ever increasing the number - /// `RefreshMetrics` loop, every time the `MergePlanner` is respawned. incarnation_started_at: Instant, } #[async_trait] impl Actor for MergePlanner { - type ObservableState = MergePlannerState; - - fn observable_state(&self) -> Self::ObservableState { - let ongoing_merge_operations = self - .ongoing_merge_operations_inventory - .list() - .iter() - .map(|tracked_operation| tracked_operation.as_ref().clone()) - .collect_vec(); - MergePlannerState { - ongoing_merge_operations, - } - } + type ObservableState = (); + + fn observable_state(&self) -> Self::ObservableState {} fn name(&self) -> String { "MergePlanner".to_string() @@ -106,13 +94,6 @@ impl Actor for MergePlanner { } async fn initialize(&mut self, ctx: &ActorContext) -> Result<(), ActorExitStatus> { - self.handle( - RefreshMetrics { - incarnation_started_at: self.incarnation_started_at, - }, - ctx, - ) - .await?; // We do not call the handle method directly and instead queue the message in order to drain // the recycled mailbox and get a consolidated vision of the set of published // splits, before scheduling any merge operation. See #3847 for more details. @@ -159,23 +140,11 @@ impl Handler for MergePlanner { ) -> Result<(), ActorExitStatus> { self.record_splits_if_necessary(new_splits.new_splits); self.send_merge_ops(ctx).await?; - if self.known_split_ids.len() >= self.num_known_splits_rebuild_threshold() { - self.known_split_ids = self.rebuild_known_split_ids(); - } self.recompute_known_splits_if_necessary(); Ok(()) } } -fn max_merge_ops(merge_op: &MergeOperation) -> usize { - merge_op - .splits_as_slice() - .iter() - .map(|split_metadata| split_metadata.num_merge_ops) - .max() - .unwrap_or(0) -} - impl MergePlanner { pub fn queue_capacity() -> QueueCapacity { // We cannot have a Queue capacity of 0 here because `try_send_self` @@ -188,6 +157,7 @@ impl MergePlanner { published_splits: Vec, merge_policy: Arc, merge_split_downloader_mailbox: Mailbox, + merge_scheduler_service: Mailbox, ) -> MergePlanner { let published_splits: Vec = published_splits .into_iter() @@ -195,10 +165,13 @@ impl MergePlanner { .collect(); let mut merge_planner = MergePlanner { known_split_ids: Default::default(), + known_split_ids_recompute_attempt_id: 0, partitioned_young_splits: Default::default(), merge_policy, merge_split_downloader_mailbox, + merge_scheduler_service, ongoing_merge_operations_inventory: Inventory::default(), + incarnation_started_at: Instant::now(), }; merge_planner.record_splits_if_necessary(published_splits); @@ -206,8 +179,7 @@ impl MergePlanner { } fn rebuild_known_split_ids(&self) -> HashSet { - let mut known_split_ids: HashSet = - HashSet::with_capacity(self.num_known_splits_rebuild_threshold()); + let mut known_split_ids: HashSet = HashSet::default(); // Add splits that in `partitioned_young_splits`. for young_split_partition in self.partitioned_young_splits.values() { for split in young_split_partition { @@ -242,43 +214,13 @@ impl MergePlanner { true } + // No need to rebuild every time, we do once out of 100 times. fn recompute_known_splits_if_necessary(&mut self) { - if self.known_split_ids.len() >= self.num_known_splits_rebuild_threshold() { + self.known_split_ids_recompute_attempt_id += 1; + if self.known_split_ids_recompute_attempt_id % 100 == 0 { self.known_split_ids = self.rebuild_known_split_ids(); + self.known_split_ids_recompute_attempt_id = 0; } - if cfg!(test) { - let merge_operation = self.ongoing_merge_operations_inventory.list(); - let mut young_splits = HashSet::new(); - for (&partition_id, young_splits_in_partition) in &self.partitioned_young_splits { - for split_metadata in young_splits_in_partition { - assert_eq!(split_metadata.partition_id, partition_id); - young_splits.insert(split_metadata.split_id()); - } - } - for merge_op in merge_operation { - assert!(!self.known_split_ids.contains(&merge_op.merge_split_id)); - for split_in_merge in merge_op.splits_as_slice() { - assert!(self.known_split_ids.contains(split_in_merge.split_id())); - } - } - assert!(self.known_split_ids.len() <= self.num_known_splits_rebuild_threshold() + 1); - } - } - - /// Whenever the number of known splits exceeds this threshold, we rebuild the `known_split_ids` - /// set. - /// - /// We have this function to return a number that is higher than 2 times the len of - /// `known_split_ids` after a rebuild to get amortization. - fn num_known_splits_rebuild_threshold(&self) -> usize { - // The idea behind this formula is that we expect the max legitimate of splits after a - // rebuild to be `num_young_splits` + `num_splits_merge`. - // The capacity of `partioned_young_splits` is a good upper bound for the number of - // partition. - // - // We can expect a maximum of 100 ongoing splits in merge per partition. (We oversize this - // because it actually depends on the merge factor. - 1 + self.num_young_splits() + (1 + self.partitioned_young_splits.capacity()) * 20 } // Record a split. This function does NOT check if the split is mature or not, or if the split @@ -331,70 +273,24 @@ impl MergePlanner { Ok(merge_operations) } - fn num_young_splits(&self) -> usize { - self.partitioned_young_splits - .values() - .map(|splits| splits.len()) - .sum() - } - async fn send_merge_ops(&mut self, ctx: &ActorContext) -> Result<(), ActorExitStatus> { - // We do not want to simply schedule all available merge operations here. + // We identify all of the merge operations we want to run and leave it + // to the merge scheduler to decide in which order these should be scheduled. // - // The reason is that in presence of partitioning, it is very possible - // to receive a set of splits opening the opportunity to run a lot "large" merge - // operations at the same time. - // - // These large merge operation will in turn prevent small merge operations - // from being executed, when in fact small merge operations should be executed - // in priority. - // - // As an alternative approach, this function push merge operations until it starts - // experience some push back, and then just "loops". - let mut merge_ops = self.compute_merge_ops(ctx).await?; - // We run smaller merges in priority. - merge_ops.sort_by_cached_key(|merge_op| Reverse(max_merge_ops(merge_op))); - while let Some(merge_operation) = merge_ops.pop() { - info!(merge_operation=?merge_operation, "planned merge operation"); + // The merge scheduler has the merit of knowing about merge operations from other + // index as well. + let merge_ops = self.compute_merge_ops(ctx).await?; + for merge_operation in merge_ops { + info!(merge_operation=?merge_operation, "schedule merge operation"); let tracked_merge_operation = self .ongoing_merge_operations_inventory .track(merge_operation); - if let Err(try_send_err) = self - .merge_split_downloader_mailbox - .try_send_message(tracked_merge_operation) - { - match try_send_err { - TrySendError::Disconnected => { - return Err(ActorExitStatus::DownstreamClosed); - } - TrySendError::Full(merge_op) => { - ctx.send_message(&self.merge_split_downloader_mailbox, merge_op) - .await?; - break; - } - } - } - } - if !merge_ops.is_empty() { - // We experienced some push back and decided to stop queueing too - // many merge operation. (For more detail see #2348) - // - // We need to re-record the related split, so that we - // perform a merge in the future. - for merge_op in merge_ops { - for split in merge_op.splits { - self.record_split(split); - } - } - // We try_self_send a `PlanMerge` message in order to ensure that - // progress on our merges. - // - // If `try_send_self_message` returns an error, it means that the - // the self queue is full, which means that the `PlanMerge` - // message is not really needed anyway. - let _ignored_result = ctx.try_send_self_message(PlanMerge { - incarnation_started_at: self.incarnation_started_at, - }); + schedule_merge( + &self.merge_scheduler_service, + tracked_merge_operation, + self.merge_split_downloader_mailbox.clone(), + ) + .await? } Ok(()) } @@ -407,43 +303,11 @@ fn belongs_to_pipeline(pipeline_id: &IndexingPipelineId, split: &SplitMetadata) && pipeline_id.node_id == split.node_id } -#[derive(Debug)] -struct RefreshMetrics { - incarnation_started_at: Instant, -} - #[derive(Debug)] struct PlanMerge { incarnation_started_at: Instant, } -#[async_trait] -impl Handler for MergePlanner { - type Reply = (); - - async fn handle( - &mut self, - refresh_metric: RefreshMetrics, - ctx: &ActorContext, - ) -> Result<(), ActorExitStatus> { - if self.incarnation_started_at != refresh_metric.incarnation_started_at { - // This message was emitted by a different incarnation. - // (See `Self::incarnation_started_at`) - return Ok(()); - } - INDEXER_METRICS - .ongoing_merge_operations - .set(self.ongoing_merge_operations_inventory.list().len() as i64); - ctx.schedule_self_msg( - *quickwit_actors::HEARTBEAT, - RefreshMetrics { - incarnation_started_at: self.incarnation_started_at, - }, - ); - Ok(()) - } -} - #[derive(Clone, Debug, Serialize)] pub struct MergePlannerState { pub(crate) ongoing_merge_operations: Vec, @@ -463,12 +327,11 @@ mod tests { use quickwit_metastore::{SplitMaturity, SplitMetadata}; use quickwit_proto::indexing::IndexingPipelineId; use quickwit_proto::types::{IndexUid, PipelineUid}; - use tantivy::TrackedObject; use time::OffsetDateTime; use crate::actors::MergePlanner; use crate::merge_policy::{ - merge_policy_from_settings, MergeOperation, MergePolicy, StableLogMergePolicy, + merge_policy_from_settings, MergePolicy, MergeTask, StableLogMergePolicy, }; use crate::models::NewSplits; @@ -521,6 +384,7 @@ mod tests { Vec::new(), merge_policy, merge_split_downloader_mailbox, + universe.get_or_spawn_one(), ); let (merge_planner_mailbox, merge_planner_handle) = @@ -561,8 +425,7 @@ mod tests { }; merge_planner_mailbox.send_message(message).await?; merge_planner_handle.process_pending_and_observe().await; - let operations = merge_split_downloader_inbox - .drain_for_test_typed::>(); + let operations = merge_split_downloader_inbox.drain_for_test_typed::(); assert_eq!(operations.len(), 2); let mut merge_operations = operations.into_iter().sorted_by(|left_op, right_op| { left_op.splits[0] @@ -581,155 +444,6 @@ mod tests { Ok(()) } - #[tokio::test] - async fn test_merge_planner_priority() -> anyhow::Result<()> { - let universe = Universe::with_accelerated_time(); - let (merge_split_downloader_mailbox, merge_split_downloader_inbox) = - universe.create_test_mailbox(); - let index_uid = IndexUid::new_with_random_ulid("test-index"); - let pipeline_id = IndexingPipelineId { - index_uid: index_uid.clone(), - source_id: "test-source".to_string(), - node_id: "test-node".to_string(), - pipeline_uid: PipelineUid::default(), - }; - let merge_policy_config = ConstWriteAmplificationMergePolicyConfig { - merge_factor: 2, - max_merge_factor: 2, - max_merge_ops: 3, - ..Default::default() - }; - let indexing_settings = IndexingSettings { - merge_policy: MergePolicyConfig::ConstWriteAmplification(merge_policy_config), - ..Default::default() - }; - let merge_policy: Arc = merge_policy_from_settings(&indexing_settings); - let merge_planner = MergePlanner::new( - pipeline_id, - Vec::new(), - merge_policy, - merge_split_downloader_mailbox, - ); - let (merge_planner_mailbox, merge_planner_handle) = - universe.spawn_builder().spawn(merge_planner); - // send 4 splits, offering 2 merge opportunities. - let message = NewSplits { - new_splits: vec![ - split_metadata_for_test(&index_uid, "2_a", 2, 100, 2), - split_metadata_for_test(&index_uid, "2_b", 2, 100, 2), - split_metadata_for_test(&index_uid, "1_a", 1, 10, 1), - split_metadata_for_test(&index_uid, "1_b", 1, 10, 1), - ], - }; - merge_planner_mailbox.send_message(message).await?; - merge_planner_handle.process_pending_and_observe().await; - let merge_ops: Vec> = - merge_split_downloader_inbox.drain_for_test_typed(); - assert_eq!(merge_ops.len(), 2); - assert_eq!(merge_ops[0].splits_as_slice()[0].num_merge_ops, 1); - assert_eq!(merge_ops[1].splits_as_slice()[0].num_merge_ops, 2); - universe.assert_quit().await; - Ok(()) - } - - #[tokio::test] - async fn test_merge_planner_priority_only_queue_up_to_capacity() { - let universe = Universe::with_accelerated_time(); - let (merge_split_downloader_mailbox, merge_split_downloader_inbox) = universe - .spawn_ctx() - .create_mailbox("MergeSplitDownloader", QueueCapacity::Bounded(2)); - let index_uid = IndexUid::new_with_random_ulid("test-index"); - let pipeline_id = IndexingPipelineId { - index_uid: index_uid.clone(), - source_id: "test-source".to_string(), - node_id: "test-node".to_string(), - pipeline_uid: PipelineUid::default(), - }; - let merge_policy_config = ConstWriteAmplificationMergePolicyConfig { - merge_factor: 2, - max_merge_factor: 2, - max_merge_ops: 3, - ..Default::default() - }; - let indexing_settings = IndexingSettings { - merge_policy: MergePolicyConfig::ConstWriteAmplification(merge_policy_config), - ..Default::default() - }; - let merge_policy: Arc = merge_policy_from_settings(&indexing_settings); - let merge_planner = MergePlanner::new( - pipeline_id, - Vec::new(), - merge_policy, - merge_split_downloader_mailbox, - ); - let universe = Universe::with_accelerated_time(); - let (merge_planner_mailbox, _) = universe.spawn_builder().spawn(merge_planner); - tokio::task::spawn(async move { - // Sending 20 splits offering 10 split opportunities - let messages_with_merge_ops2 = NewSplits { - new_splits: (0..10) - .flat_map(|partition_id| { - [ - split_metadata_for_test( - &index_uid, - &format!("{partition_id}_a_large"), - partition_id, - 1_000_000, - 2, - ), - split_metadata_for_test( - &index_uid, - &format!("{partition_id}_b_large"), - partition_id, - 1_000_000, - 2, - ), - ] - }) - .collect(), - }; - merge_planner_mailbox - .send_message(messages_with_merge_ops2) - .await - .unwrap(); - let messages_with_merge_ops1 = NewSplits { - new_splits: (0..10) - .flat_map(|partition_id| { - [ - split_metadata_for_test( - &index_uid, - &format!("{partition_id}_a_small"), - partition_id, - 100_000, - 1, - ), - split_metadata_for_test( - &index_uid, - &format!("{partition_id}_b_small"), - partition_id, - 100_000, - 1, - ), - ] - }) - .collect(), - }; - merge_planner_mailbox - .send_message(messages_with_merge_ops1) - .await - .unwrap(); - }); - tokio::task::spawn_blocking(move || { - let mut merge_ops: Vec> = Vec::new(); - while merge_ops.len() < 20 { - merge_ops.extend(merge_split_downloader_inbox.drain_for_test_typed()); - } - }) - .await - .unwrap(); - universe.assert_quit().await; - } - #[tokio::test] async fn test_merge_planner_spawns_merge_over_existing_splits_on_startup() -> anyhow::Result<()> { @@ -770,16 +484,17 @@ mod tests { pre_existing_splits.clone(), merge_policy, merge_split_downloader_mailbox, + universe.get_or_spawn_one(), ); let (merge_planner_mailbox, merge_planner_handle) = universe.spawn_builder().spawn(merge_planner); // We wait for the first merge ops. If we sent the Quit message right away, it would have // been queue before first `PlanMerge` message. - let merge_op = merge_split_downloader_inbox - .recv_typed_message::>() + let merge_task_res = merge_split_downloader_inbox + .recv_typed_message::() .await; - assert!(merge_op.is_some()); + assert!(merge_task_res.is_ok()); // We make sure that the known splits filtering set filters out splits are currently in // merge. @@ -791,8 +506,7 @@ mod tests { let _ = merge_planner_handle.process_pending_and_observe().await; - let merge_ops = - merge_split_downloader_inbox.drain_for_test_typed::>(); + let merge_ops = merge_split_downloader_inbox.drain_for_test_typed::(); assert!(merge_ops.is_empty()); @@ -800,8 +514,7 @@ mod tests { let (exit_status, _last_state) = merge_planner_handle.join().await; assert!(matches!(exit_status, ActorExitStatus::Quit)); - let merge_ops = - merge_split_downloader_inbox.drain_for_test_typed::>(); + let merge_ops = merge_split_downloader_inbox.drain_for_test_typed::(); assert!(merge_ops.is_empty()); universe.assert_quit().await; Ok(()) @@ -858,6 +571,7 @@ mod tests { pre_existing_splits.clone(), merge_policy, merge_split_downloader_mailbox, + universe.get_or_spawn_one(), ); let (merge_planner_mailbox, merge_planner_handle) = universe.spawn_builder().spawn(merge_planner); @@ -865,10 +579,9 @@ mod tests { merge_planner_mailbox.send_message(Command::Quit).await?; let (exit_status, _last_state) = merge_planner_handle.join().await; assert!(matches!(exit_status, ActorExitStatus::Quit)); - let merge_ops = - merge_split_downloader_inbox.drain_for_test_typed::>(); + let merge_tasks = merge_split_downloader_inbox.drain_for_test_typed::(); - assert!(merge_ops.is_empty()); + assert!(merge_tasks.is_empty()); universe.assert_quit().await; Ok(()) } @@ -914,8 +627,8 @@ mod tests { pre_existing_splits.clone(), merge_policy, merge_split_downloader_mailbox, + universe.get_or_spawn_one(), ); - let universe = Universe::with_accelerated_time(); // We create a fake old mailbox that contains two new splits and a PlanMerge message from an // old incarnation. This could happen in real life if the merge pipeline failed @@ -935,87 +648,18 @@ mod tests { // sent in the initialize method. // Instead, we wait for the first merge ops. - let merge_ops = merge_split_downloader_inbox - .recv_typed_message::>() + let merge_task_res = merge_split_downloader_inbox + .recv_typed_message::() .await; - assert!(merge_ops.is_some()); + assert!(merge_task_res.is_ok()); // At this point, our merge has been initialized. merge_planner_mailbox.send_message(Command::Quit).await?; let (exit_status, _last_state) = merge_planner_handle.join().await; assert!(matches!(exit_status, ActorExitStatus::Quit)); - let merge_ops = - merge_split_downloader_inbox.drain_for_test_typed::>(); - assert!(merge_ops.is_empty()); - - universe.assert_quit().await; - Ok(()) - } - - #[tokio::test] - async fn test_merge_planner_known_splits_set_size_stays_bounded() -> anyhow::Result<()> { - let universe = Universe::with_accelerated_time(); - let (merge_split_downloader_mailbox, merge_split_downloader_inbox) = universe - .spawn_ctx() - .create_mailbox("MergeSplitDownloader", QueueCapacity::Unbounded); - let index_uid = IndexUid::new_with_random_ulid("test-index"); - let pipeline_id = IndexingPipelineId { - index_uid: index_uid.clone(), - source_id: "test-source".to_string(), - node_id: "test-node".to_string(), - pipeline_uid: PipelineUid::default(), - }; - let merge_policy_config = ConstWriteAmplificationMergePolicyConfig { - merge_factor: 2, - max_merge_factor: 2, - max_merge_ops: 3, - ..Default::default() - }; - let indexing_settings = IndexingSettings { - merge_policy: MergePolicyConfig::ConstWriteAmplification(merge_policy_config), - ..Default::default() - }; - let merge_policy: Arc = merge_policy_from_settings(&indexing_settings); - let merge_planner = MergePlanner::new( - pipeline_id, - Vec::new(), - merge_policy, - merge_split_downloader_mailbox, - ); - let universe = Universe::with_accelerated_time(); - - // We spawn our merge planner with this recycled mailbox. - let (merge_planner_mailbox, merge_planner_handle) = - universe.spawn_builder().spawn(merge_planner); - - for j in 0..100 { - for i in 0..10 { - merge_planner_mailbox - .ask(NewSplits { - new_splits: vec![split_metadata_for_test( - &index_uid, - &format!("split_{}", j * 10 + i), - 0, - 1_000_000, - 1, - )], - }) - .await - .unwrap(); - } - // We drain the merge_ops to make sure merge ops are dropped (as if merges where - // successful) and that we are properly testing that the known_splits_set is - // bounded. - let merge_ops = merge_split_downloader_inbox - .drain_for_test_typed::>(); - assert_eq!(merge_ops.len(), 5); - } - - // At this point, our merge has been initialized. - merge_planner_mailbox.send_message(Command::Quit).await?; - let (exit_status, _last_state) = merge_planner_handle.join().await; - assert!(matches!(exit_status, ActorExitStatus::Quit)); + let merge_tasks = merge_split_downloader_inbox.drain_for_test_typed::(); + assert!(merge_tasks.is_empty()); universe.assert_quit().await; Ok(()) diff --git a/quickwit/quickwit-indexing/src/actors/merge_scheduler_service.rs b/quickwit/quickwit-indexing/src/actors/merge_scheduler_service.rs new file mode 100644 index 00000000000..91f29e16927 --- /dev/null +++ b/quickwit/quickwit-indexing/src/actors/merge_scheduler_service.rs @@ -0,0 +1,462 @@ +// Copyright (C) 2024 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 std::cmp::Reverse; +use std::collections::binary_heap::PeekMut; +use std::collections::BinaryHeap; +use std::sync::Arc; + +use anyhow::Context; +use async_trait::async_trait; +use quickwit_actors::{Actor, ActorContext, ActorExitStatus, Handler, Mailbox}; +use tantivy::TrackedObject; +use tokio::sync::{OwnedSemaphorePermit, Semaphore}; +use tracing::error; + +use super::MergeSplitDownloader; +use crate::merge_policy::{MergeOperation, MergeTask}; + +pub struct MergePermit { + _semaphore_permit: Option, + merge_scheduler_mailbox: Option>, +} + +impl MergePermit { + #[cfg(any(test, feature = "testsuite"))] + pub fn for_test() -> MergePermit { + MergePermit { + _semaphore_permit: None, + merge_scheduler_mailbox: None, + } + } +} + +impl Drop for MergePermit { + fn drop(&mut self) { + let Some(merge_scheduler_mailbox) = self.merge_scheduler_mailbox.take() else { + return; + }; + if merge_scheduler_mailbox + .send_message_with_high_priority(PermitReleased) + .is_err() + { + error!("merge scheduler service is dead"); + } + } +} + +pub async fn schedule_merge( + merge_scheduler_service: &Mailbox, + merge_operation: TrackedObject, + merge_split_downloader_mailbox: Mailbox, +) -> anyhow::Result<()> { + let schedule_merge = ScheduleMerge::new(merge_operation, merge_split_downloader_mailbox); + // TODO add backpressure. + merge_scheduler_service + .ask(schedule_merge) + .await + .context("failed to acquire permit")?; + Ok(()) +} + +struct ScheduledMerge { + score: u64, + id: u64, //< just for total ordering. + merge_operation: TrackedObject, + split_downloader_mailbox: Mailbox, +} + +impl ScheduledMerge { + fn order_key(&self) -> (u64, Reverse) { + (self.score, std::cmp::Reverse(self.id)) + } +} + +impl Eq for ScheduledMerge {} + +impl PartialEq for ScheduledMerge { + fn eq(&self, other: &Self) -> bool { + self.cmp(other).is_eq() + } +} + +impl PartialOrd for ScheduledMerge { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for ScheduledMerge { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.order_key().cmp(&other.order_key()) + } +} + +/// The merge scheduler service is in charge of keeping track of all scheduled merge operations, +/// and schedule them in the best possible order, respecting the `merge_concurrency` limit. +/// +/// This actor is not supervised and should stay as simple as possible. +/// In particular, +/// - the `ScheduleMerge` handler should reply in microseconds. +/// - the task should never be dropped before reaching its `split_downloader_mailbox` destination +/// as it would break the consistency of `MergePlanner` with the metastore (ie: several splits will +/// never be merged). +pub struct MergeSchedulerService { + merge_semaphore: Arc, + merge_concurrency: usize, + pending_merge_queue: BinaryHeap, + next_merge_id: u64, + pending_merge_bytes: u64, +} + +impl Default for MergeSchedulerService { + fn default() -> MergeSchedulerService { + MergeSchedulerService::new(3) + } +} + +impl MergeSchedulerService { + pub fn new(merge_concurrency: usize) -> MergeSchedulerService { + let merge_semaphore = Arc::new(Semaphore::new(merge_concurrency)); + MergeSchedulerService { + merge_semaphore, + merge_concurrency, + pending_merge_queue: BinaryHeap::default(), + next_merge_id: 0, + pending_merge_bytes: 0, + } + } + + fn schedule_pending_merges(&mut self, ctx: &ActorContext) { + // We schedule as many pending merges as we can, + // until there are no permits available or merges to schedule. + loop { + let merge_semaphore = self.merge_semaphore.clone(); + let Some(next_merge) = self.pending_merge_queue.peek_mut() else { + // No merge to schedule. + break; + }; + let Ok(semaphore_permit) = Semaphore::try_acquire_owned(merge_semaphore) else { + // No permit available right away. + break; + }; + let merge_permit = MergePermit { + _semaphore_permit: Some(semaphore_permit), + merge_scheduler_mailbox: Some(ctx.mailbox().clone()), + }; + let ScheduledMerge { + merge_operation, + split_downloader_mailbox, + .. + } = PeekMut::pop(next_merge); + let merge_task = MergeTask { + merge_operation, + _merge_permit: merge_permit, + }; + self.pending_merge_bytes -= merge_task.merge_operation.total_num_bytes(); + crate::metrics::INDEXER_METRICS + .pending_merge_operations + .set(self.pending_merge_queue.len() as i64); + crate::metrics::INDEXER_METRICS + .pending_merge_bytes + .set(self.pending_merge_bytes as i64); + match split_downloader_mailbox.try_send_message(merge_task) { + Ok(_) => {} + Err(quickwit_actors::TrySendError::Full(_)) => { + // The split downloader mailbox has an unbounded queue capacity, + error!("split downloader queue is full: please report"); + } + Err(quickwit_actors::TrySendError::Disconnected) => { + // It means the split downloader is dead. + // This is fine, the merge pipeline has probably been restarted. + } + } + } + let num_merges = + self.merge_concurrency as i64 - self.merge_semaphore.available_permits() as i64; + crate::metrics::INDEXER_METRICS + .ongoing_merge_operations + .set(num_merges); + } +} + +#[async_trait] +impl Actor for MergeSchedulerService { + type ObservableState = (); + + fn observable_state(&self) {} + + async fn initialize(&mut self, _ctx: &ActorContext) -> Result<(), ActorExitStatus> { + Ok(()) + } +} + +#[derive(Debug)] +struct ScheduleMerge { + score: u64, + merge_operation: TrackedObject, + split_downloader_mailbox: Mailbox, +} + +/// The higher, the sooner we will execute the merge operation. +/// A good merge operation +/// - strongly reduces the number splits +/// - is light. +fn score_merge_operation(merge_operation: &MergeOperation) -> u64 { + let total_num_bytes: u64 = merge_operation.total_num_bytes(); + if total_num_bytes == 0 { + // Silly corner case that should never happen. + return u64::MAX; + } + // We will remove splits.len() and add 1 merge splits. + let delta_num_splits = (merge_operation.splits.len() - 1) as u64; + // We use integer arithmetic to avoid `f64 are not ordered` silliness. + (delta_num_splits << 48) + .checked_div(total_num_bytes) + .unwrap_or(1u64) +} + +impl ScheduleMerge { + pub fn new( + merge_operation: TrackedObject, + split_downloader_mailbox: Mailbox, + ) -> ScheduleMerge { + let score = score_merge_operation(&merge_operation); + ScheduleMerge { + score, + merge_operation, + split_downloader_mailbox, + } + } +} + +#[async_trait] +impl Handler for MergeSchedulerService { + type Reply = (); + + async fn handle( + &mut self, + schedule_merge: ScheduleMerge, + ctx: &ActorContext, + ) -> Result<(), ActorExitStatus> { + let ScheduleMerge { + score, + merge_operation, + split_downloader_mailbox, + } = schedule_merge; + let merge_id = self.next_merge_id; + self.next_merge_id += 1; + let scheduled_merge = ScheduledMerge { + score, + id: merge_id, + merge_operation, + split_downloader_mailbox, + }; + self.pending_merge_bytes += scheduled_merge.merge_operation.total_num_bytes(); + self.pending_merge_queue.push(scheduled_merge); + crate::metrics::INDEXER_METRICS + .pending_merge_operations + .set(self.pending_merge_queue.len() as i64); + crate::metrics::INDEXER_METRICS + .pending_merge_bytes + .set(self.pending_merge_bytes as i64); + self.schedule_pending_merges(ctx); + Ok(()) + } +} + +#[derive(Debug)] +struct PermitReleased; + +#[async_trait] +impl Handler for MergeSchedulerService { + type Reply = (); + + async fn handle( + &mut self, + _: PermitReleased, + ctx: &ActorContext, + ) -> Result<(), ActorExitStatus> { + self.schedule_pending_merges(ctx); + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::time::Duration; + + use quickwit_actors::Universe; + use quickwit_metastore::SplitMetadata; + use tantivy::Inventory; + use tokio::time::timeout; + + use super::*; + use crate::merge_policy::{MergeOperation, MergeTask}; + + fn build_merge_operation(num_splits: usize, num_bytes_per_split: u64) -> MergeOperation { + let splits: Vec = std::iter::repeat_with(|| SplitMetadata { + footer_offsets: num_bytes_per_split..num_bytes_per_split, + ..Default::default() + }) + .take(num_splits) + .collect(); + MergeOperation::new_merge_operation(splits) + } + + #[test] + fn test_score_merge_operation() { + let score_merge_operation_aux = |num_splits, num_bytes_per_split| { + let merge_operation = build_merge_operation(num_splits, num_bytes_per_split); + score_merge_operation(&merge_operation) + }; + assert!(score_merge_operation_aux(10, 10_000_000) < score_merge_operation_aux(10, 999_999)); + assert!( + score_merge_operation_aux(10, 10_000_000) > score_merge_operation_aux(9, 10_000_000) + ); + assert_eq!( + // 9 - 1 = 8 splits removed. + score_merge_operation_aux(9, 10_000_000), + // 5 - 1 = 4 splits removed. + score_merge_operation_aux(5, 10_000_000 * 9 / 10) + ); + } + + #[tokio::test] + async fn test_merge_schedule_service_prioritize() { + let universe = Universe::new(); + let (merge_scheduler_service, _) = universe + .spawn_builder() + .spawn(MergeSchedulerService::new(2)); + let inventory = Inventory::new(); + + let (merge_split_downloader_mailbox, merge_split_downloader_inbox) = + universe.create_test_mailbox(); + { + let large_merge_operation = build_merge_operation(10, 4_000_000); + let tracked_large_merge_operation = inventory.track(large_merge_operation); + schedule_merge( + &merge_scheduler_service, + tracked_large_merge_operation, + merge_split_downloader_mailbox.clone(), + ) + .await + .unwrap(); + } + { + let large_merge_operation2 = build_merge_operation(10, 3_000_000); + let tracked_large_merge_operation2 = inventory.track(large_merge_operation2); + schedule_merge( + &merge_scheduler_service, + tracked_large_merge_operation2, + merge_split_downloader_mailbox.clone(), + ) + .await + .unwrap(); + } + { + let large_merge_operation2 = build_merge_operation(10, 5_000_000); + let tracked_large_merge_operation2 = inventory.track(large_merge_operation2); + schedule_merge( + &merge_scheduler_service, + tracked_large_merge_operation2, + merge_split_downloader_mailbox.clone(), + ) + .await + .unwrap(); + } + { + let large_merge_operation2 = build_merge_operation(10, 2_000_000); + let tracked_large_merge_operation2 = inventory.track(large_merge_operation2); + schedule_merge( + &merge_scheduler_service, + tracked_large_merge_operation2, + merge_split_downloader_mailbox.clone(), + ) + .await + .unwrap(); + } + { + let large_merge_operation2 = build_merge_operation(10, 1_000_000); + let tracked_large_merge_operation2 = inventory.track(large_merge_operation2); + schedule_merge( + &merge_scheduler_service, + tracked_large_merge_operation2, + merge_split_downloader_mailbox.clone(), + ) + .await + .unwrap(); + } + { + let merge_task: MergeTask = merge_split_downloader_inbox + .recv_typed_message::() + .await + .unwrap(); + assert_eq!( + merge_task.merge_operation.splits[0].footer_offsets.end, + 4_000_000 + ); + let merge_task2: MergeTask = merge_split_downloader_inbox + .recv_typed_message::() + .await + .unwrap(); + assert_eq!( + merge_task2.merge_operation.splits[0].footer_offsets.end, + 3_000_000 + ); + assert!(timeout( + Duration::from_millis(200), + merge_split_downloader_inbox.recv_typed_message::() + ) + .await + .is_err()); + } + { + let merge_task: MergeTask = merge_split_downloader_inbox + .recv_typed_message::() + .await + .unwrap(); + assert_eq!( + merge_task.merge_operation.splits[0].footer_offsets.end, + 1_000_000 + ); + } + { + let merge_task: MergeTask = merge_split_downloader_inbox + .recv_typed_message::() + .await + .unwrap(); + assert_eq!( + merge_task.merge_operation.splits[0].footer_offsets.end, + 2_000_000 + ); + } + { + let merge_task: MergeTask = merge_split_downloader_inbox + .recv_typed_message::() + .await + .unwrap(); + assert_eq!( + merge_task.merge_operation.splits[0].footer_offsets.end, + 5_000_000 + ); + } + universe.assert_quit().await; + } +} diff --git a/quickwit/quickwit-indexing/src/actors/merge_split_downloader.rs b/quickwit/quickwit-indexing/src/actors/merge_split_downloader.rs index 0e80c23e4a9..09a782f72c1 100644 --- a/quickwit/quickwit-indexing/src/actors/merge_split_downloader.rs +++ b/quickwit/quickwit-indexing/src/actors/merge_split_downloader.rs @@ -24,11 +24,11 @@ use quickwit_actors::{Actor, ActorContext, ActorExitStatus, Handler, Mailbox, Qu use quickwit_common::io::IoControls; use quickwit_common::temp_dir::{self, TempDirectory}; use quickwit_metastore::SplitMetadata; -use tantivy::{Directory, TrackedObject}; +use tantivy::Directory; use tracing::{debug, info, instrument}; use super::MergeExecutor; -use crate::merge_policy::MergeOperation; +use crate::merge_policy::MergeTask; use crate::models::MergeScratch; use crate::split_store::IndexingSplitStore; @@ -45,7 +45,7 @@ impl Actor for MergeSplitDownloader { fn observable_state(&self) -> Self::ObservableState {} fn queue_capacity(&self) -> QueueCapacity { - QueueCapacity::Bounded(1) + QueueCapacity::Unbounded } fn name(&self) -> String { @@ -54,17 +54,17 @@ impl Actor for MergeSplitDownloader { } #[async_trait] -impl Handler> for MergeSplitDownloader { +impl Handler for MergeSplitDownloader { type Reply = (); #[instrument( name = "merge_split_downloader", - parent = merge_operation.merge_parent_span.id(), + parent = merge_task.merge_parent_span.id(), skip_all, )] async fn handle( &mut self, - merge_operation: TrackedObject, + merge_task: MergeTask, ctx: &ActorContext, ) -> Result<(), quickwit_actors::ActorExitStatus> { let merge_scratch_directory = temp_dir::Builder::default() @@ -78,13 +78,13 @@ impl Handler> for MergeSplitDownloader { .map_err(|error| anyhow::anyhow!(error))?; let tantivy_dirs = self .download_splits( - merge_operation.splits_as_slice(), + merge_task.splits_as_slice(), downloaded_splits_directory.path(), ctx, ) .await?; let msg = MergeScratch { - merge_operation, + merge_task, merge_scratch_directory, downloaded_splits_directory, tantivy_dirs, @@ -139,9 +139,9 @@ mod tests { use quickwit_actors::Universe; use quickwit_common::split_file; use quickwit_storage::{PutPayload, RamStorageBuilder, SplitPayloadBuilder}; - use tantivy::Inventory; use super::*; + use crate::merge_policy::MergeOperation; use crate::new_split_id; #[tokio::test] @@ -179,10 +179,10 @@ mod tests { }; let (merge_split_downloader_mailbox, merge_split_downloader_handler) = universe.spawn_builder().spawn(merge_split_downloader); - let inventory = Inventory::new(); - let merge_operation = inventory.track(MergeOperation::new_merge_operation(splits_to_merge)); + let merge_operation: MergeOperation = MergeOperation::new_merge_operation(splits_to_merge); + let merge_task = MergeTask::from_merge_operation_for_test(merge_operation); merge_split_downloader_mailbox - .send_message(merge_operation) + .send_message(merge_task) .await?; merge_split_downloader_handler .process_pending_and_observe() @@ -195,8 +195,8 @@ mod tests { .unwrap() .downcast::() .unwrap(); - assert_eq!(merge_scratch.merge_operation.splits_as_slice().len(), 10); - for split in merge_scratch.merge_operation.splits_as_slice() { + assert_eq!(merge_scratch.merge_task.splits_as_slice().len(), 10); + for split in merge_scratch.merge_task.splits_as_slice() { let split_filename = split_file(split.split_id()); let split_filepath = merge_scratch .downloaded_splits_directory diff --git a/quickwit/quickwit-indexing/src/actors/mod.rs b/quickwit/quickwit-indexing/src/actors/mod.rs index e85a4289ef8..6c7befd41ba 100644 --- a/quickwit/quickwit-indexing/src/actors/mod.rs +++ b/quickwit/quickwit-indexing/src/actors/mod.rs @@ -25,6 +25,7 @@ mod indexing_service; mod merge_executor; mod merge_pipeline; mod merge_planner; +mod merge_scheduler_service; mod merge_split_downloader; mod packager; mod publisher; @@ -43,6 +44,7 @@ pub use indexing_service::{ pub use merge_executor::{combine_partition_ids, merge_split_attrs, MergeExecutor}; pub use merge_pipeline::MergePipeline; pub use merge_planner::MergePlanner; +pub use merge_scheduler_service::{schedule_merge, MergePermit, MergeSchedulerService}; pub use merge_split_downloader::MergeSplitDownloader; pub use packager::Packager; pub use publisher::{Publisher, PublisherCounters, PublisherType}; diff --git a/quickwit/quickwit-indexing/src/actors/packager.rs b/quickwit/quickwit-indexing/src/actors/packager.rs index d6f24e6fb37..d47c0b59892 100644 --- a/quickwit/quickwit-indexing/src/actors/packager.rs +++ b/quickwit/quickwit-indexing/src/actors/packager.rs @@ -35,8 +35,9 @@ use quickwit_doc_mapper::NamedField; use quickwit_proto::search::{ serialize_split_fields, ListFieldType, ListFields, ListFieldsEntryResponse, }; +use tantivy::index::FieldMetadata; use tantivy::schema::{FieldType, Type}; -use tantivy::{FieldMetadata, InvertedIndexReader, ReloadPolicy, SegmentMeta}; +use tantivy::{InvertedIndexReader, ReloadPolicy, SegmentMeta}; use tokio::runtime::Handle; use tracing::{debug, info, instrument, warn}; @@ -159,7 +160,7 @@ impl Handler for Packager { batch.checkpoint_delta_opt, batch.publish_lock, batch.publish_token_opt, - batch.merge_operation_opt, + batch.merge_task_opt, batch.batch_parent_span, ), ) @@ -573,7 +574,7 @@ mod tests { checkpoint_delta_opt: IndexCheckpointDelta::for_test("source_id", 10..20).into(), publish_lock: PublishLock::default(), publish_token_opt: None, - merge_operation_opt: None, + merge_task_opt: None, batch_parent_span: Span::none(), }) .await?; diff --git a/quickwit/quickwit-indexing/src/actors/publisher.rs b/quickwit/quickwit-indexing/src/actors/publisher.rs index 435b568a2a4..5011d54b10c 100644 --- a/quickwit/quickwit-indexing/src/actors/publisher.rs +++ b/quickwit/quickwit-indexing/src/actors/publisher.rs @@ -251,7 +251,7 @@ mod tests { }), publish_lock: PublishLock::default(), publish_token_opt: None, - merge_operation: None, + merge_task: None, parent_span: tracing::Span::none(), }) .await @@ -322,7 +322,7 @@ mod tests { }), publish_lock: PublishLock::default(), publish_token_opt: None, - merge_operation: None, + merge_task: None, parent_span: tracing::Span::none(), }) .await @@ -386,7 +386,7 @@ mod tests { checkpoint_delta_opt: None, publish_lock: PublishLock::default(), publish_token_opt: None, - merge_operation: None, + merge_task: None, parent_span: Span::none(), }; assert!(publisher_mailbox @@ -428,7 +428,7 @@ mod tests { checkpoint_delta_opt: None, publish_lock, publish_token_opt: None, - merge_operation: None, + merge_task: None, parent_span: Span::none(), }) .await diff --git a/quickwit/quickwit-indexing/src/actors/uploader.rs b/quickwit/quickwit-indexing/src/actors/uploader.rs index ee691f322ff..38df82ef04c 100644 --- a/quickwit/quickwit-indexing/src/actors/uploader.rs +++ b/quickwit/quickwit-indexing/src/actors/uploader.rs @@ -38,14 +38,13 @@ use quickwit_proto::search::{ReportSplit, ReportSplitsRequest}; use quickwit_proto::types::{IndexUid, PublishToken}; use quickwit_storage::SplitPayloadBuilder; use serde::Serialize; -use tantivy::TrackedObject; use tokio::sync::oneshot::Sender; use tokio::sync::{oneshot, Semaphore, SemaphorePermit}; use tracing::{debug, info, instrument, warn, Instrument, Span}; use crate::actors::sequencer::{Sequencer, SequencerCommand}; use crate::actors::Publisher; -use crate::merge_policy::{MergeOperation, MergePolicy}; +use crate::merge_policy::{MergePolicy, MergeTask}; use crate::metrics::INDEXER_METRICS; use crate::models::{ create_split_metadata, EmptySplit, PackagedSplit, PackagedSplitBatch, PublishLock, SplitsUpdate, @@ -373,7 +372,7 @@ impl Handler for Uploader { batch.checkpoint_delta_opt, batch.publish_lock, batch.publish_token_opt, - batch.merge_operation_opt, + batch.merge_task_opt, batch.batch_parent_span, ); @@ -416,7 +415,7 @@ impl Handler for Uploader { checkpoint_delta_opt: Some(empty_split.checkpoint_delta), publish_lock: empty_split.publish_lock, publish_token_opt: empty_split.publish_token_opt, - merge_operation: None, + merge_task: None, parent_span: empty_split.batch_parent_span, }; @@ -431,7 +430,7 @@ fn make_publish_operation( checkpoint_delta_opt: Option, publish_lock: PublishLock, publish_token_opt: Option, - merge_operation: Option>, + merge_task: Option, parent_span: Span, ) -> SplitsUpdate { assert!(!packaged_splits_and_metadatas.is_empty()); @@ -449,7 +448,7 @@ fn make_publish_operation( checkpoint_delta_opt, publish_lock, publish_token_opt, - merge_operation, + merge_task, parent_span, } } diff --git a/quickwit/quickwit-indexing/src/lib.rs b/quickwit/quickwit-indexing/src/lib.rs index ec87f01a132..53a0b37b3a9 100644 --- a/quickwit/quickwit-indexing/src/lib.rs +++ b/quickwit/quickwit-indexing/src/lib.rs @@ -29,6 +29,7 @@ use quickwit_proto::metastore::MetastoreServiceClient; use quickwit_storage::StorageResolver; use tracing::info; +use crate::actors::MergeSchedulerService; pub use crate::actors::{ IndexingError, IndexingPipeline, IndexingPipelineParams, IndexingService, PublisherType, Sequencer, SplitsUpdateMailbox, @@ -70,13 +71,15 @@ pub async fn start_indexing_service( num_blocking_threads: usize, cluster: Cluster, metastore: MetastoreServiceClient, - ingest_api_service: Mailbox, ingester_pool: IngesterPool, storage_resolver: StorageResolver, event_broker: EventBroker, ) -> anyhow::Result> { info!("starting indexer service"); - + let ingest_api_service_mailbox = universe.get_one::(); + let (merge_scheduler_mailbox, _) = universe.spawn_builder().spawn(MergeSchedulerService::new( + config.indexer_config.merge_concurrency.get(), + )); // Spawn indexing service. let indexing_service = IndexingService::new( config.node_id.clone(), @@ -85,13 +88,13 @@ pub async fn start_indexing_service( num_blocking_threads, cluster, metastore.clone(), - Some(ingest_api_service), + ingest_api_service_mailbox, + merge_scheduler_mailbox, ingester_pool, storage_resolver, event_broker, ) .await?; let (indexing_service, _) = universe.spawn_builder().spawn(indexing_service); - Ok(indexing_service) } diff --git a/quickwit/quickwit-indexing/src/merge_policy/mod.rs b/quickwit/quickwit-indexing/src/merge_policy/mod.rs index cbda92b5c0a..06d63fb41f1 100644 --- a/quickwit/quickwit-indexing/src/merge_policy/mod.rs +++ b/quickwit/quickwit-indexing/src/merge_policy/mod.rs @@ -22,6 +22,7 @@ mod nop_merge_policy; mod stable_log_merge_policy; use std::fmt; +use std::ops::Deref; use std::sync::Arc; pub(crate) use const_write_amplification::ConstWriteAmplificationMergePolicy; @@ -32,8 +33,10 @@ use quickwit_config::IndexingSettings; use quickwit_metastore::{SplitMaturity, SplitMetadata}; use serde::Serialize; pub(crate) use stable_log_merge_policy::StableLogMergePolicy; +use tantivy::TrackedObject; use tracing::{info_span, Span}; +use crate::actors::MergePermit; use crate::new_split_id; #[derive(Clone, Debug, PartialEq, Eq, Serialize)] @@ -48,6 +51,37 @@ impl fmt::Display for MergeOperationType { } } +pub struct MergeTask { + pub merge_operation: TrackedObject, + pub(crate) _merge_permit: MergePermit, +} + +impl MergeTask { + #[cfg(any(test, feature = "testsuite"))] + pub fn from_merge_operation_for_test(merge_operation: MergeOperation) -> MergeTask { + let inventory = tantivy::Inventory::default(); + let tracked_merge_operation = inventory.track(merge_operation); + MergeTask { + merge_operation: tracked_merge_operation, + _merge_permit: MergePermit::for_test(), + } + } +} + +impl fmt::Debug for MergeTask { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.merge_operation.as_ref().fmt(f) + } +} + +impl Deref for MergeTask { + type Target = MergeOperation; + + fn deref(&self) -> &Self::Target { + self.merge_operation.as_ref() + } +} + #[derive(Clone, Serialize)] pub struct MergeOperation { #[serde(skip_serializing)] @@ -70,6 +104,13 @@ impl MergeOperation { } } + pub fn total_num_bytes(&self) -> u64 { + self.splits + .iter() + .map(|split: &SplitMetadata| split.footer_offsets.end) + .sum() + } + pub fn new_delete_and_merge_operation(split: SplitMetadata) -> Self { let merge_split_id = new_split_id(); let merge_parent_span = info_span!("delete", merge_split_id=%merge_split_id, split_ids=?split.split_id(), typ=%MergeOperationType::DeleteAndMerge); @@ -172,11 +213,12 @@ pub mod tests { use quickwit_proto::indexing::IndexingPipelineId; use quickwit_proto::types::{IndexUid, PipelineUid}; use rand::seq::SliceRandom; - use tantivy::TrackedObject; use time::OffsetDateTime; use super::*; - use crate::actors::{merge_split_attrs, MergePlanner, MergeSplitDownloader}; + use crate::actors::{ + merge_split_attrs, MergePlanner, MergeSchedulerService, MergeSplitDownloader, + }; use crate::models::{create_split_metadata, NewSplits}; fn pow_of_10(n: usize) -> usize { @@ -360,7 +402,7 @@ pub mod tests { check_final_configuration: CheckFn, ) -> anyhow::Result> { let universe = Universe::new(); - let (merge_op_mailbox, merge_op_inbox) = + let (merge_task_mailbox, merge_task_inbox) = universe.create_test_mailbox::(); let pipeline_id = IndexingPipelineId { index_uid: IndexUid::new_with_random_ulid("test-index"), @@ -372,7 +414,8 @@ pub mod tests { pipeline_id, Vec::new(), merge_policy.clone(), - merge_op_mailbox, + merge_task_mailbox, + universe.get_or_spawn_one::(), ); let mut split_index: HashMap = HashMap::default(); let (merge_planner_mailbox, merge_planner_handler) = @@ -388,12 +431,11 @@ pub mod tests { loop { let obs = merge_planner_handler.process_pending_and_observe().await; assert_eq!(obs.obs_type, quickwit_actors::ObservationType::Alive); - let merge_ops = - merge_op_inbox.drain_for_test_typed::>(); - if merge_ops.is_empty() { + let merge_tasks = merge_task_inbox.drain_for_test_typed::(); + if merge_tasks.is_empty() { break; } - let new_splits: Vec = merge_ops + let new_splits: Vec = merge_tasks .into_iter() .map(|merge_op| apply_merge(&merge_policy, &mut split_index, &merge_op)) .collect(); diff --git a/quickwit/quickwit-indexing/src/merge_policy/stable_log_merge_policy.rs b/quickwit/quickwit-indexing/src/merge_policy/stable_log_merge_policy.rs index 84367bcc701..e971abb71d4 100644 --- a/quickwit/quickwit-indexing/src/merge_policy/stable_log_merge_policy.rs +++ b/quickwit/quickwit-indexing/src/merge_policy/stable_log_merge_policy.rs @@ -699,6 +699,24 @@ mod tests { } } + #[tokio::test] + async fn test_simulate_stable_log_merge_planner_edge_case() { + let merge_policy = StableLogMergePolicy::default(); + let batch_num_docs = vec![ + 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, 11, + ]; + aux_test_simulate_merge_planner_num_docs( + Arc::new(merge_policy.clone()), + &batch_num_docs, + |splits| { + let num_docs = splits.iter().map(|split| split.num_docs as u64).sum(); + assert!(splits.len() <= merge_policy.max_num_splits_worst_case(num_docs)); + }, + ) + .await + .unwrap(); + } + #[tokio::test] async fn test_simulate_stable_log_merge_planner_ideal_case() -> anyhow::Result<()> { let merge_policy = StableLogMergePolicy::default(); diff --git a/quickwit/quickwit-indexing/src/metrics.rs b/quickwit/quickwit-indexing/src/metrics.rs index 071a605efd7..e24cc3872d0 100644 --- a/quickwit/quickwit-indexing/src/metrics.rs +++ b/quickwit/quickwit-indexing/src/metrics.rs @@ -28,6 +28,8 @@ pub struct IndexerMetrics { pub backpressure_micros: IntCounterVec<1>, pub available_concurrent_upload_permits: IntGaugeVec<1>, pub ongoing_merge_operations: IntGauge, + pub pending_merge_operations: IntGauge, + pub pending_merge_bytes: IntGauge, } impl Default for IndexerMetrics { @@ -65,6 +67,16 @@ impl Default for IndexerMetrics { "Number of ongoing merge operations", "quickwit_indexing", ), + pending_merge_operations: new_gauge( + "pending_merge_operations", + "Number of pending merge operations", + "quickwit_indexing", + ), + pending_merge_bytes: new_gauge( + "pending_merge_bytes", + "Number of pending merge bytes", + "quickwit_indexing", + ), } } } diff --git a/quickwit/quickwit-indexing/src/models/indexed_split.rs b/quickwit/quickwit-indexing/src/models/indexed_split.rs index 5e48f9ebd60..dfd985a30e4 100644 --- a/quickwit/quickwit-indexing/src/models/indexed_split.rs +++ b/quickwit/quickwit-indexing/src/models/indexed_split.rs @@ -26,11 +26,11 @@ use quickwit_metastore::checkpoint::IndexCheckpointDelta; use quickwit_proto::indexing::IndexingPipelineId; use quickwit_proto::types::{IndexUid, PublishToken}; use tantivy::directory::MmapDirectory; -use tantivy::{IndexBuilder, TrackedObject}; +use tantivy::IndexBuilder; use tracing::{instrument, Span}; use crate::controlled_directory::ControlledDirectory; -use crate::merge_policy::MergeOperation; +use crate::merge_policy::MergeTask; use crate::models::{PublishLock, SplitAttrs}; use crate::new_split_id; @@ -157,11 +157,11 @@ pub struct IndexedSplitBatch { pub checkpoint_delta_opt: Option, pub publish_lock: PublishLock, pub publish_token_opt: Option, - /// A [`MergeOperation`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` + /// A [`MergeTask`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` /// in the `MergePipeline` or `DeleteTaskPipeline`. /// See planners docs to understand the usage. /// If `None`, the split batch was built in the `IndexingPipeline`. - pub merge_operation_opt: Option>, + pub merge_task_opt: Option, pub batch_parent_span: Span, } diff --git a/quickwit/quickwit-indexing/src/models/merge_scratch.rs b/quickwit/quickwit-indexing/src/models/merge_scratch.rs index 146d98ed1c4..8a718d440b1 100644 --- a/quickwit/quickwit-indexing/src/models/merge_scratch.rs +++ b/quickwit/quickwit-indexing/src/models/merge_scratch.rs @@ -18,15 +18,15 @@ // along with this program. If not, see . use quickwit_common::temp_dir::TempDirectory; -use tantivy::{Directory, TrackedObject}; +use tantivy::Directory; -use crate::merge_policy::MergeOperation; +use crate::merge_policy::MergeTask; #[derive(Debug)] pub struct MergeScratch { - /// A [`MergeOperation`] tracked by either the `MergePlannner` or the `DeleteTaksPlanner` + /// A [`MergeTask`] tracked by either the `MergePlannner` or the `DeleteTaksPlanner` /// See planners docs to understand the usage. - pub merge_operation: TrackedObject, + pub merge_task: MergeTask, /// Scratch directory for computing the merge. pub merge_scratch_directory: TempDirectory, pub downloaded_splits_directory: TempDirectory, diff --git a/quickwit/quickwit-indexing/src/models/packaged_split.rs b/quickwit/quickwit-indexing/src/models/packaged_split.rs index 0214ec14ee2..078c81a75cc 100644 --- a/quickwit/quickwit-indexing/src/models/packaged_split.rs +++ b/quickwit/quickwit-indexing/src/models/packaged_split.rs @@ -24,10 +24,9 @@ use itertools::Itertools; use quickwit_common::temp_dir::TempDirectory; use quickwit_metastore::checkpoint::IndexCheckpointDelta; use quickwit_proto::types::{IndexUid, PublishToken, SplitId}; -use tantivy::TrackedObject; use tracing::Span; -use crate::merge_policy::MergeOperation; +use crate::merge_policy::MergeTask; use crate::models::{PublishLock, SplitAttrs}; pub struct PackagedSplit { @@ -66,11 +65,11 @@ pub struct PackagedSplitBatch { pub checkpoint_delta_opt: Option, pub publish_lock: PublishLock, pub publish_token_opt: Option, - /// A [`MergeOperation`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` + /// A [`MergeTask`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` /// in the `MergePipeline` or `DeleteTaskPipeline`. /// See planners docs to understand the usage. /// If `None`, the split batch was built in the `IndexingPipeline`. - pub merge_operation_opt: Option>, + pub merge_task_opt: Option, pub batch_parent_span: Span, } @@ -84,7 +83,7 @@ impl PackagedSplitBatch { checkpoint_delta_opt: Option, publish_lock: PublishLock, publish_token_opt: Option, - merge_operation_opt: Option>, + merge_task_opt: Option, batch_parent_span: Span, ) -> Self { assert!(!splits.is_empty()); @@ -100,7 +99,7 @@ impl PackagedSplitBatch { checkpoint_delta_opt, publish_lock, publish_token_opt, - merge_operation_opt, + merge_task_opt, batch_parent_span, } } diff --git a/quickwit/quickwit-indexing/src/models/publisher_message.rs b/quickwit/quickwit-indexing/src/models/publisher_message.rs index c79a3792339..5e3ce1dedae 100644 --- a/quickwit/quickwit-indexing/src/models/publisher_message.rs +++ b/quickwit/quickwit-indexing/src/models/publisher_message.rs @@ -23,10 +23,9 @@ use itertools::Itertools; use quickwit_metastore::checkpoint::IndexCheckpointDelta; use quickwit_metastore::SplitMetadata; use quickwit_proto::types::{IndexUid, PublishToken}; -use tantivy::TrackedObject; use tracing::Span; -use crate::merge_policy::MergeOperation; +use crate::merge_policy::MergeTask; use crate::models::PublishLock; pub struct SplitsUpdate { @@ -36,11 +35,11 @@ pub struct SplitsUpdate { pub checkpoint_delta_opt: Option, pub publish_lock: PublishLock, pub publish_token_opt: Option, - /// A [`MergeOperation`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` + /// A [`MergeTask`] tracked by either the `MergePlanner` or the `DeleteTaskPlanner` /// in the `MergePipeline` or `DeleteTaskPipeline`. /// See planners docs to understand the usage. /// If `None`, the split batch was built in the `IndexingPipeline`. - pub merge_operation: Option>, + pub merge_task: Option, pub parent_span: Span, } diff --git a/quickwit/quickwit-indexing/src/source/ingest/mod.rs b/quickwit/quickwit-indexing/src/source/ingest/mod.rs index d1e17983ea6..e740be7bf12 100644 --- a/quickwit/quickwit-indexing/src/source/ingest/mod.rs +++ b/quickwit/quickwit-indexing/src/source/ingest/mod.rs @@ -304,7 +304,6 @@ impl IngestSource { self.client_id.source_uid.clone(), truncate_up_to_positions.clone(), ); - // Let's record all shards that have reached Eof as complete. for (shard, truncate_up_to_position_inclusive) in &truncate_up_to_positions { if truncate_up_to_position_inclusive.is_eof() { @@ -313,12 +312,11 @@ impl IngestSource { } } } - // We publish the event to the event broker. self.event_broker.publish(shard_positions_update); // Finally, we push the information to ingesters in a best effort manner. - // If the request fail, we just log an error. + // If the request fails, we just log an error. let mut per_ingester_truncate_subrequests: FnvHashMap< &NodeId, Vec, diff --git a/quickwit/quickwit-indexing/src/source/kafka_source.rs b/quickwit/quickwit-indexing/src/source/kafka_source.rs index 264ac4a7681..d1bec163a1d 100644 --- a/quickwit/quickwit-indexing/src/source/kafka_source.rs +++ b/quickwit/quickwit-indexing/src/source/kafka_source.rs @@ -939,7 +939,8 @@ mod kafka_broker_tests { ) -> IndexUid { let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -953,7 +954,8 @@ mod kafka_broker_tests { let split_id = new_split_id(); let split_metadata = SplitMetadata::for_test(split_id.clone()); let stage_splits_request = - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata).unwrap(); + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata) + .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let mut source_delta = SourceCheckpointDelta::default(); diff --git a/quickwit/quickwit-indexing/src/source/pulsar_source.rs b/quickwit/quickwit-indexing/src/source/pulsar_source.rs index 4c499a5d4fa..b0d7ad1a44b 100644 --- a/quickwit/quickwit-indexing/src/source/pulsar_source.rs +++ b/quickwit/quickwit-indexing/src/source/pulsar_source.rs @@ -490,7 +490,8 @@ mod pulsar_broker_tests { ) -> IndexUid { let index_uri = format!("ram:///indexes/{index_id}"); let index_config = IndexConfig::for_test(index_id, &index_uri); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = + CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -504,7 +505,8 @@ mod pulsar_broker_tests { let split_id = new_split_id(); let split_metadata = SplitMetadata::for_test(split_id.clone()); let stage_splits_request = - StageSplitsRequest::try_from_split_metadata(index_uid.clone(), split_metadata).unwrap(); + StageSplitsRequest::try_from_split_metadata(index_uid.clone(), &split_metadata) + .unwrap(); metastore.stage_splits(stage_splits_request).await.unwrap(); let mut source_delta = SourceCheckpointDelta::default(); diff --git a/quickwit/quickwit-indexing/src/test_utils.rs b/quickwit/quickwit-indexing/src/test_utils.rs index aad549fbe29..bc2383a3cc3 100644 --- a/quickwit/quickwit-indexing/src/test_utils.rs +++ b/quickwit/quickwit-indexing/src/test_utils.rs @@ -101,7 +101,7 @@ impl TestSandbox { let mut metastore = metastore_resolver .resolve(&Uri::for_test(METASTORE_URI)) .await?; - let create_index_request = CreateIndexRequest::try_from_index_config(index_config.clone())?; + let create_index_request = CreateIndexRequest::try_from_index_config(&index_config)?; let index_uid: IndexUid = metastore .create_index(create_index_request) .await? @@ -109,6 +109,7 @@ impl TestSandbox { .into(); let storage = storage_resolver.resolve(&index_uri).await?; let universe = Universe::with_accelerated_time(); + let merge_scheduler_mailbox = universe.get_or_spawn_one(); let queues_dir_path = temp_dir.path().join(QUEUES_DIR_NAME); let ingest_api_service = init_ingest_api(&universe, &queues_dir_path, &IngestApiConfig::default()).await?; @@ -120,6 +121,7 @@ impl TestSandbox { cluster, metastore.clone(), Some(ingest_api_service), + merge_scheduler_mailbox, IngesterPool::default(), storage_resolver.clone(), EventBroker::default(), diff --git a/quickwit/quickwit-ingest/src/ingest_v2/fetch.rs b/quickwit/quickwit-ingest/src/ingest_v2/fetch.rs index 7e36cf8423c..64fbd28d08c 100644 --- a/quickwit/quickwit-ingest/src/ingest_v2/fetch.rs +++ b/quickwit/quickwit-ingest/src/ingest_v2/fetch.rs @@ -308,7 +308,7 @@ impl MultiFetchStream { self.retry_params, self.fetch_message_tx.clone(), ); - let fetch_task_handle = spawn_named_task(fetch_stream_future, "fetch_stream_future"); + let fetch_task_handle = spawn_named_task(fetch_stream_future, "fetch_stream"); self.fetch_task_handles.insert(queue_id, fetch_task_handle); Ok(()) } diff --git a/quickwit/quickwit-jaeger/src/integration_tests.rs b/quickwit/quickwit-jaeger/src/integration_tests.rs index 0136a4d16cf..fe0860106a1 100644 --- a/quickwit/quickwit-jaeger/src/integration_tests.rs +++ b/quickwit/quickwit-jaeger/src/integration_tests.rs @@ -27,6 +27,7 @@ use quickwit_cluster::{create_cluster_for_test, ChannelTransport, Cluster}; use quickwit_common::pubsub::EventBroker; use quickwit_common::uri::Uri; use quickwit_config::{IndexerConfig, IngestApiConfig, JaegerConfig, SearcherConfig, SourceConfig}; +use quickwit_indexing::actors::MergeSchedulerService; use quickwit_indexing::models::SpawnPipeline; use quickwit_indexing::IndexingService; use quickwit_ingest::{ @@ -346,6 +347,7 @@ async fn indexer_for_test( cluster, metastore, Some(ingester_service), + universe.get_or_spawn_one::(), ingester_pool, storage_resolver, EventBroker::default(), @@ -395,7 +397,7 @@ async fn setup_traces_index( .unwrap(); let index_config = OtlpGrpcTracesService::index_config(&index_root_uri).unwrap(); let index_id = index_config.index_id.clone(); - let create_index_request = CreateIndexRequest::try_from_index_config(index_config).unwrap(); + let create_index_request = CreateIndexRequest::try_from_index_config(&index_config).unwrap(); let index_uid: IndexUid = metastore .create_index(create_index_request) .await @@ -404,7 +406,7 @@ async fn setup_traces_index( .into(); let source_config = SourceConfig::ingest_api_default(); let add_source_request = - AddSourceRequest::try_from_source_config(index_uid.clone(), source_config.clone()).unwrap(); + AddSourceRequest::try_from_source_config(index_uid.clone(), &source_config).unwrap(); metastore.add_source(add_source_request).await.unwrap(); let create_queue_request = CreateQueueRequest { diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs b/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs index 278a8efd22b..75a0f737ef2 100644 --- a/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs +++ b/quickwit/quickwit-janitor/src/actors/delete_task_pipeline.rs @@ -23,7 +23,8 @@ use std::time::Duration; use async_trait::async_trait; use quickwit_actors::{ - Actor, ActorContext, ActorExitStatus, ActorHandle, Handler, Supervisor, SupervisorState, + Actor, ActorContext, ActorExitStatus, ActorHandle, Handler, Mailbox, Supervisor, + SupervisorState, }; use quickwit_common::io::IoControls; use quickwit_common::pubsub::EventBroker; @@ -31,8 +32,8 @@ use quickwit_common::temp_dir::{self}; use quickwit_common::uri::Uri; use quickwit_config::build_doc_mapper; use quickwit_indexing::actors::{ - MergeExecutor, MergeSplitDownloader, Packager, Publisher, PublisherCounters, Uploader, - UploaderCounters, UploaderType, + MergeExecutor, MergeSchedulerService, MergeSplitDownloader, Packager, Publisher, + PublisherCounters, Uploader, UploaderCounters, UploaderType, }; use quickwit_indexing::merge_policy::merge_policy_from_settings; use quickwit_indexing::{IndexingSplitStore, PublisherType, SplitsUpdateMailbox}; @@ -86,6 +87,7 @@ pub struct DeleteTaskPipeline { handles: Option, max_concurrent_split_uploads: usize, state: DeleteTaskPipelineState, + merge_scheduler_service: Mailbox, event_broker: EventBroker, } @@ -127,6 +129,7 @@ impl Actor for DeleteTaskPipeline { } impl DeleteTaskPipeline { + #[allow(clippy::too_many_arguments)] pub fn new( index_uid: IndexUid, metastore: MetastoreServiceClient, @@ -134,6 +137,7 @@ impl DeleteTaskPipeline { index_storage: Arc, delete_service_task_dir: PathBuf, max_concurrent_split_uploads: usize, + merge_scheduler_service: Mailbox, event_broker: EventBroker, ) -> Self { Self { @@ -145,6 +149,7 @@ impl DeleteTaskPipeline { handles: Default::default(), max_concurrent_split_uploads, state: DeleteTaskPipelineState::default(), + merge_scheduler_service, event_broker, } } @@ -230,6 +235,7 @@ impl DeleteTaskPipeline { self.metastore.clone(), self.search_job_placer.clone(), downloader_mailbox, + self.merge_scheduler_service.clone(), ); let (_, task_planner_supervisor_handler) = ctx.spawn_actor().supervise(task_planner); self.handles = Some(DeletePipelineHandle { @@ -279,9 +285,10 @@ impl Handler for DeleteTaskPipeline { #[cfg(test)] mod tests { use async_trait::async_trait; - use quickwit_actors::Handler; + use quickwit_actors::{Handler, Universe}; use quickwit_common::pubsub::EventBroker; use quickwit_common::temp_dir::TempDirectory; + use quickwit_indexing::actors::MergeSchedulerService; use quickwit_indexing::TestSandbox; use quickwit_metastore::{ListSplitsRequestExt, MetastoreServiceStreamSplitsExt, SplitState}; use quickwit_proto::metastore::{DeleteQuery, ListSplitsRequest, MetastoreService}; @@ -336,6 +343,8 @@ mod tests { ) .await .unwrap(); + let universe: &Universe = test_sandbox.universe(); + let merge_scheduler_service = universe.get_or_spawn_one::(); let index_uid = test_sandbox.index_uid(); let docs = vec![ serde_json::json!({"body": "info", "ts": 0 }), @@ -386,19 +395,16 @@ mod tests { test_sandbox.storage(), delete_service_task_dir.path().into(), 4, + merge_scheduler_service, EventBroker::default(), ); - let (pipeline_mailbox, pipeline_handler) = - test_sandbox.universe().spawn_builder().spawn(pipeline); + let (pipeline_mailbox, pipeline_handler) = universe.spawn_builder().spawn(pipeline); // Ensure that the message sent by initialize method is processed. let _ = pipeline_handler.process_pending_and_observe().await.state; // Pipeline will first fail and we need to wait a OBSERVE_PIPELINE_INTERVAL * some number // for the pipeline state to be updated. - test_sandbox - .universe() - .sleep(OBSERVE_PIPELINE_INTERVAL * 5) - .await; + universe.sleep(OBSERVE_PIPELINE_INTERVAL * 5).await; let pipeline_state = pipeline_handler.process_pending_and_observe().await.state; assert_eq!(pipeline_state.delete_task_planner.metrics.num_errors, 1); assert_eq!(pipeline_state.downloader.metrics.num_errors, 0); @@ -440,6 +446,8 @@ mod tests { let test_sandbox = TestSandbox::create(index_id, doc_mapping_yaml, "{}", &["body"]) .await .unwrap(); + let universe: &Universe = test_sandbox.universe(); + let merge_scheduler_mailbox = universe.get_or_spawn_one::(); let metastore = test_sandbox.metastore(); let mut mock_search_service = MockSearchService::new(); mock_search_service @@ -468,16 +476,13 @@ mod tests { test_sandbox.storage(), delete_service_task_dir.path().into(), 4, + merge_scheduler_mailbox, EventBroker::default(), ); - let (_pipeline_mailbox, pipeline_handler) = - test_sandbox.universe().spawn_builder().spawn(pipeline); + let (_pipeline_mailbox, pipeline_handler) = universe.spawn_builder().spawn(pipeline); pipeline_handler.quit().await; - let observations = test_sandbox - .universe() - .observe(OBSERVE_PIPELINE_INTERVAL) - .await; + let observations = universe.observe(OBSERVE_PIPELINE_INTERVAL).await; assert!(observations.into_iter().all( |observation| observation.type_name != std::any::type_name::() )); diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs b/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs index 82669ccdf95..d592f9c2a22 100644 --- a/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs +++ b/quickwit/quickwit-janitor/src/actors/delete_task_planner.rs @@ -28,7 +28,7 @@ use quickwit_actors::{Actor, ActorContext, ActorExitStatus, Handler, Mailbox, Qu use quickwit_common::extract_time_range; use quickwit_common::uri::Uri; use quickwit_doc_mapper::tag_pruning::extract_tags_from_query; -use quickwit_indexing::actors::MergeSplitDownloader; +use quickwit_indexing::actors::{schedule_merge, MergeSchedulerService, MergeSplitDownloader}; use quickwit_indexing::merge_policy::MergeOperation; use quickwit_metastore::{split_tag_filter, split_time_range_filter, ListSplitsResponseExt, Split}; use quickwit_proto::metastore::{ @@ -83,6 +83,7 @@ pub struct DeleteTaskPlanner { metastore: MetastoreServiceClient, search_job_placer: SearchJobPlacer, merge_split_downloader_mailbox: Mailbox, + merge_scheduler_service: Mailbox, /// Inventory of ongoing delete operations. If everything goes well, /// a merge operation is dropped after the publish of the split that underwent /// the delete operation. @@ -127,6 +128,7 @@ impl DeleteTaskPlanner { metastore: MetastoreServiceClient, search_job_placer: SearchJobPlacer, merge_split_downloader_mailbox: Mailbox, + merge_scheduler_service: Mailbox, ) -> Self { Self { index_uid, @@ -135,6 +137,7 @@ impl DeleteTaskPlanner { metastore, search_job_placer, merge_split_downloader_mailbox, + merge_scheduler_service, ongoing_delete_operations_inventory: Inventory::new(), } } @@ -200,9 +203,10 @@ impl DeleteTaskPlanner { let tracked_delete_operation = self .ongoing_delete_operations_inventory .track(delete_operation); - ctx.send_message( - &self.merge_split_downloader_mailbox, + schedule_merge( + &self.merge_scheduler_service, tracked_delete_operation, + self.merge_split_downloader_mailbox.clone(), ) .await?; JANITOR_METRICS @@ -422,7 +426,7 @@ impl Handler for DeleteTaskPlanner { #[cfg(test)] mod tests { use quickwit_config::build_doc_mapper; - use quickwit_indexing::merge_policy::MergeOperation; + use quickwit_indexing::merge_policy::MergeTask; use quickwit_indexing::TestSandbox; use quickwit_metastore::{ IndexMetadataResponseExt, ListSplitsRequestExt, MetastoreServiceStreamSplitsExt, @@ -431,7 +435,6 @@ mod tests { use quickwit_proto::metastore::{DeleteQuery, IndexMetadataRequest, ListSplitsRequest}; use quickwit_proto::search::{LeafSearchRequest, LeafSearchResponse}; use quickwit_search::{searcher_pool_for_test, MockSearchService}; - use tantivy::TrackedObject; use super::*; @@ -458,6 +461,7 @@ mod tests { &["body"], ) .await?; + let universe = test_sandbox.universe(); let docs = [ serde_json::json!({"body": "info", "ts": 0 }), serde_json::json!({"body": "info", "ts": 0 }), @@ -537,12 +541,14 @@ mod tests { let searcher_pool = searcher_pool_for_test([("127.0.0.1:1000", mock_search_service)]); let search_job_placer = SearchJobPlacer::new(searcher_pool); let (downloader_mailbox, downloader_inbox) = test_sandbox.universe().create_test_mailbox(); + let (merge_split_downloader_mailbox, _) = universe.create_test_mailbox(); let delete_planner_executor = DeleteTaskPlanner::new( index_uid.clone(), index_config.index_uri.clone(), doc_mapper_str, metastore.clone(), search_job_placer, + merge_split_downloader_mailbox, downloader_mailbox, ); let (delete_planner_mailbox, delete_planner_handle) = test_sandbox @@ -550,8 +556,7 @@ mod tests { .spawn_builder() .spawn(delete_planner_executor); delete_planner_handle.process_pending_and_observe().await; - let downloader_msgs: Vec> = - downloader_inbox.drain_for_test_typed(); + let downloader_msgs: Vec = downloader_inbox.drain_for_test_typed(); assert_eq!(downloader_msgs.len(), 1); // The last split will undergo a delete operation. assert_eq!( @@ -585,8 +590,7 @@ mod tests { .ask(PlanDeleteOperations) .await .unwrap(); - let downloader_last_msgs = - downloader_inbox.drain_for_test_typed::>(); + let downloader_last_msgs = downloader_inbox.drain_for_test_typed::(); assert_eq!(downloader_last_msgs.len(), 1); assert_eq!( downloader_last_msgs[0].splits[0].split_id(), diff --git a/quickwit/quickwit-janitor/src/actors/delete_task_service.rs b/quickwit/quickwit-janitor/src/actors/delete_task_service.rs index 514e1fb69a1..ff943ad91c2 100644 --- a/quickwit/quickwit-janitor/src/actors/delete_task_service.rs +++ b/quickwit/quickwit-janitor/src/actors/delete_task_service.rs @@ -22,10 +22,11 @@ use std::path::PathBuf; use std::time::Duration; use async_trait::async_trait; -use quickwit_actors::{Actor, ActorContext, ActorExitStatus, ActorHandle, Handler}; +use quickwit_actors::{Actor, ActorContext, ActorExitStatus, ActorHandle, Handler, Mailbox}; use quickwit_common::pubsub::EventBroker; use quickwit_common::temp_dir::{self}; use quickwit_config::IndexConfig; +use quickwit_indexing::actors::MergeSchedulerService; use quickwit_metastore::{IndexMetadataResponseExt, ListIndexesMetadataResponseExt}; use quickwit_proto::metastore::{ IndexMetadataRequest, ListIndexesMetadataRequest, MetastoreService, MetastoreServiceClient, @@ -61,6 +62,7 @@ pub struct DeleteTaskService { pipeline_handles_by_index_uid: HashMap>, max_concurrent_split_uploads: usize, event_broker: EventBroker, + merge_scheduler_service: Mailbox, } impl DeleteTaskService { @@ -70,6 +72,7 @@ impl DeleteTaskService { storage_resolver: StorageResolver, data_dir_path: PathBuf, max_concurrent_split_uploads: usize, + merge_scheduler_service: Mailbox, event_broker: EventBroker, ) -> anyhow::Result { let delete_service_task_path = data_dir_path.join(DELETE_SERVICE_TASK_DIR_NAME); @@ -82,6 +85,7 @@ impl DeleteTaskService { delete_service_task_dir, pipeline_handles_by_index_uid: Default::default(), max_concurrent_split_uploads, + merge_scheduler_service, event_broker, }) } @@ -180,6 +184,7 @@ impl DeleteTaskService { index_storage, self.delete_service_task_dir.clone(), self.max_concurrent_split_uploads, + self.merge_scheduler_service.clone(), self.event_broker.clone(), ); let (_pipeline_mailbox, pipeline_handler) = ctx.spawn_actor().spawn(pipeline); @@ -212,6 +217,7 @@ impl Handler for DeleteTaskService { #[cfg(test)] mod tests { + use quickwit_actors::Universe; use quickwit_common::pubsub::EventBroker; use quickwit_indexing::TestSandbox; use quickwit_proto::metastore::{ @@ -242,20 +248,20 @@ mod tests { let search_job_placer = SearchJobPlacer::new(searcher_pool); let temp_dir = tempfile::tempdir().unwrap(); let data_dir_path = temp_dir.path().to_path_buf(); + let universe: &Universe = test_sandbox.universe(); let delete_task_service = DeleteTaskService::new( metastore.clone(), search_job_placer, StorageResolver::unconfigured(), data_dir_path, 4, + universe.get_or_spawn_one(), EventBroker::default(), ) .await .unwrap(); - let (_delete_task_service_mailbox, delete_task_service_handler) = test_sandbox - .universe() - .spawn_builder() - .spawn(delete_task_service); + let (_delete_task_service_mailbox, delete_task_service_handler) = + universe.spawn_builder().spawn(delete_task_service); let state = delete_task_service_handler .process_pending_and_observe() .await; @@ -283,32 +289,20 @@ mod tests { }) .await .unwrap(); - test_sandbox - .universe() - .sleep(UPDATE_PIPELINES_INTERVAL * 2) - .await; + universe.sleep(UPDATE_PIPELINES_INTERVAL * 2).await; let state_after_deletion = delete_task_service_handler .process_pending_and_observe() .await; assert_eq!(state_after_deletion.num_running_pipelines, 0); - assert!(test_sandbox - .universe() - .get_one::() - .is_some()); - let actors_observations = test_sandbox - .universe() - .observe(UPDATE_PIPELINES_INTERVAL) - .await; + assert!(universe.get_one::().is_some()); + let actors_observations = universe.observe(UPDATE_PIPELINES_INTERVAL).await; assert!( actors_observations .into_iter() .any(|observation| observation.type_name == std::any::type_name::()) ); - assert!(test_sandbox - .universe() - .get_one::() - .is_some()); + assert!(universe.get_one::().is_some()); test_sandbox.assert_quit().await; Ok(()) } diff --git a/quickwit/quickwit-janitor/src/actors/retention_policy_executor.rs b/quickwit/quickwit-janitor/src/actors/retention_policy_executor.rs index a527d75df4f..70d3d78eec7 100644 --- a/quickwit/quickwit-janitor/src/actors/retention_policy_executor.rs +++ b/quickwit/quickwit-janitor/src/actors/retention_policy_executor.rs @@ -114,7 +114,7 @@ impl RetentionPolicyExecutor { let index_uid = index_metadata.index_uid.clone(); let index_config = index_metadata.into_index_config(); // We only care about indexes with a retention policy configured. - let retention_policy = match &index_config.retention_policy { + let retention_policy = match &index_config.retention_policy_opt { Some(policy) => policy, None => { // Remove the index from the cache if it exist. @@ -203,7 +203,7 @@ impl Handler for RetentionPolicyExecutor { }; let retention_policy = index_config - .retention_policy + .retention_policy_opt .as_ref() .expect("Expected index to have retention policy configure."); @@ -281,7 +281,7 @@ mod tests { let indexes_set: HashSet<_> = self .index_configs .values() - .map(|im| (&im.index_id, &im.retention_policy)) + .map(|im| (&im.index_id, &im.retention_policy_opt)) .collect(); let expected_indexes: Vec = make_indexes(&message.0) @@ -290,7 +290,7 @@ mod tests { .collect(); let expected_indexes_set: HashSet<_> = expected_indexes .iter() - .map(|im| (&im.index_id, &im.retention_policy)) + .map(|im| (&im.index_id, &im.retention_policy_opt)) .collect(); assert_eq!( indexes_set, expected_indexes_set, @@ -300,15 +300,15 @@ mod tests { } } - const SCHEDULE_EXPR: &str = "hourly"; + const EVALUATION_SCHEDULE: &str = "hourly"; fn make_index(index_id: &str, retention_period_opt: Option<&str>) -> IndexConfig { let mut index = IndexConfig::for_test(index_id, &format!("ram://indexes/{index_id}")); if let Some(retention_period) = retention_period_opt { - index.retention_policy = Some(RetentionPolicy::new( - retention_period.to_string(), - SCHEDULE_EXPR.to_string(), - )) + index.retention_policy_opt = Some(RetentionPolicy { + retention_period: retention_period.to_string(), + evaluation_schedule: EVALUATION_SCHEDULE.to_string(), + }) } index } @@ -338,7 +338,11 @@ mod tests { // Uses the retention policy scheduler to calculate // how much time to advance for the execution to take place. fn shift_time_by() -> Duration { - let scheduler = RetentionPolicy::new("".to_string(), SCHEDULE_EXPR.to_string()); + let scheduler = RetentionPolicy { + retention_period: "".to_string(), + evaluation_schedule: EVALUATION_SCHEDULE.to_string(), + }; + scheduler.duration_until_next_evaluation().unwrap() + Duration::from_secs(1) } diff --git a/quickwit/quickwit-janitor/src/lib.rs b/quickwit/quickwit-janitor/src/lib.rs index 9aa52b70789..70783d860db 100644 --- a/quickwit/quickwit-janitor/src/lib.rs +++ b/quickwit/quickwit-janitor/src/lib.rs @@ -22,6 +22,7 @@ use quickwit_actors::{Mailbox, Universe}; use quickwit_common::pubsub::EventBroker; use quickwit_config::NodeConfig; +use quickwit_indexing::actors::MergeSchedulerService; use quickwit_metastore::SplitInfo; use quickwit_proto::metastore::MetastoreServiceClient; use quickwit_search::SearchJobPlacer; @@ -64,6 +65,7 @@ pub async fn start_janitor_service( storage_resolver, config.data_dir_path.clone(), config.indexer_config.max_concurrent_split_uploads, + universe.get_or_spawn_one::(), event_broker, ) .await?; diff --git a/quickwit/quickwit-janitor/src/retention_policy_execution.rs b/quickwit/quickwit-janitor/src/retention_policy_execution.rs index f4abd055941..7f97fc72642 100644 --- a/quickwit/quickwit-janitor/src/retention_policy_execution.rs +++ b/quickwit/quickwit-janitor/src/retention_policy_execution.rs @@ -55,7 +55,7 @@ pub async fn run_execute_retention_policy( .with_split_state(SplitState::Published) .with_time_range_end_lte(max_retention_timestamp); - let list_splits_request = ListSplitsRequest::try_from_list_splits_query(query)?; + let list_splits_request = ListSplitsRequest::try_from_list_splits_query(&query)?; let (expired_splits, ignored_splits): (Vec, Vec) = ctx .protect_future(metastore.list_splits(list_splits_request)) .await? diff --git a/quickwit/quickwit-lambda/src/indexer/ingest.rs b/quickwit/quickwit-lambda/src/indexer/ingest.rs index e1a37bec90f..d6987cfdc7b 100644 --- a/quickwit/quickwit-lambda/src/indexer/ingest.rs +++ b/quickwit/quickwit-lambda/src/indexer/ingest.rs @@ -35,10 +35,10 @@ use quickwit_config::merge_policy_config::MergePolicyConfig; use quickwit_config::service::QuickwitService; use quickwit_config::{ load_index_config_from_user_config, ConfigFormat, IndexConfig, IndexerConfig, NodeConfig, - SourceConfig, SourceInputFormat, SourceParams, TransformConfig, CLI_INGEST_SOURCE_ID, + SourceConfig, SourceInputFormat, SourceParams, TransformConfig, CLI_SOURCE_ID, }; use quickwit_index_management::{clear_cache_directory, IndexService}; -use quickwit_indexing::actors::{IndexingService, MergePipelineId}; +use quickwit_indexing::actors::{IndexingService, MergePipelineId, MergeSchedulerService}; use quickwit_indexing::models::{ DetachIndexingPipeline, DetachMergePipeline, IndexingStatistics, SpawnPipeline, }; @@ -130,7 +130,7 @@ pub async fn ingest(args: IngestArgs) -> anyhow::Result { .vrl_script .map(|vrl_script| TransformConfig::new(vrl_script, None)); let source_config = SourceConfig { - source_id: CLI_INGEST_SOURCE_ID.to_string(), + source_id: CLI_SOURCE_ID.to_string(), max_num_pipelines_per_indexer: NonZeroUsize::new(1).expect("1 is always non-zero."), desired_num_pipelines: NonZeroUsize::new(1).expect("1 is always non-zero."), enabled: true, @@ -168,7 +168,7 @@ pub async fn ingest(args: IngestArgs) -> anyhow::Result { ); } metastore - .create_index(CreateIndexRequest::try_from_index_config(index_config)?) + .create_index(CreateIndexRequest::try_from_index_config(&index_config)?) .await?; debug!("index created"); } else if args.overwrite { @@ -197,6 +197,11 @@ pub async fn ingest(args: IngestArgs) -> anyhow::Result { runtimes_config, &HashSet::from_iter([QuickwitService::Indexer]), )?; + let merge_scheduler_service = + MergeSchedulerService::new(indexer_config.merge_concurrency.get()); + let universe = Universe::new(); + let (merge_scheduler_service_mailbox, _) = + universe.spawn_builder().spawn(merge_scheduler_service); let indexing_server = IndexingService::new( config.node_id.clone(), config.data_dir_path.clone(), @@ -205,12 +210,12 @@ pub async fn ingest(args: IngestArgs) -> anyhow::Result { cluster, metastore, None, + merge_scheduler_service_mailbox, IngesterPool::default(), storage_resolver, EventBroker::default(), ) .await?; - let universe = Universe::new(); let (indexing_server_mailbox, indexing_server_handle) = universe.spawn_builder().spawn(indexing_server); let pipeline_id = indexing_server_mailbox diff --git a/quickwit/quickwit-metastore/Cargo.toml b/quickwit/quickwit-metastore/Cargo.toml index 03edc5a7462..45503583700 100644 --- a/quickwit/quickwit-metastore/Cargo.toml +++ b/quickwit/quickwit-metastore/Cargo.toml @@ -21,6 +21,7 @@ once_cell = { workspace = true } ouroboros = { workspace = true } rand = { workspace = true } regex = { workspace = true } +regex-syntax = { workspace = true } sea-query = { workspace = true, optional = true } sea-query-binder = { workspace = true, optional = true } serde = { workspace = true } @@ -50,6 +51,7 @@ futures = { workspace = true } md5 = { workspace = true } mockall = { workspace = true } rand = { workspace = true } +serial_test = "3.0.0" tempfile = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.down.sql b/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.down.sql new file mode 100644 index 00000000000..8fb6346b066 --- /dev/null +++ b/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.down.sql @@ -0,0 +1 @@ +DROP TABLE index_templates; diff --git a/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.up.sql b/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.up.sql new file mode 100644 index 00000000000..79210522407 --- /dev/null +++ b/quickwit/quickwit-metastore/migrations/postgresql/15_create-templates.up.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS index_templates ( + template_id VARCHAR(255) NOT NULL, + positive_index_id_patterns VARCHAR(255)[] NOT NULL, + negative_index_id_patterns VARCHAR(255)[] NOT NULL, + priority INTEGER NOT NULL DEFAULT 0, + index_template_json TEXT NOT NULL, + PRIMARY KEY (template_id) +); diff --git a/quickwit/quickwit-metastore/src/backward_compatibility_tests/mod.rs b/quickwit/quickwit-metastore/src/backward_compatibility_tests/mod.rs index 9618e24d3df..9f0d52a5a0b 100644 --- a/quickwit/quickwit-metastore/src/backward_compatibility_tests/mod.rs +++ b/quickwit/quickwit-metastore/src/backward_compatibility_tests/mod.rs @@ -26,6 +26,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value as JsonValue; use crate::file_backed::file_backed_index::FileBackedIndex; +use crate::file_backed::manifest::Manifest; use crate::{IndexMetadata, SplitMetadata}; /// In order to avoid confusion, we need to make sure that the @@ -76,7 +77,7 @@ where T: TestableForRegression + std::fmt::Debug { let expected: T = deserialize_json_file(Path::new(&expected_path))?; println!("---\nTest equality of {:?}", expected); println!("---\nwith {:?}", deserialized); - deserialized.test_equality(&expected); + deserialized.assert_equality(&expected); Ok(()) } @@ -219,3 +220,8 @@ fn test_index_metadata_backward_compatibility() { fn test_file_backed_index_backward_compatibility() { test_json_backward_compatibility_helper::("file-backed-index").unwrap(); } + +#[test] +fn test_file_backed_metastore_manifest_backward_compatibility() { + test_json_backward_compatibility_helper::("manifest").unwrap(); +} diff --git a/quickwit/quickwit-metastore/src/lib.rs b/quickwit/quickwit-metastore/src/lib.rs index 7d93e9c1175..45b20864402 100644 --- a/quickwit/quickwit-metastore/src/lib.rs +++ b/quickwit/quickwit-metastore/src/lib.rs @@ -48,7 +48,7 @@ pub(crate) use metastore::index_metadata::serialize::{IndexMetadataV0_7, Version #[cfg(feature = "postgres")] pub use metastore::postgres::PostgresqlMetastore; pub use metastore::{ - file_backed, AddSourceRequestExt, CreateIndexRequestExt, IndexMetadata, + file_backed, AddSourceRequestExt, CreateIndexRequestExt, CreateIndexResponseExt, IndexMetadata, IndexMetadataResponseExt, ListIndexesMetadataResponseExt, ListSplitsQuery, ListSplitsRequestExt, ListSplitsResponseExt, MetastoreServiceExt, MetastoreServiceStreamSplitsExt, PublishSplitsRequestExt, StageSplitsRequestExt, diff --git a/quickwit/quickwit-metastore/src/metastore/control_plane_metastore.rs b/quickwit/quickwit-metastore/src/metastore/control_plane_metastore.rs index 5615632a4ed..2949401c1db 100644 --- a/quickwit/quickwit-metastore/src/metastore/control_plane_metastore.rs +++ b/quickwit/quickwit-metastore/src/metastore/control_plane_metastore.rs @@ -24,10 +24,13 @@ use quickwit_common::uri::Uri; use quickwit_proto::control_plane::{ControlPlaneService, ControlPlaneServiceClient}; use quickwit_proto::metastore::{ AcquireShardsRequest, AcquireShardsResponse, AddSourceRequest, CreateIndexRequest, - CreateIndexResponse, DeleteIndexRequest, DeleteQuery, DeleteShardsRequest, - DeleteShardsResponse, DeleteSourceRequest, DeleteSplitsRequest, DeleteTask, EmptyResponse, - IndexMetadataRequest, IndexMetadataResponse, LastDeleteOpstampRequest, - LastDeleteOpstampResponse, ListDeleteTasksRequest, ListDeleteTasksResponse, + CreateIndexResponse, CreateIndexTemplateRequest, DeleteIndexRequest, + DeleteIndexTemplatesRequest, DeleteQuery, DeleteShardsRequest, DeleteShardsResponse, + DeleteSourceRequest, DeleteSplitsRequest, DeleteTask, EmptyResponse, + FindIndexTemplateMatchesRequest, FindIndexTemplateMatchesResponse, GetIndexTemplateRequest, + GetIndexTemplateResponse, IndexMetadataRequest, IndexMetadataResponse, + LastDeleteOpstampRequest, LastDeleteOpstampResponse, ListDeleteTasksRequest, + ListDeleteTasksResponse, ListIndexTemplatesRequest, ListIndexTemplatesResponse, ListIndexesMetadataRequest, ListIndexesMetadataResponse, ListShardsRequest, ListShardsResponse, ListSplitsRequest, ListSplitsResponse, ListStaleSplitsRequest, MarkSplitsForDeletionRequest, MetastoreResult, MetastoreService, MetastoreServiceClient, MetastoreServiceStream, @@ -236,4 +239,41 @@ impl MetastoreService for ControlPlaneMetastore { ) -> MetastoreResult { self.metastore.delete_shards(request).await } + + // Index Template API + + async fn create_index_template( + &mut self, + request: CreateIndexTemplateRequest, + ) -> MetastoreResult { + self.metastore.create_index_template(request).await + } + + async fn get_index_template( + &mut self, + request: GetIndexTemplateRequest, + ) -> MetastoreResult { + self.metastore.get_index_template(request).await + } + + async fn find_index_template_matches( + &mut self, + request: FindIndexTemplateMatchesRequest, + ) -> MetastoreResult { + self.metastore.find_index_template_matches(request).await + } + + async fn list_index_templates( + &mut self, + request: ListIndexTemplatesRequest, + ) -> MetastoreResult { + self.metastore.list_index_templates(request).await + } + + async fn delete_index_templates( + &mut self, + request: DeleteIndexTemplatesRequest, + ) -> MetastoreResult { + self.metastore.delete_index_templates(request).await + } } diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed/file_backed_index/mod.rs b/quickwit/quickwit-metastore/src/metastore/file_backed/file_backed_index/mod.rs index 8154280cbcf..e02ec3f7ed3 100644 --- a/quickwit/quickwit-metastore/src/metastore/file_backed/file_backed_index/mod.rs +++ b/quickwit/quickwit-metastore/src/metastore/file_backed/file_backed_index/mod.rs @@ -122,8 +122,8 @@ impl quickwit_config::TestableForRegression for FileBackedIndex { FileBackedIndex::new(index_metadata, splits, per_source_shards, delete_tasks) } - fn test_equality(&self, other: &Self) { - self.metadata().test_equality(other.metadata()); + fn assert_equality(&self, other: &Self) { + self.metadata().assert_equality(other.metadata()); assert_eq!(self.splits, other.splits); assert_eq!(self.per_source_shards, other.per_source_shards); assert_eq!(self.delete_tasks, other.delete_tasks); @@ -132,10 +132,19 @@ impl quickwit_config::TestableForRegression for FileBackedIndex { impl From for FileBackedIndex { fn from(index_metadata: IndexMetadata) -> Self { + let per_source_shards = index_metadata + .sources + .keys() + .map(|source_id| { + let shards = Shards::empty(index_metadata.index_uid.clone(), source_id.clone()); + (source_id.clone(), shards) + }) + .collect(); + Self { metadata: index_metadata, splits: Default::default(), - per_source_shards: Default::default(), + per_source_shards, delete_tasks: Default::default(), stamper: Default::default(), recently_modified: false, diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed/index_id_matcher.rs b/quickwit/quickwit-metastore/src/metastore/file_backed/index_id_matcher.rs new file mode 100644 index 00000000000..a29ccd0dfd5 --- /dev/null +++ b/quickwit/quickwit-metastore/src/metastore/file_backed/index_id_matcher.rs @@ -0,0 +1,162 @@ +// Copyright (C) 2024 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_config::validate_index_id_pattern; +use quickwit_proto::metastore::{MetastoreError, MetastoreResult}; +use regex::RegexSet; +use regex_syntax::escape_into; + +pub(super) type IndexIdPattern = String; + +#[derive(Debug)] +pub(super) struct IndexIdMatcher { + positive_matcher: RegexSet, + negative_matcher: RegexSet, +} + +impl IndexIdMatcher { + /// Builds an [`IndexIdMatcher`] from an set of index ID patterns using the following rules: + /// - If the given pattern does not contain a `*` char, it matches the exact pattern. + /// - If the given pattern contains one or more `*`, it matches the regex built from a regex + /// where `*` is replaced by `.*`. All other regular expression meta characters are escaped. + pub fn try_from_index_id_patterns( + index_id_patterns: &[IndexIdPattern], + ) -> MetastoreResult { + let mut positive_patterns: Vec<&str> = Vec::new(); + let mut negative_patterns: Vec<&str> = Vec::new(); + + for pattern in index_id_patterns { + if let Some(negative_pattern) = pattern.strip_prefix('-') { + negative_patterns.push(negative_pattern); + } else { + positive_patterns.push(pattern); + } + } + if positive_patterns.is_empty() { + let message = "failed to build index ID matcher: at least one positive index ID \ + pattern must be provided" + .to_string(); + return Err(MetastoreError::InvalidArgument { message }); + } + let positive_matcher = build_regex_set(&positive_patterns)?; + let negative_matcher = build_regex_set(&negative_patterns)?; + + let matcher = IndexIdMatcher { + positive_matcher, + negative_matcher, + }; + Ok(matcher) + } + + pub fn is_match(&self, index_id: &str) -> bool { + self.positive_matcher.is_match(index_id) && !self.negative_matcher.is_match(index_id) + } +} + +fn build_regex_set(patterns: &[&str]) -> MetastoreResult { + for pattern in patterns { + if *pattern == "*" { + let regex_set = RegexSet::new([".*"]).expect("regular expression set should compile"); + return Ok(regex_set); + } + validate_index_id_pattern(pattern, false).map_err(|error| { + let message = format!("failed to build index ID matcher: {error}"); + MetastoreError::InvalidArgument { message } + })?; + } + let regexes = patterns.iter().map(|pattern| build_regex(pattern)); + + let regex_set = RegexSet::new(regexes).map_err(|error| { + let message = format!("failed to build index ID matcher: {error}"); + MetastoreError::InvalidArgument { message } + })?; + Ok(regex_set) +} + +fn build_regex(pattern: &str) -> String { + let mut regex = String::new(); + regex.push('^'); + + for (idx, part) in pattern.split('*').enumerate() { + if idx > 0 { + regex.push_str(".*"); + } + escape_into(part, &mut regex); + } + regex.push('$'); + regex +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_build_regex() { + let regex = build_regex(""); + assert_eq!(regex, r"^$"); + + let regex = build_regex("*"); + assert_eq!(regex, r"^.*$"); + + let regex = build_regex("index-1"); + assert_eq!(regex, r"^index\-1$"); + + let regex = build_regex("*-index-*-1"); + assert_eq!(regex, r"^.*\-index\-.*\-1$"); + + let regex = build_regex("INDEX.2*-1"); + assert_eq!(regex, r"^INDEX\.2.*\-1$"); + } + + #[test] + fn test_build_regex_set() { + let error = build_regex_set(&["_index-1"]).unwrap_err(); + assert!(matches!(error, MetastoreError::InvalidArgument { .. })); + + let regex_set = build_regex_set(&["index-1"]).unwrap(); + assert!(regex_set.is_match("index-1")); + assert!(!regex_set.is_match("index-2")); + + let regex_set = build_regex_set(&["index-1", "index-2"]).unwrap(); + assert!(regex_set.is_match("index-1")); + assert!(regex_set.is_match("index-2")); + assert!(!regex_set.is_match("index-3")); + + let regex_set = build_regex_set(&["index-1*"]).unwrap(); + assert!(regex_set.is_match("index-1")); + assert!(regex_set.is_match("index-10")); + assert!(!regex_set.is_match("index-2")); + } + + #[test] + fn test_index_id_matcher() { + let error = IndexIdMatcher::try_from_index_id_patterns(&[]).unwrap_err(); + assert!(matches!(error, MetastoreError::InvalidArgument { .. })); + + let matcher = IndexIdMatcher::try_from_index_id_patterns(&[ + "index-foo*".to_string(), + "-index-foobar".to_string(), + ]) + .unwrap(); + assert!(matcher.is_match("index-foo")); + assert!(matcher.is_match("index-fooo")); + assert!(!matcher.is_match("index-foobar")); + } +} diff --git a/quickwit/quickwit-metastore/src/metastore/file_backed/index_template_matcher.rs b/quickwit/quickwit-metastore/src/metastore/file_backed/index_template_matcher.rs new file mode 100644 index 00000000000..cf3f60eee0b --- /dev/null +++ b/quickwit/quickwit-metastore/src/metastore/file_backed/index_template_matcher.rs @@ -0,0 +1,151 @@ +// Copyright (C) 2024 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_config::{IndexTemplate, IndexTemplateId}; +use quickwit_proto::metastore::MetastoreResult; + +use super::index_id_matcher::IndexIdMatcher; + +struct InnerMatcher { + template_id: IndexTemplateId, + priority: usize, + matcher: IndexIdMatcher, +} + +impl InnerMatcher { + /// Compares two matchers by (-,