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

Pin transitive dep 'aiohttp==3.8.6' #1806

Merged
merged 2 commits into from
Nov 22, 2023
Merged

Conversation

b-deam
Copy link
Member

@b-deam b-deam commented Nov 22, 2023

Debugging #1804 confirmed we're hitting this bug aio-libs/aiohttp#7864.

aiohttp is a transitive dependency of esrally and 3.9.0 was released on the 18th Nov, which is around the first time we noticed this bug that was caught by CI on the 19th.

Pinning this at 3.8.6 until a fix is merged upstream.

Reproduction

You too can reproduce this bug with an Elasticsearch cluster and this script:

from elasticsearch import AsyncElasticsearch
import elasticsearch
import asyncio
import ssl
import certifi
import warnings

warnings.filterwarnings("ignore", category=DeprecationWarning)


ssl_context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH, cafile=certifi.where())
ssl_context.check_hostname = False
ssl_context.verify_mode = ssl.CERT_NONE

es = AsyncElasticsearch(
    hosts=["https://elasticsearch:9200"],
    ssl_context=ssl_context,
    verify_certs=False,
    basic_auth=("elastic", "changeme"),
)


async def tasks():
    complete = False
    try:
        await es.indices.forcemerge(request_timeout=0.001)
        complete = True
    except elasticsearch.ConnectionTimeout:
        print("Timing out")
        pass
    while not complete:
        tasks = await es.tasks.list(params={"actions": "indices:admin/forcemerge"})
        if len(tasks["nodes"]) == 0:
            # empty nodes response indicates no tasks
            complete = True


async def create_datastreams():
    await es.cluster.put_settings(persistent={"cluster.max_shards_per_node": 5000})
    # Create an index lifecycle policy
    await es.ilm.put_lifecycle(name="repro", policy={"phases": {}})
    # Create component templates
    compontent_template = {"settings": {"index.lifecycle.name": "repro", "index.number_of_shards": 250}}
    await es.cluster.put_component_template(name="repro", template=compontent_template)
    # Create an index template
    await es.indices.put_index_template(name="repro", composed_of=["repro"], index_patterns="logs*", data_stream={})
    # Create the data stream
    for i in range(1, 11):
        await es.indices.create_data_stream(name=f"logs-{i}", ignore=400)


async def main():
    await create_datastreams()
    while True:
        await tasks()


if __name__ == "__main__":
    loop = asyncio.get_event_loop()
    loop.run_until_complete(main())

Test with each version of aiohttp:

$ pip install "aiohttp==3.9.0"
$ python reproduce_bug.py                          
Timing out
Traceback (most recent call last):
  File "resp_bug.py", line 60, in <module>
    loop.run_until_complete(main())
  File "/Users/bradleydeam/.pyenv/versions/3.8.16/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "resp_bug.py", line 55, in main
    await tasks()
  File "resp_bug.py", line 33, in tasks
    if len(tasks["nodes"]) == 0:
  File "/Users/bradleydeam/perf/github.com/b-deam/rally/.venv/lib/python3.8/site-packages/elastic_transport/_response.py", line 188, in __getitem__
    return self.body[item]  # type: ignore[index]
KeyError: 'nodes'
Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x1068be340>
Unclosed connector
connections: ['[(<aiohttp.client_proto.ResponseHandler object at 0x1068c2220>, 11.885923)]']
connector: <aiohttp.connector.TCPConnector object at 0x1068be400>

$ pip install "aiohttp==3.8.6"
# infinite loop
$ python resp_bug.py          
Timing out
Timing out
Timing out
Timing out
Timing out
^CTraceback (most recent call last):

@b-deam b-deam added bug Something's wrong :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. dependencies Pull requests that update a dependency file labels Nov 22, 2023
@b-deam b-deam requested a review from a team November 22, 2023 04:21
@b-deam b-deam self-assigned this Nov 22, 2023
Copy link
Member

@pquentin pquentin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM.

@b-deam
Copy link
Member Author

b-deam commented Nov 22, 2023

I actually wonder if this warrants a new release given that users installing rally from PyPi are going to (likely) get aiohttp 3.9.0?

Copy link
Member

@gareth-ellis gareth-ellis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@b-deam b-deam merged commit c1f04a3 into elastic:master Nov 22, 2023
15 checks passed
@b-deam b-deam added this to the 2.x milestone Nov 28, 2023
@b-deam b-deam mentioned this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something's wrong dependencies Pull requests that update a dependency file :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants