Skip to content

Add better handling for Keep-Alive connections #130

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

Merged
merged 1 commit into from
Dec 23, 2015

Conversation

itay
Copy link
Contributor

@itay itay commented Dec 10, 2015

Python's httplib generally has bad support for Keep-Alive connections,
which is why we never used it. We didn't explicitly ask for "Connection: Close",
but it was implicit. Additionally, splunkd would always return it.

However, an issue arises when talking to a Load Balancer which issues
a Keep-Alive request to splunkd, and returns that header to us. Python
then goes into a particular path which the code didn't handle gracefully.

What this change does is two-fold:

  1. Always request to close the connection via the "Connection: Close" header.
  2. If we get back a "Connection: Keep-Alive" header, we properly handle
    it by not immediately closing our reference to the connection and only
    doing so when the response has been completed.

Python's httplib generally has bad support for Keep-Alive connections,
which is why we never used it. We didn't explicitly ask for "Connection: Close",
but it was implicit. Additionally, splunkd would always return it.

However, an issue arises when talking to a Load Balancer which issues
a Keep-Alive request to splunkd, and returns that header to us. Python
then goes into a particular path which the code didn't handle gracefully.

What this change does is two-fold:

1. Always request to close the connection via the "Connection: Close" header.

2. If we get back a "Connection: Keep-Alive" header, we properly handle
it by not immediately closing our reference to the connection and only
doing so when the response has been completed.
@itay itay mentioned this pull request Dec 10, 2015
@David-Noble-at-work
Copy link

LGTM

@Qmando
Copy link

Qmando commented Dec 10, 2015

This fixes the problem I was having. Thanks.

itay added a commit that referenced this pull request Dec 23, 2015
Add better handling for Keep-Alive connections
@itay itay merged commit 441f017 into develop Dec 23, 2015
@shakeelmohamed shakeelmohamed deleted the bugfix/handle-keep-alive-connections branch May 19, 2016 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants