Skip to content

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