introduce reader deadlines for net.Conn (minio#19023)

Bonus: set "retry-after" header for AWS SDKs if possible to honor them.

Author: harshavardhana

cmd/api-errors.go

@@ -24,6 +24,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"os"
 	"strconv"
 	"strings"
 
@@ -297,7 +298,6 @@ const (
 	ErrAdminConfigLDAPValidation
 	ErrAdminConfigIDPCfgNameAlreadyExists
 	ErrAdminConfigIDPCfgNameDoesNotExist
-	ErrAdminCredentialsMismatch
 	ErrInsecureClientRequest
 	ErrObjectTampered
 
@@ -1425,11 +1425,6 @@ var errorCodes = errorCodeMap{
 		Description:    "Unable to perform the requested operation because profiling is not enabled",
 		HTTPStatusCode: http.StatusBadRequest,
 	},
-	ErrAdminCredentialsMismatch: {
-		Code:           "XMinioAdminCredentialsMismatch",
-		Description:    "Credentials in config mismatch with server environment variables",
-		HTTPStatusCode: http.StatusServiceUnavailable,
-	},
 	ErrAdminBucketQuotaExceeded: {
 		Code:           "XMinioAdminBucketQuotaExceeded",
 		Description:    "Bucket quota exceeded",
@@ -2082,6 +2077,11 @@ func toAPIErrorCode(ctx context.Context, err error) (apiErr APIErrorCode) {
 		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.
 	// This way downstream context.Canceled will still report ErrRequestTimedout
 	if contextCanceled(ctx) && errors.Is(ctx.Err(), context.Canceled) {

cmd/api-response.go

@@ -946,11 +946,13 @@ func writeSuccessResponseHeadersOnly(w http.ResponseWriter) {
 
 // writeErrorRespone writes error headers
 func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError, reqURL *url.URL) {
-	switch err.Code {
-	case "SlowDown", "XMinioServerNotInitialized", "XMinioReadQuorum", "XMinioWriteQuorum":
+	if err.HTTPStatusCode == http.StatusServiceUnavailable {
 		// 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
 		w.Header().Set(xhttp.RetryAfter, "120")
+	}
+
+	switch err.Code {
 	case "InvalidRegion":
 		err.Description = fmt.Sprintf("Region does not match; expecting '%s'.", globalSite.Region)
 	case "AuthorizationHeaderMalformed":

cmd/apierrorcode_string.go

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

cmd/common-main.go

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

cmd/globals.go

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

cmd/server-main.go

@@ -147,4 +147,9 @@ if [ $ret -ne 0 ]; then
 	exit 1
 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

docs/distributed/decom-compressed-sse-s3.sh

@@ -158,4 +158,9 @@ if [ $ret -ne 0 ]; then
 	exit 1
 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

docs/distributed/decom-encrypted-sse-s3.sh

@@ -144,4 +144,9 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
 	exit 1
 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

docs/distributed/decom-encrypted.sh

@@ -210,4 +210,9 @@ if [ "${expected_checksum}" != "${got_checksum}" ]; then
 	exit 1
 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

docs/distributed/decom.sh

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

internal/http/listener.go

@@ -109,8 +109,12 @@ func (srv *Server) Init(listenCtx context.Context, listenErrCallback func(listen
 	wrappedHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// If server is in shutdown.
 		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")
+
+			// Add 1 minute retry header, incase-client wants to honor it
+			w.Header().Set(RetryAfter, "60")
+
 			w.WriteHeader(http.StatusServiceUnavailable)
 			w.Write([]byte(http.ErrServerClosed.Error()))
 			return