introduce reader deadlines for net.Conn (#19023)

Bonus: set "retry-after" header for AWS SDKs if possible to honor them.
This commit is contained in:
Harshavardhana 2024-02-09 13:25:16 -08:00 committed by GitHub
parent 8e68ff9321
commit 997ba3a574
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 183 additions and 142 deletions

View File

@ -24,6 +24,7 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"net/url" "net/url"
"os"
"strconv" "strconv"
"strings" "strings"
@ -297,7 +298,6 @@ const (
ErrAdminConfigLDAPValidation ErrAdminConfigLDAPValidation
ErrAdminConfigIDPCfgNameAlreadyExists ErrAdminConfigIDPCfgNameAlreadyExists
ErrAdminConfigIDPCfgNameDoesNotExist ErrAdminConfigIDPCfgNameDoesNotExist
ErrAdminCredentialsMismatch
ErrInsecureClientRequest ErrInsecureClientRequest
ErrObjectTampered ErrObjectTampered
@ -1425,11 +1425,6 @@ var errorCodes = errorCodeMap{
Description: "Unable to perform the requested operation because profiling is not enabled", Description: "Unable to perform the requested operation because profiling is not enabled",
HTTPStatusCode: http.StatusBadRequest, HTTPStatusCode: http.StatusBadRequest,
}, },
ErrAdminCredentialsMismatch: {
Code: "XMinioAdminCredentialsMismatch",
Description: "Credentials in config mismatch with server environment variables",
HTTPStatusCode: http.StatusServiceUnavailable,
},
ErrAdminBucketQuotaExceeded: { ErrAdminBucketQuotaExceeded: {
Code: "XMinioAdminBucketQuotaExceeded", Code: "XMinioAdminBucketQuotaExceeded",
Description: "Bucket quota exceeded", Description: "Bucket quota exceeded",
@ -2082,6 +2077,11 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) {
return ErrNone return ErrNone
} }
// Errors that are generated by net.Conn and any context errors must be handled here.
if errors.Is(err, os.ErrDeadlineExceeded) || errors.Is(err, context.DeadlineExceeded) {
return ErrRequestTimedout
}
// Only return ErrClientDisconnected if the provided context is actually canceled. // Only return ErrClientDisconnected if the provided context is actually canceled.
// This way downstream context.Canceled will still report ErrRequestTimedout // This way downstream context.Canceled will still report ErrRequestTimedout
if contextCanceled(ctx) && errors.Is(ctx.Err(), context.Canceled) { if contextCanceled(ctx) && errors.Is(ctx.Err(), context.Canceled) {

View File

@ -946,11 +946,13 @@ func writeSuccessResponseHeadersOnly(w http.ResponseWriter) {
// writeErrorRespone writes error headers // writeErrorRespone writes error headers
func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError, reqURL *url.URL) { func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError, reqURL *url.URL) {
switch err.Code { if err.HTTPStatusCode == http.StatusServiceUnavailable {
case "SlowDown", "XMinioServerNotInitialized", "XMinioReadQuorum", "XMinioWriteQuorum":
// Set retry-after header to indicate user-agents to retry request after 120secs. // Set retry-after header to indicate user-agents to retry request after 120secs.
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
w.Header().Set(xhttp.RetryAfter, "120") w.Header().Set(xhttp.RetryAfter, "120")
}
switch err.Code {
case "InvalidRegion": case "InvalidRegion":
err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalSite.Region) err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalSite.Region)
case "AuthorizationHeaderMalformed": case "AuthorizationHeaderMalformed":

File diff suppressed because one or more lines are too long

View File

@ -394,6 +394,7 @@ func buildServerCtxt(ctx *cli.Context, ctxt *serverCtxt) (err error) {
ctxt.UserTimeout = ctx.Duration("conn-user-timeout") ctxt.UserTimeout = ctx.Duration("conn-user-timeout")
ctxt.ConnReadDeadline = ctx.Duration("conn-read-deadline") ctxt.ConnReadDeadline = ctx.Duration("conn-read-deadline")
ctxt.ConnWriteDeadline = ctx.Duration("conn-write-deadline") ctxt.ConnWriteDeadline = ctx.Duration("conn-write-deadline")
ctxt.ConnClientReadDeadline = ctx.Duration("conn-client-read-deadline")
ctxt.ShutdownTimeout = ctx.Duration("shutdown-timeout") ctxt.ShutdownTimeout = ctx.Duration("shutdown-timeout")
ctxt.IdleTimeout = ctx.Duration("idle-timeout") ctxt.IdleTimeout = ctx.Duration("idle-timeout")

View File

@ -160,9 +160,10 @@ type serverCtxt struct {
FTP []string FTP []string
SFTP []string SFTP []string
UserTimeout time.Duration UserTimeout time.Duration
ConnReadDeadline time.Duration ConnReadDeadline time.Duration
ConnWriteDeadline time.Duration ConnWriteDeadline time.Duration
ConnClientReadDeadline time.Duration
ShutdownTimeout time.Duration ShutdownTimeout time.Duration
IdleTimeout time.Duration IdleTimeout time.Duration

View File

@ -101,6 +101,12 @@ var ServerFlags = []cli.Flag{
EnvVar: "MINIO_READ_HEADER_TIMEOUT", EnvVar: "MINIO_READ_HEADER_TIMEOUT",
Hidden: true, Hidden: true,
}, },
cli.DurationFlag{
Name: "conn-client-read-deadline",
Usage: "custom connection READ deadline for incoming requests",
Hidden: true,
EnvVar: "MINIO_CONN_CLIENT_READ_DEADLINE",
},
cli.DurationFlag{ cli.DurationFlag{
Name: "conn-read-deadline", Name: "conn-read-deadline",
Usage: "custom connection READ deadline", Usage: "custom connection READ deadline",
@ -351,8 +357,9 @@ func serverHandleCmdArgs(ctxt serverCtxt) {
}) })
globalTCPOptions = xhttp.TCPOptions{ globalTCPOptions = xhttp.TCPOptions{
UserTimeout: int(ctxt.UserTimeout.Milliseconds()), UserTimeout: int(ctxt.UserTimeout.Milliseconds()),
Interface: ctxt.Interface, ClientReadTimeout: ctxt.ConnClientReadDeadline,
Interface: ctxt.Interface,
} }
// On macOS, if a process already listens on LOCALIPADDR:PORT, net.Listen() falls back // On macOS, if a process already listens on LOCALIPADDR:PORT, net.Listen() falls back

View File

@ -147,4 +147,9 @@ if [ $ret -ne 0 ]; then
exit 1 exit 1
fi fi
# kill $pid go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
kill $pid

View File

@ -158,4 +158,9 @@ if [ $ret -ne 0 ]; then
exit 1 exit 1
fi fi
go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
kill $pid kill $pid

View File

@ -144,4 +144,9 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
exit 1 exit 1
fi fi
go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
kill $pid kill $pid

View File

@ -210,4 +210,9 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
exit 1 exit 1
fi fi
go install github.com/minio/minio/docs/debugging/s3-check-md5@latest
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket bucket2
s3-check-md5 -versions -access-key minioadmin -secret-key minioadmin -endpoint http://127.0.0.1:9001/ -bucket versioned
kill $pid kill $pid

View File

@ -22,6 +22,9 @@ import (
"fmt" "fmt"
"net" "net"
"syscall" "syscall"
"time"
"github.com/minio/minio/internal/deadlineconn"
) )
type acceptResult struct { type acceptResult struct {
@ -32,6 +35,7 @@ type acceptResult struct {
// httpListener - HTTP listener capable of handling multiple server addresses. // httpListener - HTTP listener capable of handling multiple server addresses.
type httpListener struct { type httpListener struct {
opts TCPOptions
tcpListeners []*net.TCPListener // underlying TCP listeners. tcpListeners []*net.TCPListener // underlying TCP listeners.
acceptCh chan acceptResult // channel where all TCP listeners write accepted connection. acceptCh chan acceptResult // channel where all TCP listeners write accepted connection.
ctx context.Context ctx context.Context
@ -74,7 +78,8 @@ func (listener *httpListener) Accept() (conn net.Conn, err error) {
select { select {
case result, ok := <-listener.acceptCh: case result, ok := <-listener.acceptCh:
if ok { if ok {
return result.conn, result.err return deadlineconn.New(result.conn).
WithReadDeadline(listener.opts.ClientReadTimeout), result.err
} }
case <-listener.ctx.Done(): case <-listener.ctx.Done():
} }
@ -119,9 +124,10 @@ func (listener *httpListener) Addrs() (addrs []net.Addr) {
// TCPOptions specify customizable TCP optimizations on raw socket // TCPOptions specify customizable TCP optimizations on raw socket
type TCPOptions struct { type TCPOptions struct {
UserTimeout int // this value is expected to be in milliseconds UserTimeout int // this value is expected to be in milliseconds
Interface string // this is a VRF device passed via `--interface` flag ClientReadTimeout time.Duration // When the net.Conn is idle for more than ReadTimeout duration, we close the connection on the client proactively.
Trace func(msg string) // Trace when starting. Interface string // this is a VRF device passed via `--interface` flag
Trace func(msg string) // Trace when starting.
} }
// newHTTPListener - creates new httpListener object which is interface compatible to net.Listener. // newHTTPListener - creates new httpListener object which is interface compatible to net.Listener.
@ -173,6 +179,7 @@ func newHTTPListener(ctx context.Context, serverAddrs []string, opts TCPOptions)
listener = &httpListener{ listener = &httpListener{
tcpListeners: tcpListeners, tcpListeners: tcpListeners,
acceptCh: make(chan acceptResult, len(tcpListeners)), acceptCh: make(chan acceptResult, len(tcpListeners)),
opts: opts,
} }
listener.ctx, listener.ctxCanceler = context.WithCancel(ctx) listener.ctx, listener.ctxCanceler = context.WithCancel(ctx)
if opts.Trace != nil { if opts.Trace != nil {

View File

@ -109,8 +109,12 @@ func (srv *Server) Init(listenCtx context.Context, listenErrCallback func(listen
wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// If server is in shutdown. // If server is in shutdown.
if atomic.LoadUint32(&srv.inShutdown) != 0 { if atomic.LoadUint32(&srv.inShutdown) != 0 {
// To indicate disable keep-alive // To indicate disable keep-alive, server is shutting down.
w.Header().Set("Connection", "close") w.Header().Set("Connection", "close")
// Add 1 minute retry header, incase-client wants to honor it
w.Header().Set(RetryAfter, "60")
w.WriteHeader(http.StatusServiceUnavailable) w.WriteHeader(http.StatusServiceUnavailable)
w.Write([]byte(http.ErrServerClosed.Error())) w.Write([]byte(http.ErrServerClosed.Error()))
return return