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.

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

Remove implicit request timeout from force merge operation in polling #1122

Merged
merged 1 commit into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions esrally/driver/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -676,9 +676,6 @@ async def __call__(self, es, params):
if max_num_segments:
merge_params["max_num_segments"] = max_num_segments
if mode == "polling":
# we ignore the request_timeout if we are in polling mode and deliberately timeout early
# no reason to wait as long as a whole {polling-period} (which has a minimum of 1 second)
merge_params["request_timeout"] = 1
complete = False
try:
await es.indices.forcemerge(**merge_params)
Expand All @@ -687,7 +684,7 @@ async def __call__(self, es, params):
pass
while not complete:
await asyncio.sleep(params.get("poll-period"))
tasks = await es.tasks.list(params={"actions":"indices:admin/forcemerge"})
tasks = await es.tasks.list(params={"actions": "indices:admin/forcemerge"})
if len(tasks["nodes"]) == 0:
# empty nodes response indicates no tasks
complete = True
Expand Down
8 changes: 4 additions & 4 deletions tests/driver/runner_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ async def test_force_merge_with_polling_no_timeout(self, es):

force_merge = runner.ForceMerge()
await force_merge(es, params={"index" : "_all", "mode": "polling", 'poll-period': 0})
es.indices.forcemerge.assert_called_once_with(index="_all", request_timeout=1)
es.indices.forcemerge.assert_called_once_with(index="_all")

@mock.patch("elasticsearch.Elasticsearch")
@run_async
Expand Down Expand Up @@ -1038,8 +1038,8 @@ async def test_force_merge_with_polling(self, es):
})
]
force_merge = runner.ForceMerge()
await force_merge(es, params={"index" : "_all", "mode": "polling", "poll-period": 0})
es.indices.forcemerge.assert_called_once_with(index="_all", request_timeout=1)
await force_merge(es, params={"index": "_all", "mode": "polling", "poll-period": 0})
es.indices.forcemerge.assert_called_once_with(index="_all")

@mock.patch("elasticsearch.Elasticsearch")
@run_async
Expand Down Expand Up @@ -1092,7 +1092,7 @@ async def test_force_merge_with_polling_and_params(self, es):
# request-timeout should be ignored as mode:polling
await force_merge(es, params={"index" : "_all", "mode": "polling", "max-num-segments": 1,
"request-timeout": 50000, "poll-period": 0})
es.indices.forcemerge.assert_called_once_with(index="_all", max_num_segments=1, request_timeout=1)
es.indices.forcemerge.assert_called_once_with(index="_all", max_num_segments=1, request_timeout=50000)


class IndicesStatsRunnerTests(TestCase):
Expand Down