From e79d63208e0b2b5452fec4d85ed1092489c848d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Thu, 16 May 2024 19:39:30 +0200 Subject: [PATCH] feat: limit concurrent processing of thumbnail requests --- .../unreleased/thumbnail-request-limit.md | 6 +++++ ocis-pkg/middleware/throttle.go | 19 ++++++++++++++++ services/thumbnails/README.md | 21 +++++++++--------- services/thumbnails/pkg/command/server.go | 1 + .../pkg/config/defaults/defaultconfig.go | 7 +++--- services/thumbnails/pkg/config/http.go | 9 ++++---- services/thumbnails/pkg/server/http/option.go | 22 +++++++++++++------ services/thumbnails/pkg/server/http/server.go | 1 + .../thumbnails/pkg/service/http/v0/service.go | 5 ++--- services/webdav/pkg/service/v0/search.go | 2 +- services/webdav/pkg/service/v0/service.go | 21 ++++++++---------- 11 files changed, 74 insertions(+), 40 deletions(-) create mode 100644 changelog/unreleased/thumbnail-request-limit.md create mode 100644 ocis-pkg/middleware/throttle.go diff --git a/changelog/unreleased/thumbnail-request-limit.md b/changelog/unreleased/thumbnail-request-limit.md new file mode 100644 index 00000000000..8c1ea0f85d7 --- /dev/null +++ b/changelog/unreleased/thumbnail-request-limit.md @@ -0,0 +1,6 @@ +Enhancement: Limit concurrent thumbnail requests + +The number of concurrent requests to the thumbnail service can be limited now +to have more control over the consumed system resources. + +https://github.com/owncloud/ocis/pull/9199 diff --git a/ocis-pkg/middleware/throttle.go b/ocis-pkg/middleware/throttle.go new file mode 100644 index 00000000000..2e331b9b05b --- /dev/null +++ b/ocis-pkg/middleware/throttle.go @@ -0,0 +1,19 @@ +package middleware + +import ( + "net/http" + + "github.com/go-chi/chi/v5/middleware" +) + +// Throttle limits the number of concurrent requests. +func Throttle(limit int) func(http.Handler) http.Handler { + if limit > 0 { + return middleware.Throttle(limit) + } + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + next.ServeHTTP(w, r) + }) + } +} diff --git a/services/thumbnails/README.md b/services/thumbnails/README.md index 47a9939e39f..813b5cc16d0 100644 --- a/services/thumbnails/README.md +++ b/services/thumbnails/README.md @@ -41,19 +41,19 @@ Thumbnails can either be generated as `png`, `jpg` or `gif` files. These types a ## Thumbnail Query String Parameters -Clients can request thumbnail previews for files by adding `?preview=1` to the file URL. Requests for files with thumbnail availabe respond with HTTP status `404`. +Clients can request thumbnail previews for files by adding `?preview=1` to the file URL. Requests for files with thumbnail available respond with HTTP status `404`. The following query parameters are supported: | Parameter | Required | Default Value | Description | -| --------- | -------- | ---------------------------------------------------- | ------------------------------------------------------------------------------- | +|-----------|----------|------------------------------------------------------|---------------------------------------------------------------------------------| | preview | YES | 1 | generates preview | | x | YES | first x-value configured in `THUMBNAILS_RESOLUTIONS` | horizontal target size | | y | YES | first y-value configured in `THUMBNAILS_RESOLUTIONS` | vertical target size | -| scalingup | NO | 0 | prevents upscaling of small images | +| scalingup | NO | 0 | prevents up-scaling of small images | | a | NO | 1 | aspect ratio | | c | NO | Caching string | Clients should send the etag, so they get a fresh thumbnail after a file change | -| processor | NO | `resize` for gif's and `thumbnail` for all others | preferred thumbnail processor | +| processor | NO | `resize` for gifs and `thumbnail` for all others | preferred thumbnail processor | ## Thumbnail Resolution @@ -61,21 +61,21 @@ Various resolutions can be defined via `THUMBNAILS_RESOLUTIONS`. A requestor can Example: -Requested: 18x12 -Available: 30x20, 15x10, 9x6 -Returned: 15x10 +Requested: 18x12 +Available: 30x20, 15x10, 9x6 +Returned: 15x10 ## Thumbnail Processors -Normally, an image might get cropped when creating a preview, depending on the aspect ratio of the original image. This can have negative +Normally, an image might get cropped when creating a preview, depending on the aspect ratio of the original image. This can have negative impacts on previews as only a part of the image will be shown. When using an _optional_ processor in the request, cropping can be avoided by defining on how the preview image generation will be done. The following processors are available: * `resize` resizes the image to the specified width and height and returns the transformed image. If one of width or height is 0, the image aspect ratio is preserved. * `fit` scales down the image to fit the specified maximum width and height and returns the transformed image. * `fill`: creates an image with the specified dimensions and fills it with the scaled source image. To achieve the correct aspect ratio without stretching, the source image will be cropped. -* `thumbnail` scales the image up or down, crops it to the specified width and hight and returns the transformed image. +* `thumbnail` scales the image up or down, crops it to the specified width and height and returns the transformed image. -To apply one of those, a query parameter has to be added to the request, like `?processor=fit`. If no query parameter or processor is added, the default behaviour applies which is `resize` for gif's and `thumbnail` for all others. +To apply one of those, a query parameter has to be added to the request, like `?processor=fit`. If no query parameter or processor is added, the default behaviour applies which is `resize` for gifs and `thumbnail` for all others. ## Deleting Thumbnails @@ -84,3 +84,4 @@ As of now, there is no automated thumbnail deletion. This is especially true whe ## Memory Considerations Since source files need to be loaded into memory when generating thumbnails, large source files could potentially crash this service if there is insufficient memory available. For bigger instances when using container orchestration deployment methods, this service can be dedicated to its own server(s) with more memory. +To have more control over memory (and CPU) consumption the maximum number of concurrent requests can be limited by setting the environment variable `THUMBNAILS_MAX_CONCURRENT_REQUESTS`. The default value is 0 which does not apply any restrictions to the number of concurrent requests. As soon as the number of concurrent requests is reached any further request will be responded with `429/Too Many Requests` and the client can retry at a later point in time. diff --git a/services/thumbnails/pkg/command/server.go b/services/thumbnails/pkg/command/server.go index d62095dd28e..ba869dae9de 100644 --- a/services/thumbnails/pkg/command/server.go +++ b/services/thumbnails/pkg/command/server.go @@ -98,6 +98,7 @@ func Server(cfg *config.Config) *cli.Command { http.Metrics(m), http.Namespace(cfg.HTTP.Namespace), http.TraceProvider(traceProvider), + http.MaxConcurrentRequests(cfg.HTTP.MaxConcurrentRequests), ) if err != nil { logger.Info(). diff --git a/services/thumbnails/pkg/config/defaults/defaultconfig.go b/services/thumbnails/pkg/config/defaults/defaultconfig.go index 81d70775a27..be5453e0de2 100644 --- a/services/thumbnails/pkg/config/defaults/defaultconfig.go +++ b/services/thumbnails/pkg/config/defaults/defaultconfig.go @@ -32,9 +32,10 @@ func DefaultConfig() *config.Config { Namespace: "com.owncloud.api", }, HTTP: config.HTTP{ - Addr: "127.0.0.1:9186", - Root: "/thumbnails", - Namespace: "com.owncloud.web", + Addr: "127.0.0.1:9186", + Root: "/thumbnails", + Namespace: "com.owncloud.web", + MaxConcurrentRequests: 0, }, Service: config.Service{ Name: "thumbnails", diff --git a/services/thumbnails/pkg/config/http.go b/services/thumbnails/pkg/config/http.go index 45b6ff6de67..1e779d2e725 100644 --- a/services/thumbnails/pkg/config/http.go +++ b/services/thumbnails/pkg/config/http.go @@ -4,8 +4,9 @@ import "github.com/owncloud/ocis/v2/ocis-pkg/shared" // HTTP defines the available http configuration. type HTTP struct { - Addr string `yaml:"addr" env:"THUMBNAILS_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"pre5.0"` - TLS shared.HTTPServiceTLS `yaml:"tls"` - Root string `yaml:"root" env:"THUMBNAILS_HTTP_ROOT" desc:"Subdirectory that serves as the root for this HTTP service." introductionVersion:"pre5.0"` - Namespace string `yaml:"-"` + Addr string `yaml:"addr" env:"THUMBNAILS_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"pre5.0"` + TLS shared.HTTPServiceTLS `yaml:"tls"` + Root string `yaml:"root" env:"THUMBNAILS_HTTP_ROOT" desc:"Subdirectory that serves as the root for this HTTP service." introductionVersion:"pre5.0"` + Namespace string `yaml:"-"` + MaxConcurrentRequests int `yaml:"max_concurrent_requests" env:"THUMBNAILS_MAX_CONCURRENT_REQUESTS" desc:"Number of maximum concurrent thumbnail requests. Default is 0 which is unlimited." introductionVersion:"6.0"` } diff --git a/services/thumbnails/pkg/server/http/option.go b/services/thumbnails/pkg/server/http/option.go index 6bbf4627be6..c94fdc3dfb6 100644 --- a/services/thumbnails/pkg/server/http/option.go +++ b/services/thumbnails/pkg/server/http/option.go @@ -16,13 +16,14 @@ type Option func(o *Options) // Options defines the available options for this package. type Options struct { - Namespace string - Logger log.Logger - Context context.Context - Config *config.Config - Metrics *metrics.Metrics - Flags []cli.Flag - TraceProvider trace.TracerProvider + Namespace string + Logger log.Logger + Context context.Context + Config *config.Config + Metrics *metrics.Metrics + Flags []cli.Flag + TraceProvider trace.TracerProvider + MaxConcurrentRequests int } // newOptions initializes the available default options. @@ -81,3 +82,10 @@ func TraceProvider(traceProvider trace.TracerProvider) Option { } } } + +// MaxConcurrentRequests provides a function to set the MaxConcurrentRequests option. +func MaxConcurrentRequests(val int) Option { + return func(o *Options) { + o.MaxConcurrentRequests = val + } +} diff --git a/services/thumbnails/pkg/server/http/server.go b/services/thumbnails/pkg/server/http/server.go index 660e85d5d66..ad160b848c2 100644 --- a/services/thumbnails/pkg/server/http/server.go +++ b/services/thumbnails/pkg/server/http/server.go @@ -39,6 +39,7 @@ func Server(opts ...Option) (http.Service, error) { svc.Middleware( middleware.RealIP, middleware.RequestID, + ocismiddleware.Throttle(options.MaxConcurrentRequests), ocismiddleware.Version( options.Config.Service.Name, version.GetString(), diff --git a/services/thumbnails/pkg/service/http/v0/service.go b/services/thumbnails/pkg/service/http/v0/service.go index 7207507a7e7..175b24230c7 100644 --- a/services/thumbnails/pkg/service/http/v0/service.go +++ b/services/thumbnails/pkg/service/http/v0/service.go @@ -3,12 +3,11 @@ package svc import ( "context" "fmt" - "net/http" - "strconv" - "github.com/go-chi/chi/v5" "github.com/golang-jwt/jwt/v4" "github.com/riandyrn/otelchi" + "net/http" + "strconv" "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/ocis-pkg/tracing" diff --git a/services/webdav/pkg/service/v0/search.go b/services/webdav/pkg/service/v0/search.go index a5818f88db5..3ad3184e444 100644 --- a/services/webdav/pkg/service/v0/search.go +++ b/services/webdav/pkg/service/v0/search.go @@ -47,7 +47,7 @@ func (g Webdav) Search(w http.ResponseWriter, r *http.Request) { return } - t := r.Header.Get(TokenHeader) + t := r.Header.Get(revactx.TokenHeader) ctx := revactx.ContextSetToken(r.Context(), t) ctx = metadata.Set(ctx, revactx.TokenHeader, t) diff --git a/services/webdav/pkg/service/v0/service.go b/services/webdav/pkg/service/v0/service.go index cbe3a74f5ab..ad914752725 100644 --- a/services/webdav/pkg/service/v0/service.go +++ b/services/webdav/pkg/service/v0/service.go @@ -13,6 +13,7 @@ import ( gatewayv1beta1 "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" rpcv1beta1 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" + revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/v2/pkg/storage/utils/templates" "github.com/go-chi/chi/v5" @@ -37,10 +38,6 @@ func init() { chi.RegisterMethod("REPORT") } -const ( - TokenHeader = "X-Access-Token" -) - var ( codesEnum = map[int]string{ http.StatusBadRequest: "Sabre\\DAV\\Exception\\BadRequest", @@ -52,8 +49,8 @@ var ( // Service defines the extension handlers. type Service interface { - ServeHTTP(http.ResponseWriter, *http.Request) - Thumbnail(http.ResponseWriter, *http.Request) + ServeHTTP(w http.ResponseWriter, r *http.Request) + Thumbnail(w http.ResponseWriter, r *http.Request) } // NewService returns a service implementation for Service. @@ -235,7 +232,7 @@ func (g Webdav) SpacesThumbnail(w http.ResponseWriter, r *http.Request) { renderError(w, r, errBadRequest(err.Error())) return } - t := r.Header.Get(TokenHeader) + t := r.Header.Get(revactx.TokenHeader) fullPath := filepath.Join(tr.Identifier, tr.Filepath) rsp, err := g.thumbnailsClient.GetThumbnail(r.Context(), &thumbnailssvc.GetThumbnailRequest{ @@ -284,7 +281,7 @@ func (g Webdav) Thumbnail(w http.ResponseWriter, r *http.Request) { return } - t := r.Header.Get(TokenHeader) + t := r.Header.Get(revactx.TokenHeader) gatewayClient, err := g.gatewaySelector.Next() if err != nil { @@ -312,7 +309,7 @@ func (g Webdav) Thumbnail(w http.ResponseWriter, r *http.Request) { user = userRes.GetUser() } else { // look up user from URL via GetUserByClaim - ctx := grpcmetadata.AppendToOutgoingContext(r.Context(), TokenHeader, t) + ctx := grpcmetadata.AppendToOutgoingContext(r.Context(), revactx.TokenHeader, t) userRes, err := gatewayClient.GetUserByClaim(ctx, &userv1beta1.GetUserByClaimRequest{ Claim: "username", Value: tr.Identifier, @@ -475,11 +472,11 @@ func (g Webdav) sendThumbnailResponse(rsp *thumbnailssvc.GetThumbnailResponse, w if dlRsp.StatusCode != http.StatusOK { logger.Debug(). - Str("transfer_token", rsp.TransferToken). - Str("data_endpoint", rsp.DataEndpoint). + Str("transfer_token", rsp.GetTransferToken()). + Str("data_endpoint", rsp.GetDataEndpoint()). Str("response_status", dlRsp.Status). Msg("could not download thumbnail") - renderError(w, r, errInternalError("could not download thumbnail")) + renderError(w, r, newErrResponse(dlRsp.StatusCode, "could not download thumbnail")) return }