Skip to content

pkg/httputil: configure HTTP transport timeouts - #645

Open
thaJeztah wants to merge 1 commit into
rootless-containers:masterfrom
thaJeztah:httpclient_opts
Open

thaJeztah wants to merge 1 commit into
rootless-containers:masterfrom
thaJeztah:httpclient_opts

Conversation

@thaJeztah

Copy link
Copy Markdown
Contributor

Configure a timeout for connecting to the RootlessKit API socket, and set an idle connection timeout on the HTTP transport.

Previously, idle connections had no expiration because IdleConnTimeout was left at its zero value. This could leave connections open indefinitely for callers that create short-lived clients without explicitly closing idle connections.

Use the same dial and idle connection timeouts as http.DefaultTransport.

Configure a timeout for connecting to the RootlessKit API socket, and set
an idle connection timeout on the HTTP transport.

Previously, idle connections had no expiration because IdleConnTimeout was
left at its zero value. This could leave connections open indefinitely for
callers that create short-lived clients without explicitly closing idle
connections.

Use the same dial and idle connection timeouts as http.DefaultTransport.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah

Copy link
Copy Markdown
Contributor Author

cc @AkihiroSuda

Comment thread pkg/httputil/httputil.go
}

dialer := &net.Dialer{
Timeout: 30 * time.Second,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be passed from the caller as an option like WithTimeout() ?

Comment thread pkg/httputil/httputil.go
return d.DialContext(ctx, "unix", socketPath)
return dialer.DialContext(ctx, "unix", socketPath)
},
IdleConnTimeout: 90 * time.Second,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe:

Suggested change
IdleConnTimeout: 90 * time.Second,
IdleConnTimeout: dialer.Timeout * 3

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.

2 participants