Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove buggy indices.exists() calls from code #1580

Merged
merged 7 commits into from
Sep 14, 2022
Merged

Remove buggy indices.exists() calls from code #1580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e46de91
fix deleting big indices
Sep 13, 2022
dd1567d
docs and formatting
Sep 13, 2022
8775206
test fix
Sep 13, 2022
b664ca9
pivot to using .get()
Sep 13, 2022
cb9936a
allow us to handle missing data streams
Sep 14, 2022
8f0acad
fix up more operations and leave a comment
Sep 14, 2022
e906071
properly handle legacy index template api
Sep 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 40 additions & 17 deletions esrally/driver/runner.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1371,10 +1371,15 @@ async def __call__(self, es, params):
if not only_if_exists:
await es.indices.delete(index=index_name, params=request_params)
ops += 1
elif only_if_exists and await es.indices.exists(index=index_name):
self.logger.info("Index [%s] already exists. Deleting it.", index_name)
await es.indices.delete(index=index_name, params=request_params)
ops += 1
elif only_if_exists:
# here we use .get() and check for 404 instead of exists due to a bug in some versions
# of elasticsearch-py/elastic-transport with HEAD calls.
# can change back once using elasticsearch-py >= 8.0.0 and elastic-transport >= 8.1.0
get_response = await es.indices.get(index=index_name, ignore=[404])
if not get_response.get("status") == 404:
self.logger.info("Index [%s] already exists. Deleting it.", index_name)
await es.indices.delete(index=index_name, params=request_params)
ops += 1
finally:
await set_destructive_requires_name(es, prior_destructive_setting)
return {
Expand Down Expand Up @@ -1403,10 +1408,15 @@ async def __call__(self, es, params):
if not only_if_exists:
await es.indices.delete_data_stream(name=data_stream, ignore=[404], params=request_params)
ops += 1
elif only_if_exists and await es.indices.exists(index=data_stream):
self.logger.info("Data stream [%s] already exists. Deleting it.", data_stream)
await es.indices.delete_data_stream(name=data_stream, params=request_params)
ops += 1
elif only_if_exists:
# here we use .get() and check for 404 instead of exists due to a bug in some versions
# of elasticsearch-py/elastic-transport with HEAD calls.
# can change back once using elasticsearch-py >= 8.0.0 and elastic-transport >= 8.1.0
get_response = await es.indices.get(index=data_stream, ignore=[404])
if not get_response.get("status") == 404:
self.logger.info("Data stream [%s] already exists. Deleting it.", data_stream)
await es.indices.delete_data_stream(name=data_stream, params=request_params)
ops += 1

return {
"weight": ops,
Expand Down Expand Up @@ -1455,10 +1465,15 @@ async def __call__(self, es, params):
if not only_if_exists:
await es.cluster.delete_component_template(name=template_name, params=request_params, ignore=[404])
ops_count += 1
elif only_if_exists and await es.cluster.exists_component_template(name=template_name):
self.logger.info("Component Index template [%s] already exists. Deleting it.", template_name)
await es.cluster.delete_component_template(name=template_name, params=request_params)
ops_count += 1
elif only_if_exists:
# here we use .get() and check for 404 instead of exists_component_template due to a bug in some versions
# of elasticsearch-py/elastic-transport with HEAD calls.
# can change back once using elasticsearch-py >= 8.0.0 and elastic-transport >= 8.1.0
component_template_exists = await es.cluster.get_component_template(name=template_name, ignore=[404])
if not component_template_exists.get("status") == 404:
self.logger.info("Component Index template [%s] already exists. Deleting it.", template_name)
await es.cluster.delete_component_template(name=template_name, params=request_params)
ops_count += 1
return {
"weight": ops_count,
"unit": "ops",
Expand Down Expand Up @@ -1505,10 +1520,15 @@ async def __call__(self, es, params):
if not only_if_exists:
await es.indices.delete_index_template(name=template_name, params=request_params, ignore=[404])
ops_count += 1
elif only_if_exists and await es.indices.exists_index_template(name=template_name):
self.logger.info("Composable Index template [%s] already exists. Deleting it.", template_name)
await es.indices.delete_index_template(name=template_name, params=request_params)
ops_count += 1
elif only_if_exists:
# here we use .get() and check for 404 instead of exists_index_template due to a bug in some versions
# of elasticsearch-py/elastic-transport with HEAD calls.
# can change back once using elasticsearch-py >= 8.0.0 and elastic-transport >= 8.1.0
index_template_exists = await es.indices.get_index_template(name=template_name, ignore=[404])
if not index_template_exists.get("status") == 404:
self.logger.info("Composable Index template [%s] already exists. Deleting it.", template_name)
await es.indices.delete_index_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:
await es.indices.delete(index=index_pattern)
Expand Down Expand Up @@ -1560,7 +1580,10 @@ async def __call__(self, es, params):
if not only_if_exists:
await es.indices.delete_template(name=template_name, params=request_params)
ops_count += 1
elif only_if_exists and await es.indices.exists_template(name=template_name):
# here we use .get_template() and check for empty instead of exists_template due to a bug in some versions
# of elasticsearch-py/elastic-transport with HEAD calls.
# can change back once using elasticsearch-py >= 8.0.0 and elastic-transport >= 8.1.0
elif only_if_exists and await es.indices.get_template(name=template_name, ignore=[404]):
self.logger.info("Index template [%s] already exists. Deleting it.", template_name)
await es.indices.delete_template(name=template_name, params=request_params)
ops_count += 1
Expand Down
13 changes: 5 additions & 8 deletions tests/driver/runner_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2712,7 +2712,7 @@ class TestDeleteIndexRunner:
@mock.patch("elasticsearch.Elasticsearch")
@pytest.mark.asyncio
async def test_deletes_existing_indices(self, es):
es.indices.exists = mock.AsyncMock(side_effect=[False, True])
es.indices.get = mock.AsyncMock(side_effect=[{"status": 404}, {"status": 200}])
es.indices.delete = mock.AsyncMock()
es.cluster.get_settings = mock.AsyncMock(return_value={"persistent": {}, "transient": {"action.destructive_requires_name": True}})
es.cluster.put_settings = mock.AsyncMock()
Expand Down Expand Up @@ -2777,7 +2777,7 @@ class TestDeleteDataStreamRunner:
@mock.patch("elasticsearch.Elasticsearch")
@pytest.mark.asyncio
async def test_deletes_existing_data_streams(self, es):
es.indices.exists = mock.AsyncMock(side_effect=[False, True])
es.indices.get = mock.AsyncMock(side_effect=[{"status": 404}, {"status": 200}])
es.indices.delete_data_stream = mock.AsyncMock()

r = runner.DeleteDataStream()
Expand Down Expand Up @@ -2907,7 +2907,7 @@ async def test_deletes_all_index_templates(self, es):
@mock.patch("elasticsearch.Elasticsearch")
@pytest.mark.asyncio
async def test_deletes_only_existing_index_templates(self, es):
es.indices.exists_template = mock.AsyncMock(side_effect=[False, True])
es.indices.get_template = mock.AsyncMock(side_effect=[False, True])
es.indices.delete_template = mock.AsyncMock()
es.indices.delete = mock.AsyncMock()

Expand Down Expand Up @@ -3038,10 +3038,7 @@ async def test_deletes_all_index_templates(self, es):
@mock.patch("elasticsearch.Elasticsearch")
@pytest.mark.asyncio
async def test_deletes_only_existing_index_templates(self, es):
async def _side_effect(name):
return name == "templateB"

es.cluster.exists_component_template = mock.AsyncMock(side_effect=_side_effect)
es.cluster.get_component_template = mock.AsyncMock(side_effect=[{"status": 404}, {"status": 200}])
es.cluster.delete_component_template = mock.AsyncMock()

r = runner.DeleteComponentTemplate()
Expand Down Expand Up @@ -3201,7 +3198,7 @@ async def test_deletes_all_index_templates(self, es):
@mock.patch("elasticsearch.Elasticsearch")
@pytest.mark.asyncio
async def test_deletes_only_existing_index_templates(self, es):
es.indices.exists_index_template = mock.AsyncMock(side_effect=[False, True])
es.indices.get_index_template = mock.AsyncMock(side_effect=[{"status": 404}, {"status": 200}])
es.indices.delete_index_template = mock.AsyncMock()

r = runner.DeleteComposableTemplate()
Expand Down