diff --git a/docs/migrate.rst b/docs/migrate.rst index 648c30423..6c87d95bc 100644 --- a/docs/migrate.rst +++ b/docs/migrate.rst @@ -1,6 +1,45 @@ Migration Guide =============== +Migrating to Rally 1.1.0 +------------------------ + +``request-params`` in operations are passed as is and not serialized +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +With Rally 1.1.0 any operations supporting the optional ``request-params`` property will pass the structure as is without attempting to serialize values. +Until now, ``request-params`` relied on parameters being supported by the Elasticsearch Python client API calls. This means that for example boolean type parameters +should be specified as strings i.e. ``"true"`` or ``"false"`` rather than ``true/false``. + +**Example** + +Using ``create-index`` before ``1.1.0``:: + + { + "name": "create-all-indices", + "operation-type": "create-index", + "settings": { + "index.number_of_shards": 1 + }, + "request-params": { + "wait_for_active_shards": true + } + } + +Using ``create-index`` starting with ``1.1.0``:: + + { + "name": "create-all-indices", + "operation-type": "create-index", + "settings": { + "index.number_of_shards": 1 + }, + "request-params": { + "wait_for_active_shards": "true" + } + } + + Migrating to Rally 1.0.1 ------------------------ diff --git a/docs/track.rst b/docs/track.rst index 3656f332f..f477b5633 100644 --- a/docs/track.rst +++ b/docs/track.rst @@ -369,7 +369,12 @@ With the operation type ``search`` you can execute `request body searches `_ that defines which indices should be targeted by this query. Only needed if the ``index`` section contains more than one index. Otherwise, Rally will automatically derive the index to use. If you have defined multiple indices and want to query all of them, just specify ``"index": "_all"``. * ``type`` (optional): Defines the type within the specified index for this query. By default, no ``type`` will be used and the query will be performed across all types in the provided index. Also, types have been removed in Elasticsearch 7.0.0 so you must not specify this property if you want to benchmark Elasticsearch 7.0.0 or later. * ``cache`` (optional): Whether to use the query request cache. By default, Rally will define no value thus the default depends on the benchmark candidate settings and Elasticsearch version. -* ``request-params`` (optional): A structure containing arbitrary request parameters. The supported parameters names are documented in the `Python ES client API docs `_. Parameters that are implicitly set by Rally (e.g. `body` or `request_cache`) are not supported (i.e. you should not try to set them and if so expect unspecified behavior). +* ``request-params`` (optional): A structure containing arbitrary request parameters. The supported parameters names are documented in the `Search URI Request docs `_. + + .. note:: + 1. Parameters that are implicitly set by Rally (e.g. `body` or `request_cache`) are not supported (i.e. you should not try to set them and if so expect unspecified behavior). + 2. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). + * ``body`` (mandatory): The query body. * ``pages`` (optional): Number of pages to retrieve. If this parameter is present, a scroll query will be executed. If you want to retrieve all result pages, use the value "all". * ``results-per-page`` (optional): Number of documents to retrieve per page for scroll queries. @@ -386,7 +391,7 @@ Example:: }, "request-params": { "_source_include": "some_field", - "analyze_wildcard": false + "analyze_wildcard": "false" } } @@ -435,7 +440,7 @@ cluster-health With the operation ``cluster-health`` you can execute the `cluster health API `_. It supports the following properties: -* ``request-params`` (optional): A structure containing any request parameters that are allowed by the cluster health API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the cluster health API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). * ``index`` (optional): The name of the index that should be used to check. The ``cluster-health`` operation will check whether the expected cluster health and will report a failure if this is not the case. Use ``--on-error`` on the command line to control Rally's behavior in case of such failures. @@ -471,13 +476,13 @@ With the operation ``create-index`` you can execute the `create index API `__ that are allowed by the create index API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the create index API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). If you want it to create one specific index instead, you can specify the following properties: * ``index`` (mandatory): One or more names of the indices that should be created. If only one index should be created, you can use a string otherwise this needs to be a list of strings. * ``body`` (optional): The body for the create index API call. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the create index API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the create index API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). **Examples** @@ -490,7 +495,7 @@ The following snippet will create all indices that have been defined in the ``in "index.number_of_shards": 1 }, "request-params": { - "wait_for_active_shards": true + "wait_for_active_shards": "true" } } @@ -529,13 +534,13 @@ With the operation ``delete-index`` you can execute the `delete index API `__ that are allowed by the delete index API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the delete index API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). If you want it to delete one specific index (pattern) instead, you can specify the following properties: * ``index`` (mandatory): One or more names of the indices that should be deleted. If only one index should be deleted, you can use a string otherwise this needs to be a list of strings. * ``only-if-exists`` (optional, defaults to ``true``): Defines whether an index should only be deleted if it exists. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the delete index API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the delete index API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). **Examples** @@ -555,8 +560,8 @@ With the following snippet we will delete all ``logs-*`` indices:: "only-if-exists": false, "request-params": { "expand_wildcards": "all", - "allow_no_indices": true, - "ignore_unavailable": true + "allow_no_indices": "true", + "ignore_unavailable": "true" } } @@ -571,13 +576,13 @@ If you want it to create index templates that have been declared in the ``templa * ``template`` (optional): If you specify a template name, only the template with this name will be created. * ``settings`` (optional): Allows to specify additional settings that will be merged with the settings specified in the body of the index template in the ``templates`` section. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the create index template API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the create index template API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). If you want it to create one specific index instead, you can specify the following properties: * ``template`` (mandatory): The name of the index template that should be created. * ``body`` (mandatory): The body for the create index template API call. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the create index template API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the create index template API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters (see example below). **Examples** @@ -587,7 +592,7 @@ The following snippet will create all index templates that have been defined in "name": "create-all-templates", "operation-type": "create-index-template", "request-params": { - "create": true + "create": "true" } } @@ -625,7 +630,7 @@ With the operation ``delete-index-template`` you can execute the `delete index t If you want it to delete all index templates that have been declared in the ``templates`` section, you can specify the following properties: * ``only-if-exists`` (optional, defaults to ``true``): Defines whether an index template should only be deleted if it exists. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the delete index template API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the delete index template API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters. If you want it to delete one specific index template instead, you can specify the following properties: @@ -633,7 +638,7 @@ If you want it to delete one specific index template instead, you can specify th * ``only-if-exists`` (optional, defaults to ``true``): Defines whether the index template should only be deleted if it exists. * ``delete-matching-indices`` (optional, defaults to ``false``): Whether to delete indices that match the index template's index pattern. * ``index-pattern`` (mandatory iff ``delete-matching-indices`` is ``true``): Specifies the index pattern to delete. -* ``request-params`` (optional): A structure containing any `request parameters `__ that are allowed by the delete index template API. +* ``request-params`` (optional): A structure containing any request parameters that are allowed by the delete index template API. Rally will not attempt to serialize the parameters and pass them as is. Always use "true" / "false" strings for boolean parameters. **Examples** diff --git a/esrally/driver/runner.py b/esrally/driver/runner.py index 5d4544840..1b13228c4 100644 --- a/esrally/driver/runner.py +++ b/esrally/driver/runner.py @@ -593,12 +593,12 @@ def __call__(self, es, params): def request_body_query(self, es, params): request_params = params.get("request-params", {}) if "cache" in params: - request_params["request_cache"] = params["cache"] + request_params["request_cache"] = str(params["cache"]).lower() r = es.search( index=params.get("index", "_all"), doc_type=params.get("type"), body=mandatory(params, "body", self), - **request_params) + params=request_params) hits = r["hits"]["total"] if isinstance(hits, dict): hits_total = hits["value"] @@ -637,7 +637,7 @@ def scroll_query(self, es, params): scroll="10s", size=size, request_cache=cache, - **request_params + params=request_params ) # This should only happen if we concurrently create an index and start searching self.scroll_id = r.get("_scroll_id", None) @@ -727,9 +727,7 @@ def status(v): # either the user has defined something or we're good with any count of relocating shards. expected_relocating_shards = int(request_params.get("wait_for_relocating_shards", sys.maxsize)) - # This would not work if the request parameter is not a proper method parameter for the ES client... - # result = es.cluster.health(**request_params) - result = es.transport.perform_request("GET", _make_path("_cluster", "health", index), params=request_params) + result = es.cluster.health(index=index, params=request_params) cluster_status = result["status"] relocating_shards = result["relocating_shards"] @@ -783,12 +781,7 @@ def __call__(self, es, params): indices = mandatory(params, "indices", self) request_params = params.get("request-params", {}) for index, body in indices: - # We don't use es.indices.create() because it doesn't support params - # Ref: https://elasticsearch-py.readthedocs.io/en/master/api.html?highlight=indices%20create#elasticsearch.client.IndicesClient.create - es.transport.perform_request(method="PUT", - url="/{}".format(index), - body=body, - params=request_params) + es.indices.create(index=index, body=body, params=request_params) return len(indices), "ops" def __repr__(self, *args, **kwargs): @@ -809,11 +802,11 @@ def __call__(self, es, params): for index_name in indices: if not only_if_exists: - es.indices.delete(index=index_name, **request_params) + es.indices.delete(index=index_name, params=request_params) ops += 1 elif only_if_exists and es.indices.exists(index=index_name): self.logger.info("Index [%s] already exists. Deleting it.", index_name) - es.indices.delete(index=index_name, **request_params) + es.indices.delete(index=index_name, params=request_params) ops += 1 return ops, "ops" @@ -833,7 +826,7 @@ def __call__(self, es, params): for template, body in templates: es.indices.put_template(name=template, body=body, - **request_params) + params=request_params) return len(templates), "ops" def __repr__(self, *args, **kwargs): @@ -853,11 +846,11 @@ def __call__(self, es, params): for template_name, delete_matching_indices, index_pattern in template_names: if not only_if_exists: - es.indices.delete_template(name=template_name, **request_params) + es.indices.delete_template(name=template_name, params=request_params) ops_count += 1 elif only_if_exists and es.indices.exists_template(template_name): self.logger.info("Index template [%s] already exists. Deleting it.", template_name) - es.indices.delete_template(name=template_name, **request_params) + es.indices.delete_template(name=template_name, params=request_params) ops_count += 1 # ensure that we do not provide an empty index pattern by accident if delete_matching_indices and index_pattern: diff --git a/tests/driver/runner_test.py b/tests/driver/runner_test.py index 0d2793133..fe344e382 100644 --- a/tests/driver/runner_test.py +++ b/tests/driver/runner_test.py @@ -690,6 +690,7 @@ def test_query_match_only_request_body_defined(self, es): query_runner = runner.Query() params = { + "cache": True, "body": { "query": { "match_all": {} @@ -708,6 +709,65 @@ def test_query_match_only_request_body_defined(self, es): self.assertEqual(5, result["took"]) self.assertFalse("error-type" in result) + es.search.assert_called_once_with( + index="_all", + doc_type=None, + body=params["body"], + params={"request_cache": "true"} + ) + + @mock.patch("elasticsearch.Elasticsearch") + def test_query_match_using_request_params(self, es): + es.search.return_value = { + "timed_out": False, + "took": 62, + "hits": { + "total": { + "value": 2, + "relation": "eq" + }, + "hits": [ + { + "some-doc-1" + }, + { + "some-doc-2" + } + + ] + } + } + + query_runner = runner.Query() + params = { + "cache": True, + "body": None, + "request-params": { + "q": "user:kimchy" + } + } + + with query_runner: + result = query_runner(es, params) + + self.assertEqual(1, result["weight"]) + self.assertEqual("ops", result["unit"]) + self.assertEqual(2, result["hits"]) + self.assertEqual("eq", result["hits_relation"]) + self.assertFalse(result["timed_out"]) + self.assertEqual(62, result["took"]) + self.assertFalse("error-type" in result) + + es.search.assert_called_once_with( + index="_all", + doc_type=None, + body=params["body"], + params={ + "request_cache": "true", + "q": "user:kimchy" + } + ) + @mock.patch("elasticsearch.Elasticsearch") def test_query_hits_total_as_number(self, es): es.search.return_value = { @@ -1195,7 +1255,7 @@ def test_param_id_mandatory(self, es): class ClusterHealthRunnerTests(TestCase): @mock.patch("elasticsearch.Elasticsearch") def test_waits_for_expected_cluster_status(self, es): - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": "green", "relocating_shards": 0 } @@ -1217,11 +1277,11 @@ def test_waits_for_expected_cluster_status(self, es): "relocating-shards": 0 }, result) - es.transport.perform_request.assert_called_once_with("GET", "/_cluster/health", params={"wait_for_status": "green"}) + es.cluster.health.assert_called_once_with(index=None, params={"wait_for_status": "green"}) @mock.patch("elasticsearch.Elasticsearch") def test_accepts_better_cluster_status(self, es): - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": "green", "relocating_shards": 0 } @@ -1243,11 +1303,11 @@ def test_accepts_better_cluster_status(self, es): "relocating-shards": 0 }, result) - es.transport.perform_request.assert_called_once_with("GET", "/_cluster/health", params={"wait_for_status": "yellow"}) + es.cluster.health.assert_called_once_with(index=None, params={"wait_for_status": "yellow"}) @mock.patch("elasticsearch.Elasticsearch") def test_rejects_relocating_shards(self, es): - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": "yellow", "relocating_shards": 3 } @@ -1271,12 +1331,12 @@ def test_rejects_relocating_shards(self, es): "relocating-shards": 3 }, result) - es.transport.perform_request.assert_called_once_with("GET", "/_cluster/health/logs-*", - params={"wait_for_status": "red", "wait_for_no_relocating_shards": True}) + es.cluster.health.assert_called_once_with(index="logs-*", + params={"wait_for_status": "red", "wait_for_no_relocating_shards": True}) @mock.patch("elasticsearch.Elasticsearch") def test_rejects_unknown_cluster_status(self, es): - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": None, "relocating_shards": 0 } @@ -1298,7 +1358,7 @@ def test_rejects_unknown_cluster_status(self, es): "relocating-shards": 0 }, result) - es.transport.perform_request.assert_called_once_with("GET", "/_cluster/health", params={"wait_for_status": "green"}) + es.cluster.health.assert_called_once_with(index=None, params={"wait_for_status": "green"}) class CreateIndexRunnerTests(TestCase): @@ -1307,7 +1367,7 @@ def test_creates_multiple_indices(self, es): r = runner.CreateIndex() request_params = { - "wait_for_active_shards": True + "wait_for_active_shards": "true" } params = { @@ -1322,9 +1382,9 @@ def test_creates_multiple_indices(self, es): self.assertEqual((2, "ops"), result) - es.transport.perform_request.assert_has_calls([ - mock.call(method="PUT", url="/indexA", body={"settings": {}}, params=request_params), - mock.call(method="PUT", url="/indexB", body={"settings": {}}, params=request_params) + es.indices.create.assert_has_calls([ + mock.call(index="indexA", body={"settings": {}}, params=request_params), + mock.call(index="indexB", body={"settings": {}}, params=request_params) ]) @mock.patch("elasticsearch.Elasticsearch") @@ -1356,7 +1416,7 @@ def test_deletes_existing_indices(self, es): self.assertEqual((1, "ops"), result) - es.indices.delete.assert_called_once_with(index="indexB") + es.indices.delete.assert_called_once_with(index="indexB", params={}) @mock.patch("elasticsearch.Elasticsearch") def test_deletes_all_indices(self, es): @@ -1366,7 +1426,7 @@ def test_deletes_all_indices(self, es): "indices": ["indexA", "indexB"], "only-if-exists": False, "request-params": { - "ignore_unavailable": True, + "ignore_unavailable": "true", "expand_wildcards": "none" } } @@ -1376,8 +1436,8 @@ def test_deletes_all_indices(self, es): self.assertEqual((2, "ops"), result) es.indices.delete.assert_has_calls([ - mock.call(index="indexA", ignore_unavailable=True, expand_wildcards="none"), - mock.call(index="indexB", ignore_unavailable=True, expand_wildcards="none") + mock.call(index="indexA", params=params["request-params"]), + mock.call(index="indexB", params=params["request-params"]) ]) self.assertEqual(0, es.indices.exists.call_count) @@ -1394,7 +1454,7 @@ def test_create_index_templates(self, es): ], "request-params": { "timeout": 50, - "create": True + "create": "true" } } @@ -1403,8 +1463,8 @@ def test_create_index_templates(self, es): self.assertEqual((2, "ops"), result) es.indices.put_template.assert_has_calls([ - mock.call(name="templateA", body={"settings": {}}, timeout=50, create=True), - mock.call(name="templateB", body={"settings": {}}, timeout=50, create=True) + mock.call(name="templateA", body={"settings": {}}, params=params["request-params"]), + mock.call(name="templateB", body={"settings": {}}, params=params["request-params"]) ]) @mock.patch("elasticsearch.Elasticsearch") @@ -1440,8 +1500,8 @@ def test_deletes_all_index_templates(self, es): self.assertEqual((3, "ops"), result) es.indices.delete_template.assert_has_calls([ - mock.call(name="templateA", timeout=60), - mock.call(name="templateB", timeout=60) + mock.call(name="templateA", params=params["request-params"]), + mock.call(name="templateB", params=params["request-params"]) ]) es.indices.delete.assert_called_once_with(index="logs-*") @@ -1467,7 +1527,7 @@ def test_deletes_only_existing_index_templates(self, es): # 2 times delete index template, one time delete matching indices self.assertEqual((1, "ops"), result) - es.indices.delete_template.assert_called_once_with(name="templateB", timeout=60) + es.indices.delete_template.assert_called_once_with(name="templateB", params=params["request-params"]) # not called because the matching index is empty. self.assertEqual(0, es.indices.delete.call_count) @@ -1744,7 +1804,7 @@ class ShrinkIndexTests(TestCase): @mock.patch("time.sleep") def test_shrink_index_with_shrink_node(self, sleep, es): # cluster health API - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": "green", "relocating_shards": 0 } @@ -1773,9 +1833,9 @@ def test_shrink_index_with_shrink_node(self, sleep, es): }, preserve_existing=True) - es.transport.perform_request.assert_has_calls([ - mock.call("GET", "/_cluster/health/src", params={"wait_for_no_relocating_shards": "true"}), - mock.call("GET", "/_cluster/health/target", params={"wait_for_no_relocating_shards": "true"}), + es.cluster.health.assert_has_calls([ + mock.call(index="src", params={"wait_for_no_relocating_shards": "true"}), + mock.call(index="target", params={"wait_for_no_relocating_shards": "true"}), ]) es.indices.shrink.assert_called_once_with(index="src", target="target", body={ @@ -1792,7 +1852,7 @@ def test_shrink_index_with_shrink_node(self, sleep, es): @mock.patch("time.sleep") def test_shrink_index_derives_shrink_node(self, sleep, es): # cluster health API - es.transport.perform_request.return_value = { + es.cluster.health.return_value = { "status": "green", "relocating_shards": 0 } @@ -1851,9 +1911,9 @@ def test_shrink_index_derives_shrink_node(self, sleep, es): }, preserve_existing=True) - es.transport.perform_request.assert_has_calls([ - mock.call("GET", "/_cluster/health/src", params={"wait_for_no_relocating_shards": "true"}), - mock.call("GET", "/_cluster/health/target", params={"wait_for_no_relocating_shards": "true"}), + es.cluster.health.assert_has_calls([ + mock.call(index="src", params={"wait_for_no_relocating_shards": "true"}), + mock.call(index="target", params={"wait_for_no_relocating_shards": "true"}), ]) es.indices.shrink.assert_called_once_with(index="src", target="target", body={