diff --git a/README.md b/README.md index 5fb6a49..f4f7ae4 100644 --- a/README.md +++ b/README.md @@ -126,6 +126,17 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it; default 75% of (free space + what the cache holds) - the rest are in the table under Configuration below +- **Memory:** pixa can use up to `max_concurrent_processing` times 1.6 GiB for + the images it processes at once, plus `upstream_connections` times + `upstream_max_response_size` for the source images it has fetched that wait + for processing. 1.6 GiB is the most pixad's resident memory was measured to + grow while it processed one 8192x8192 source, the largest it accepts, into + each output format in turn, one at a time; it grew most for a 40 MB JPEG of + random pixels saved as AVIF. At the defaults that is half the CPUs pixa sees + times 1.6 GiB, plus 64 times 50 MiB: on a host where pixa sees 48 CPUs, 24 + times 1.6 GiB plus 3.1 GiB, about 42 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING` + and `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may + use. - **Health check:** the image's `HEALTHCHECK` requests `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds after a deploy and marks the deploy failed unless it is `healthy`. The probe @@ -515,7 +526,7 @@ tell. With no file, pixa uses the environment and the defaults. | `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` | | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` | | `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` | -| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default the number of CPUs | +| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default half the CPUs, at least 1 | | `PIXA_IMAGE_REQUESTS_PER_MINUTE` | `image_requests_per_minute` | Image requests per client per minute; `0` turns it off; default `600` | | `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` | | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB | @@ -601,10 +612,12 @@ Key settings in more detail: does, and `downstream_timeout` has not ended first, the request is answered 503 with the error `server busy, try again later` - `max_concurrent_processing` — the most images decoded and encoded at once; - default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a - container's CPU limit. A request that finds all of them in use waits up to 10 - seconds for one to free up; if none does, and `downstream_timeout` has not - ended first, it is answered 503 the same way + default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's + CPU limit), rounded down, and at least 1. A value that is not a whole number + of at least 1 aborts startup. A request that finds all of them in use waits up + to 10 seconds for one to free up; if none does, and `downstream_timeout` has + not ended first, it is answered 503 the same way. Each one can take a lot of + memory: see "Running under upaas" - `image_requests_per_minute` — how many requests one client may send to the image routes per minute (see the limit on them under Routes); default `600`. `0` turns the limit off, and a negative or non-integer value aborts startup. diff --git a/TODO.md b/TODO.md index 7793ef4..56c6740 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,16 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees, + rounded down, and at least 1 (closes #242): it defaulted to one image per CPU, + and a container with no CPU limit sees every CPU of the host, so on a 48-CPU + host pixa processed 48 images at once, each of which can take about 1.6 GiB. + `imageprocessor.DefaultMaxConcurrentProcessing` gives the default to the + config and to `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still + overrides it. "Running under upaas" in `README.md` gives the worst-case + memory, measured, and tells the operator to set + `PIXA_MAX_CONCURRENT_PROCESSING` and `PIXA_UPSTREAM_CONNECTIONS` to fit the + container's memory. - 2026-10-15 a `:` in the source path or query no longer moves where the path ends and the query begins (closes #247): the signed data and the cache key joined host, path and query with `:` and no escaping, so a URL signed for diff --git a/configs/config.example.yml b/configs/config.example.yml index cdf1022..6a0ab17 100644 --- a/configs/config.example.yml +++ b/configs/config.example.yml @@ -108,10 +108,11 @@ upstream_connections_per_host: 20 # downstream_timeout has ended first. upstream_connections: 64 -# Maximum number of images decoded and encoded at once (default: the -# number of CPUs pixa can use, which follows a container's CPU limit). A -# request that finds none free waits up to 10 seconds for one, and if none -# frees up it is answered 503, unless downstream_timeout has ended first. +# Maximum number of images decoded and encoded at once (default: half the +# number of CPUs pixa can use, at least 1; the number of CPUs follows a +# container's CPU limit). A request that finds none free waits up to 10 +# seconds for one, and if none frees up it is answered 503, unless +# downstream_timeout has ended first. # max_concurrent_processing: 4 # How many requests to the image routes (/v1/image/ and /v1/e/) one client diff --git a/internal/config/concurrency_limits_internal_test.go b/internal/config/concurrency_limits_internal_test.go index 33634a4..395264b 100644 --- a/internal/config/concurrency_limits_internal_test.go +++ b/internal/config/concurrency_limits_internal_test.go @@ -13,8 +13,8 @@ const ( ) // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted -// max_concurrent_processing is the number of CPUs Go uses and an omitted -// upstream_connections is 64. +// max_concurrent_processing is half the CPUs Go uses, and at least 1, and +// an omitted upstream_connections is 64. func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) { t.Parallel() @@ -23,9 +23,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) { t.Fatalf("minimal config should be valid, got error: %v", err) } - if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) { - t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU", - c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0)) + if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) { + t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs", + c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1)) } if c.UpstreamConnections != 64 { diff --git a/internal/config/config.go b/internal/config/config.go index d4b78a2..e330a3e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,7 +12,6 @@ import ( "os" "path/filepath" "regexp" - "runtime" "sort" "strconv" "strings" @@ -21,6 +20,7 @@ import ( "git.eeqj.de/sneak/smartconfig" "go.uber.org/fx" + "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/logger" ) @@ -297,10 +297,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), UpstreamConnections: loader.intVal( keyUpstreamConnections, DefaultUpstreamConnections), - // Decoding and encoding are CPU-bound, so the default is one image - // per CPU Go uses, which follows a container's CPU limit. - MaxConcurrentProcessing: loader.intVal( - keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)), + MaxConcurrentProcessing: loader.intVal(keyMaxConcurrentProcessing, + imageprocessor.DefaultMaxConcurrentProcessing()), UpstreamFetchTimeout: loader.durationVal( keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), UpstreamMaxResponseSize: loader.int64Val( diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index 92cee8a..fda494b 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -20,11 +20,11 @@ import ( var vipsOnce sync.Once // initVips initializes libvips with quiet logging, one worker thread per -// image and no operation cache. Process already works on one image per CPU -// by default, so more threads per image would only compete for the CPUs. -// Each request decodes different source bytes, so the operation cache -// would rarely be hit and would hold memory outside MaxConcurrentProcessing; -// repeated requests are served from pixa's disk cache instead. +// image and no operation cache. With one thread each, the images processed +// at once use no more CPUs than MaxConcurrentProcessing. Each request +// decodes different source bytes, so the operation cache would rarely be +// hit and would hold memory outside MaxConcurrentProcessing; repeated +// requests are served from pixa's disk cache instead. func initVips() { vipsOnce.Do(func() { vips.LoggingSettings(nil, vips.LogLevelError) @@ -179,10 +179,17 @@ type Params struct { // If <= 0, DefaultMaxInputBytes is used. MaxInputBytes int64 // MaxConcurrentProcessing is the most images processed at once. - // If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used. + // If <= 0, DefaultMaxConcurrentProcessing() is used. MaxConcurrentProcessing int } +// DefaultMaxConcurrentProcessing returns the most images processed at once +// when none is set: half the CPUs Go uses (runtime.GOMAXPROCS(0), which +// follows a container's CPU limit), rounded down, and at least 1. +func DefaultMaxConcurrentProcessing() int { + return max(runtime.GOMAXPROCS(0)/2, 1) +} + // New creates a new image processor with the given parameters. // A zero-value Params{} uses sensible defaults. func New(params Params) *ImageProcessor { @@ -195,7 +202,7 @@ func New(params Params) *ImageProcessor { maxConcurrentProcessing := params.MaxConcurrentProcessing if maxConcurrentProcessing <= 0 { - maxConcurrentProcessing = runtime.GOMAXPROCS(0) + maxConcurrentProcessing = DefaultMaxConcurrentProcessing() } return &ImageProcessor{ diff --git a/internal/imageprocessor/max_concurrent_processing_internal_test.go b/internal/imageprocessor/max_concurrent_processing_internal_test.go index a174752..7cb7380 100644 --- a/internal/imageprocessor/max_concurrent_processing_internal_test.go +++ b/internal/imageprocessor/max_concurrent_processing_internal_test.go @@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) { } } -func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) { +func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) { t.Parallel() for _, limit := range []int{0, -1} { proc := New(Params{MaxConcurrentProcessing: limit}) - if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) { - t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU", - limit, got, runtime.GOMAXPROCS(0)) + if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) { + t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs", + limit, got, max(runtime.GOMAXPROCS(0)/2, 1)) } } diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 52684a9..4fb770a 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -52,7 +52,7 @@ type ServiceConfig struct { // Allowlist is the list of hosts that don't require signatures Allowlist []string // MaxConcurrentProcessing is the most images processed at once; zero - // uses the image processor's default, one per CPU + // uses the image processor's default, half the CPUs MaxConcurrentProcessing int // Logger for logging Logger *slog.Logger