Skip to content

Enable TCP Keep-Alive in Docker client#415

Merged
thaJeztah merged 1 commit intodocker:masterfrom
jlhawn:client_dial_enable_tcp_keepalive
Aug 3, 2017
Merged

Enable TCP Keep-Alive in Docker client#415
thaJeztah merged 1 commit intodocker:masterfrom
jlhawn:client_dial_enable_tcp_keepalive

Conversation

@jlhawn
Copy link
Contributor

@jlhawn jlhawn commented Aug 2, 2017

Some network environments may have NATs, proxies, or gateways which
kill idle connections. There are many Docker API operations which may
be idle for long periods of time (such as ContainerWait and ContainerAttach)
and may result in unexpected connection closures or hangs if TCP keepalives
are not used.

This patch updates the default HTTP transport used by the Docker client
package to enable TCP Keep-Alive with a keep-alive interval of 30 seconds.
It also sets a connect timeout of 30 seconds.

This PR complements the change to the moby client package here: moby/moby#34359

Some network environments may have NATs, proxies, or gateways which
kill idle connections. There are many Docker API operations which may
be idle for long periods of time (such as ContainerWait and ContainerAttach)
and may result in unexpected connection closures or hangs if TCP keepalives
are not used.

This patch updates the default HTTP transport used by the Docker client
package to enable TCP Keep-Alive with a keep-alive interval of 30 seconds.
It also sets a connect timeout of 30 seconds.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
@thaJeztah
Copy link
Member

ugh circleCI keeps having problems, now its a memory issue (possibly);

./scripts/build/cross
Exited with code 137

Hint: Exit code 137 typyically means the process is killed because it was running out of memory
Hint: Check if you can optimize the memory usage in your app
Hint: Max memory usage of this container is 58572800
 according to /sys/fs/cgroup/memory/memory.max_usage_in_bytes

@codecov-io
Copy link

codecov-io commented Aug 3, 2017

Codecov Report

Merging #415 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #415      +/-   ##
==========================================
- Coverage   46.26%   46.25%   -0.02%     
==========================================
  Files         193      193              
  Lines       16093    16097       +4     
==========================================
  Hits         7445     7445              
- Misses       8259     8263       +4     
  Partials      389      389

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

not sure what's with CircleCI

never mind, it's green now 👍

Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit f7b78dc into docker:master Aug 3, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.08.0 milestone Aug 3, 2017
@jlhawn jlhawn deleted the client_dial_enable_tcp_keepalive branch August 3, 2017 17:48
tr := &http.Transport{
TLSClientConfig: config,
DialContext: (&net.Dialer{
KeepAlive: 30 * time.Second,
Copy link
Contributor

Choose a reason for hiding this comment

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

For now this can help keep connections alive over the NAT timeout issue we've been facing, but would really like to see more knobs to turn in a configurable manner much like curl: https://curl.haxx.se/libcurl/c/CURLOPT_TCP_KEEPALIVE.html

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants