From a5702f978e214903428152b7e798a7c044678c6c Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 16 Aug 2024 01:43:49 -0700 Subject: [PATCH] remove requests deadline, instead just reject the requests (#20272) Additionally set - x-ratelimit-limit - x-ratelimit-remaining To indicate the request rates. --- cmd/api-errors.go | 2 +- cmd/api-response.go | 11 +--------- cmd/handler-api.go | 41 +++++++++++++------------------------ docs/config/README.md | 18 ++++++++-------- docs/throttle/README.md | 22 -------------------- internal/config/api/api.go | 14 ++----------- internal/config/api/help.go | 8 +------- 7 files changed, 29 insertions(+), 87 deletions(-) diff --git a/cmd/api-errors.go b/cmd/api-errors.go index 1c5ec4c30..c828a43b0 100644 --- a/cmd/api-errors.go +++ b/cmd/api-errors.go @@ -1486,7 +1486,7 @@ var errorCodes = errorCodeMap{ }, ErrTooManyRequests: { Code: "TooManyRequests", - Description: "Deadline exceeded while waiting in incoming queue, please reduce your request rate", + Description: "Please reduce your request rate", HTTPStatusCode: http.StatusTooManyRequests, }, ErrUnsupportedMetadata: { diff --git a/cmd/api-response.go b/cmd/api-response.go index e9d68aba5..5477c9a65 100644 --- a/cmd/api-response.go +++ b/cmd/api-response.go @@ -947,19 +947,10 @@ func writeSuccessResponseHeadersOnly(w http.ResponseWriter) { // writeErrorResponse writes error headers func writeErrorResponse(ctx context.Context, w http.ResponseWriter, err APIError, reqURL *url.URL) { switch err.HTTPStatusCode { - case http.StatusServiceUnavailable: + case http.StatusServiceUnavailable, http.StatusTooManyRequests: // Set retry-after header to indicate user-agents to retry request after 60 seconds. // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After w.Header().Set(xhttp.RetryAfter, "60") - case http.StatusTooManyRequests: - _, deadline := globalAPIConfig.getRequestsPool() - if deadline <= 0 { - // Set retry-after header to indicate user-agents to retry request after 10 seconds. - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After - w.Header().Set(xhttp.RetryAfter, "10") - } else { - w.Header().Set(xhttp.RetryAfter, strconv.Itoa(int(deadline.Seconds()))) - } } switch err.Code { diff --git a/cmd/handler-api.go b/cmd/handler-api.go index 8f6eb3172..c0f1d6800 100644 --- a/cmd/handler-api.go +++ b/cmd/handler-api.go @@ -39,7 +39,6 @@ import ( type apiConfig struct { mu sync.RWMutex - requestsDeadline time.Duration requestsPool chan struct{} clusterDeadline time.Duration listQuorum string @@ -164,7 +163,6 @@ func (t *apiConfig) init(cfg api.Config, setDriveCounts []int, legacy bool) { // but this shouldn't last long. t.requestsPool = make(chan struct{}, apiRequestsMaxPerNode) } - t.requestsDeadline = cfg.RequestsDeadline listQuorum := cfg.ListQuorum if listQuorum == "" { listQuorum = "strict" @@ -290,19 +288,15 @@ func (t *apiConfig) getRequestsPoolCapacity() int { return cap(t.requestsPool) } -func (t *apiConfig) getRequestsPool() (chan struct{}, time.Duration) { +func (t *apiConfig) getRequestsPool() chan struct{} { t.mu.RLock() defer t.mu.RUnlock() if t.requestsPool == nil { - return nil, 10 * time.Second + return nil } - if t.requestsDeadline <= 0 { - return t.requestsPool, 10 * time.Second - } - - return t.requestsPool, t.requestsDeadline + return t.requestsPool } // maxClients throttles the S3 API calls @@ -325,27 +319,19 @@ func maxClients(f http.HandlerFunc) http.HandlerFunc { } globalHTTPStats.addRequestsInQueue(1) - pool, deadline := globalAPIConfig.getRequestsPool() + pool := globalAPIConfig.getRequestsPool() if pool == nil { globalHTTPStats.addRequestsInQueue(-1) f.ServeHTTP(w, r) return } - // No deadline to wait, there is nothing to queue - // perform the API call immediately. - if deadline <= 0 { - globalHTTPStats.addRequestsInQueue(-1) - f.ServeHTTP(w, r) - return - } - if tc, ok := r.Context().Value(mcontext.ContextTraceKey).(*mcontext.TraceCtxt); ok { tc.FuncName = "s3.MaxClients" } - deadlineTimer := time.NewTimer(deadline) - defer deadlineTimer.Stop() + w.Header().Set("X-RateLimit-Limit", strconv.Itoa(cap(pool))) + w.Header().Set("X-RateLimit-Remaining", strconv.Itoa(cap(pool)-len(pool))) ctx := r.Context() select { @@ -357,7 +343,13 @@ func maxClients(f http.HandlerFunc) http.HandlerFunc { return } f.ServeHTTP(w, r) - case <-deadlineTimer.C: + case <-r.Context().Done(): + globalHTTPStats.addRequestsInQueue(-1) + // When the client disconnects before getting the S3 handler + // status code response, set the status code to 499 so this request + // will be properly audited and traced. + w.WriteHeader(499) + default: globalHTTPStats.addRequestsInQueue(-1) if contextCanceled(ctx) { w.WriteHeader(499) @@ -367,12 +359,7 @@ func maxClients(f http.HandlerFunc) http.HandlerFunc { writeErrorResponse(ctx, w, errorCodes.ToAPIErr(ErrTooManyRequests), r.URL) - case <-r.Context().Done(): - globalHTTPStats.addRequestsInQueue(-1) - // When the client disconnects before getting the S3 handler - // status code response, set the status code to 499 so this request - // will be properly audited and traced. - w.WriteHeader(499) + } } } diff --git a/docs/config/README.md b/docs/config/README.md index 5336b1fe5..c2f2ebed8 100644 --- a/docs/config/README.md +++ b/docs/config/README.md @@ -132,39 +132,41 @@ KEY: api manage global HTTP API call specific features, such as throttling, authentication types, etc. ARGS: -requests_max (number) set the maximum number of concurrent requests (default: '0') -requests_deadline (duration) set the deadline for API requests waiting to be processed (default: '10s') +requests_max (number) set the maximum number of concurrent requests (default: 'auto') cluster_deadline (duration) set the deadline for cluster readiness check (default: '10s') cors_allow_origin (csv) set comma separated list of origins allowed for CORS requests (default: '*') remote_transport_deadline (duration) set the deadline for API requests on remote transports while proxying between federated instances e.g. "2h" (default: '2h') -list_quorum (string) set the acceptable quorum expected for list operations e.g. "optimal", "reduced", "disk", "strict" (default: 'strict') +list_quorum (string) set the acceptable quorum expected for list operations e.g. "optimal", "reduced", "disk", "strict", "auto" (default: 'strict') replication_priority (string) set replication priority (default: 'auto') +replication_max_workers (number) set the maximum number of replication workers (default: '500') transition_workers (number) set the number of transition workers (default: '100') stale_uploads_expiry (duration) set to expire stale multipart uploads older than this values (default: '24h') stale_uploads_cleanup_interval (duration) set to change intervals when stale multipart uploads are expired (default: '6h') delete_cleanup_interval (duration) set to change intervals when deleted objects are permanently deleted from ".trash" folder (default: '5m') -odirect (boolean) set to enable or disable O_DIRECT for read and writes under special conditions. NOTE: do not disable O_DIRECT without prior testing (default: 'on') +odirect (boolean) set to enable or disable O_DIRECT for writes under special conditions. NOTE: do not disable O_DIRECT without prior testing (default: 'on') root_access (boolean) turn 'off' root credential access for all API calls including s3, admin operations (default: 'on') sync_events (boolean) set to enable synchronous bucket notifications (default: 'off') +object_max_versions (number) set max allowed number of versions per object (default: '9223372036854775807') ``` or environment variables ``` -MINIO_API_REQUESTS_MAX (number) set the maximum number of concurrent requests (default: '0') -MINIO_API_REQUESTS_DEADLINE (duration) set the deadline for API requests waiting to be processed (default: '10s') +MINIO_API_REQUESTS_MAX (number) set the maximum number of concurrent requests (default: 'auto') MINIO_API_CLUSTER_DEADLINE (duration) set the deadline for cluster readiness check (default: '10s') MINIO_API_CORS_ALLOW_ORIGIN (csv) set comma separated list of origins allowed for CORS requests (default: '*') MINIO_API_REMOTE_TRANSPORT_DEADLINE (duration) set the deadline for API requests on remote transports while proxying between federated instances e.g. "2h" (default: '2h') -MINIO_API_LIST_QUORUM (string) set the acceptable quorum expected for list operations e.g. "optimal", "reduced", "disk", "strict" (default: 'strict') +MINIO_API_LIST_QUORUM (string) set the acceptable quorum expected for list operations e.g. "optimal", "reduced", "disk", "strict", "auto" (default: 'strict') MINIO_API_REPLICATION_PRIORITY (string) set replication priority (default: 'auto') +MINIO_API_REPLICATION_MAX_WORKERS (number) set the maximum number of replication workers (default: '500') MINIO_API_TRANSITION_WORKERS (number) set the number of transition workers (default: '100') MINIO_API_STALE_UPLOADS_EXPIRY (duration) set to expire stale multipart uploads older than this values (default: '24h') MINIO_API_STALE_UPLOADS_CLEANUP_INTERVAL (duration) set to change intervals when stale multipart uploads are expired (default: '6h') MINIO_API_DELETE_CLEANUP_INTERVAL (duration) set to change intervals when deleted objects are permanently deleted from ".trash" folder (default: '5m') -MINIO_API_ODIRECT (boolean) set to enable or disable O_DIRECT for read and writes under special conditions. NOTE: do not disable O_DIRECT without prior testing (default: 'on') +MINIO_API_ODIRECT (boolean) set to enable or disable O_DIRECT for writes under special conditions. NOTE: do not disable O_DIRECT without prior testing (default: 'on') MINIO_API_ROOT_ACCESS (boolean) turn 'off' root credential access for all API calls including s3, admin operations (default: 'on') MINIO_API_SYNC_EVENTS (boolean) set to enable synchronous bucket notifications (default: 'off') +MINIO_API_OBJECT_MAX_VERSIONS (number) set max allowed number of versions per object (default: '9223372036854775807') ``` #### Notifications diff --git a/docs/throttle/README.md b/docs/throttle/README.md index 82d399347..b49c0f7b7 100644 --- a/docs/throttle/README.md +++ b/docs/throttle/README.md @@ -31,25 +31,3 @@ mc admin service restart myminio/ > NOTE: A zero value of `requests_max` means MinIO will automatically calculate requests based on available RAM size and that is the default behavior. -### Configuring connection (wait) deadline - -This value works in conjunction with max connection setting, setting this value allows for long waiting requests to quickly time out when there is no slot available to perform the request. - -This will reduce the pileup of waiting requests when clients are not configured with timeouts. Default wait time is *10 seconds* if *MINIO_API_REQUESTS_MAX* is enabled. This may need to be tuned to your application needs. - -Example: Limit a MinIO cluster to accept at max 1600 simultaneous S3 API requests across 8 servers, and set the wait deadline of *2 minutes* per API operation. - -```sh -export MINIO_API_REQUESTS_MAX=1600 -export MINIO_API_REQUESTS_DEADLINE=2m -export MINIO_ROOT_USER=your-access-key -export MINIO_ROOT_PASSWORD=your-secret-key -minio server http://server{1...8}/mnt/hdd{1...16} -``` - -or - -```sh -mc admin config set myminio/ api requests_max=1600 requests_deadline=2m -mc admin service restart myminio/ -``` diff --git a/internal/config/api/api.go b/internal/config/api/api.go index d7f493bb0..d3ab6a1fb 100644 --- a/internal/config/api/api.go +++ b/internal/config/api/api.go @@ -33,7 +33,6 @@ import ( // API sub-system constants const ( apiRequestsMax = "requests_max" - apiRequestsDeadline = "requests_deadline" apiClusterDeadline = "cluster_deadline" apiCorsAllowOrigin = "cors_allow_origin" apiRemoteTransportDeadline = "remote_transport_deadline" @@ -81,6 +80,7 @@ const ( // Deprecated key and ENVs const ( apiReadyDeadline = "ready_deadline" + apiRequestsDeadline = "requests_deadline" apiReplicationWorkers = "replication_workers" apiReplicationFailedWorkers = "replication_failed_workers" ) @@ -92,10 +92,6 @@ var ( Key: apiRequestsMax, Value: "0", }, - config.KV{ - Key: apiRequestsDeadline, - Value: "10s", - }, config.KV{ Key: apiClusterDeadline, Value: "10s", @@ -171,7 +167,6 @@ var ( // Config storage class configuration type Config struct { RequestsMax int `json:"requests_max"` - RequestsDeadline time.Duration `json:"requests_deadline"` ClusterDeadline time.Duration `json:"cluster_deadline"` CorsAllowOrigin []string `json:"cors_allow_origin"` RemoteTransportDeadline time.Duration `json:"remote_transport_deadline"` @@ -205,6 +200,7 @@ func (sCfg *Config) UnmarshalJSON(data []byte) error { func LookupConfig(kvs config.KVS) (cfg Config, err error) { deprecatedKeys := []string{ apiReadyDeadline, + apiRequestsDeadline, "extend_list_cache_life", apiReplicationWorkers, apiReplicationFailedWorkers, @@ -251,12 +247,6 @@ func LookupConfig(kvs config.KVS) (cfg Config, err error) { return cfg, errors.New("invalid API max requests value") } - requestsDeadline, err := time.ParseDuration(env.Get(EnvAPIRequestsDeadline, kvs.GetWithDefault(apiRequestsDeadline, DefaultKVS))) - if err != nil { - return cfg, err - } - cfg.RequestsDeadline = requestsDeadline - clusterDeadline, err := time.ParseDuration(env.Get(EnvAPIClusterDeadline, kvs.GetWithDefault(apiClusterDeadline, DefaultKVS))) if err != nil { return cfg, err diff --git a/internal/config/api/help.go b/internal/config/api/help.go index 18c20e13d..2c4b8b2bb 100644 --- a/internal/config/api/help.go +++ b/internal/config/api/help.go @@ -28,16 +28,10 @@ var ( Help = config.HelpKVS{ config.HelpKV{ Key: apiRequestsMax, - Description: `set the maximum number of concurrent requests` + defaultHelpPostfix(apiRequestsMax), + Description: `set the maximum number of concurrent requests (default: auto)`, Optional: true, Type: "number", }, - config.HelpKV{ - Key: apiRequestsDeadline, - Description: `set the deadline for API requests waiting to be processed` + defaultHelpPostfix(apiRequestsDeadline), - Optional: true, - Type: "duration", - }, config.HelpKV{ Key: apiClusterDeadline, Description: `set the deadline for cluster readiness check` + defaultHelpPostfix(apiClusterDeadline),