Skip to content

Commit

Permalink
Remove timeout from putobject and listobjects (minio#9986)
Browse files Browse the repository at this point in the history
Use a separate client for these calls that can take a long time.

Add request context to these so they are canceled when the client 
disconnects instead except for ListObject which doesn't have any equivalent.
  • Loading branch information
klauspost authored Jul 7, 2020
1 parent 93e7e4a commit aa4d102
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 7 deletions.
29 changes: 25 additions & 4 deletions cmd/object-handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,7 @@ func getCpObjMetadataFromHeader(ctx context.Context, r *http.Request, userMeta m

// getRemoteInstanceTransport contains a singleton roundtripper.
var getRemoteInstanceTransport http.RoundTripper
var getRemoteInstanceTransportLongTO http.RoundTripper
var getRemoteInstanceTransportOnce sync.Once

// Returns a minio-go Client configured to access remote host described by destDNSRecord
Expand All @@ -715,11 +716,31 @@ var getRemoteInstanceClient = func(r *http.Request, host string) (*miniogo.Core,
}
getRemoteInstanceTransportOnce.Do(func() {
getRemoteInstanceTransport = NewGatewayHTTPTransport()
getRemoteInstanceTransportLongTO = newGatewayHTTPTransport(time.Hour)
})
core.SetCustomTransport(getRemoteInstanceTransport)
return core, nil
}

// Returns a minio-go Client configured to access remote host described by destDNSRecord
// Applicable only in a federated deployment.
// The transport does not contain any timeout except for dialing.
func getRemoteInstanceClientLongTimeout(r *http.Request, host string) (*miniogo.Core, error) {
cred := getReqAccessCred(r, globalServerRegion)
// In a federated deployment, all the instances share config files
// and hence expected to have same credentials.
core, err := miniogo.NewCore(host, cred.AccessKey, cred.SecretKey, globalIsSSL)
if err != nil {
return nil, err
}
getRemoteInstanceTransportOnce.Do(func() {
getRemoteInstanceTransport = NewGatewayHTTPTransport()
getRemoteInstanceTransportLongTO = newGatewayHTTPTransport(time.Hour)
})
core.SetCustomTransport(getRemoteInstanceTransportLongTO)
return core, nil
}

// Check if the destination bucket is on a remote site, this code only gets executed
// when federation is enabled, ie when globalDNSConfig is non 'nil'.
//
Expand Down Expand Up @@ -1166,7 +1187,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
}

// Send PutObject request to appropriate instance (in federated deployment)
client, rerr := getRemoteInstanceClient(r, getHostFromSrv(dstRecords))
client, rerr := getRemoteInstanceClientLongTimeout(r, getHostFromSrv(dstRecords))
if rerr != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, rerr), r.URL, guessIsBrowserReq(r))
return
Expand All @@ -1181,7 +1202,7 @@ func (api objectAPIHandlers) CopyObjectHandler(w http.ResponseWriter, r *http.Re
ServerSideEncryption: dstOpts.ServerSideEncryption,
UserTags: tag.ToMap(),
}
remoteObjInfo, rerr := client.PutObject(dstBucket, dstObject, srcInfo.Reader,
remoteObjInfo, rerr := client.PutObjectWithContext(ctx, dstBucket, dstObject, srcInfo.Reader,
srcInfo.Size, "", "", opts)
if rerr != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, rerr), r.URL, guessIsBrowserReq(r))
Expand Down Expand Up @@ -1858,13 +1879,13 @@ func (api objectAPIHandlers) CopyObjectPartHandler(w http.ResponseWriter, r *htt
}

// Send PutObject request to appropriate instance (in federated deployment)
client, rerr := getRemoteInstanceClient(r, getHostFromSrv(dstRecords))
client, rerr := getRemoteInstanceClientLongTimeout(r, getHostFromSrv(dstRecords))
if rerr != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, rerr), r.URL, guessIsBrowserReq(r))
return
}

partInfo, err := client.PutObjectPart(dstBucket, dstObject, uploadID, partID,
partInfo, err := client.PutObjectPartWithContext(ctx, dstBucket, dstObject, uploadID, partID,
srcInfo.Reader, srcInfo.Size, "", "", dstOpts.ServerSideEncryption)
if err != nil {
writeErrorResponse(ctx, w, toAPIError(ctx, err), r.URL, guessIsBrowserReq(r))
Expand Down
7 changes: 5 additions & 2 deletions cmd/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,13 +477,16 @@ func newCustomHTTPTransport(tlsConfig *tls.Config, dialTimeout time.Duration) fu
// This sets the value for MaxIdleConnsPerHost from 2 (go default)
// to 256.
func NewGatewayHTTPTransport() *http.Transport {
return newGatewayHTTPTransport(1 * time.Minute)
}

func newGatewayHTTPTransport(timeout time.Duration) *http.Transport {
tr := newCustomHTTPTransport(&tls.Config{
RootCAs: globalRootCAs,
}, defaultDialTimeout)()
// Set aggressive timeouts for gateway
tr.ResponseHeaderTimeout = 1 * time.Minute

// Allow more requests to be in flight.
tr.ResponseHeaderTimeout = timeout
tr.MaxConnsPerHost = 256
tr.MaxIdleConnsPerHost = 16
tr.MaxIdleConns = 256
Expand Down
2 changes: 1 addition & 1 deletion cmd/web-handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ func (web *webAPIHandlers) ListObjects(r *http.Request, args *ListObjectsArgs, r
}
return toJSONError(ctx, err, args.BucketName)
}
core, err := getRemoteInstanceClient(r, getHostFromSrv(sr))
core, err := getRemoteInstanceClientLongTimeout(r, getHostFromSrv(sr))
if err != nil {
return toJSONError(ctx, err, args.BucketName)
}
Expand Down

0 comments on commit aa4d102

Please sign in to comment.