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

Allow to forcefully close SSL connections #1016

Merged
merged 1 commit into from
Jun 12, 2020
Merged

Allow to forcefully close SSL connections #1016

merged 1 commit into from
Jun 12, 2020

Conversation

danielmitterdorfer
Copy link
Member

With this commit we expose a new client option enable_cleanup_closed
which allows to forcefully close an SSL connection that might not have
been properly closed on the server-side. This avoids potentially leaking
connections.

@danielmitterdorfer
Allow to forcefully close SSL connections
1ee2e01 
With this commit we expose a new client option `enable_cleanup_closed`
which allows to forcefully close an SSL connection that might not have
been properly closed on the server-side. This avoids potentially leaking
connections.
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc. labels Jun 12, 2020
@danielmitterdorfer danielmitterdorfer added this to the 2.0.1 milestone Jun 12, 2020
@danielmitterdorfer danielmitterdorfer self-assigned this Jun 12, 2020
dliappis
dliappis approved these changes Jun 12, 2020
View reviewed changes
Copy link
Contributor

@dliappis dliappis left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@danielmitterdorfer danielmitterdorfer merged commit 6961f87 into elastic:master Jun 12, 2020
@danielmitterdorfer danielmitterdorfer deleted the enable-cleanup-closed branch June 12, 2020 11:12
@jaymode
Copy link
Member

jaymode commented Jun 25, 2020

Do we know if there is an issue with how Elasticsearch closes SSL connections that needs to be fixed as well?

@dliappis
Copy link
Contributor

Do we know if there is an issue with how Elasticsearch closes SSL connections that needs to be fixed as well?

A deep investigation on the Elasticsearch side hasn't been done.

The observation was that aiohttp Python library ended up constantly increasing connections and according to the description of enable_cleanup_closed:

enable_cleanup_closed (bool) – some SSL servers do not properly complete SSL shutdown process, in that case asyncio leaks ssl connections. If this parameter is set to True, aiohttp additionally aborts underlining transport after 2 seconds. It is off by default.

Me and @danielmitterdorfer were both skeptical, given the wide usage that Elasticsearch using TLS has, that this would be the first time it's been noticed and wondered if it's some quirk of the Python aiohttp library instead. Have you heard any reports of such an issue yourself on the Elasticsearch side or with another library?

@pquentin
Copy link
Member

For the record, there is an issue with Elasticsearch and TLS 1.3: elastic/elasticsearch#76642

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dliappis dliappis dliappis approved these changes

Assignees

@danielmitterdorfer danielmitterdorfer

Labels
enhancement Improves the status quo :Load Driver Changes that affect the core of the load driver such as scheduling, the measurement approach etc.
Projects
None yet
Milestone
2.0.1
Development

Successfully merging this pull request may close these issues.
4 participants
@danielmitterdorfer @jaymode @dliappis @pquentin