From ae7564b7695c545d4d541a3db038b17c32aa3aa8 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 09:04:40 +0000 Subject: [PATCH 01/10] Test that an invalid PIXA_MAX_CONCURRENT_PROCESSING aborts startup An empty, fractional, negative or non-numeric value must abort startup naming the variable and the key, never fall back to the default. The config loader already refuses these; this pins it before the default changes (https://git.eeqj.de/sneak/pixa/issues/242). Model: opus-5-5 --- .../config/concurrency_limits_internal_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/internal/config/concurrency_limits_internal_test.go b/internal/config/concurrency_limits_internal_test.go index c5f4b20..867b041 100644 --- a/internal/config/concurrency_limits_internal_test.go +++ b/internal/config/concurrency_limits_internal_test.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "runtime" "testing" ) @@ -160,3 +161,19 @@ func TestConcurrencyLimitsFromEnvironment(t *testing.T) { }) } } + +// TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup checks +// that a PIXA_MAX_CONCURRENT_PROCESSING that is not a whole number of at +// least 1, an empty one included, aborts startup naming the variable and +// the key instead of taking the default. +func TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup(t *testing.T) { + for _, value := range []string{"", " ", "0.5", "half", "-1", "4 images"} { + t.Run(fmt.Sprintf("value %q", value), func(t *testing.T) { + t.Setenv(testMaxConcurrentProcessingVar, value) + + _, err := configFromYAML(t, signingKeyLine) + wantStartupError(t, err, + testMaxConcurrentProcessingVar, keyMaxConcurrentProcessing) + }) + } +} -- 2.54.0 From de7ecfdb5e552d62cf5057c86258e5ba2217ce65 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 09:05:01 +0000 Subject: [PATCH 02/10] Test that max_concurrent_processing defaults to half the CPUs An omitted max_concurrent_processing, and New given none, must be half the CPUs Go uses, rounded down, and at least 1. The tests set GOMAXPROCS to 1, 2, 3 and 8 and fail until the default changes (https://git.eeqj.de/sneak/pixa/issues/242). Model: opus-5-5 --- .../concurrency_limits_internal_test.go | 30 +++++++++++++++++++ ...max_concurrent_processing_internal_test.go | 24 +++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/internal/config/concurrency_limits_internal_test.go b/internal/config/concurrency_limits_internal_test.go index 867b041..2d61e03 100644 --- a/internal/config/concurrency_limits_internal_test.go +++ b/internal/config/concurrency_limits_internal_test.go @@ -33,6 +33,36 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) { } } +// TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne checks +// that an omitted max_concurrent_processing is half the CPUs Go uses, +// rounded down, and 1 when Go uses one CPU. +// +//nolint:paralleltest // it sets GOMAXPROCS, which other tests read +func TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne(t *testing.T) { + cpusBefore := runtime.GOMAXPROCS(0) + + t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) }) + + for _, tc := range []struct{ cpus, want int }{ + {1, 1}, {2, 1}, {3, 1}, {8, 4}, + } { + runtime.GOMAXPROCS(tc.cpus) + + c, err := configFromYAML(t, signingKeyLine) + if err != nil { + t.Fatalf("minimal config should be valid, got error: %v", err) + } + + t.Logf("GOMAXPROCS %d: MaxConcurrentProcessing %d", + tc.cpus, c.MaxConcurrentProcessing) + + if c.MaxConcurrentProcessing != tc.want { + t.Errorf("GOMAXPROCS %d: MaxConcurrentProcessing = %d, want %d", + tc.cpus, c.MaxConcurrentProcessing, tc.want) + } + } +} + // TestExplicitConcurrencyLimitsAreUsed checks that valid values for the // two limits are used as given. func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) { diff --git a/internal/imageprocessor/max_concurrent_processing_internal_test.go b/internal/imageprocessor/max_concurrent_processing_internal_test.go index ba5388c..30f3fc5 100644 --- a/internal/imageprocessor/max_concurrent_processing_internal_test.go +++ b/internal/imageprocessor/max_concurrent_processing_internal_test.go @@ -132,6 +132,30 @@ func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) { } } +// TestNewDefaultIsHalfTheCPUsAndAtLeastOne checks that New, given no +// MaxConcurrentProcessing, makes one slot for every two CPUs Go uses, +// rounded down, and one slot when Go uses one CPU. +// +//nolint:paralleltest // it sets GOMAXPROCS, which other tests read +func TestNewDefaultIsHalfTheCPUsAndAtLeastOne(t *testing.T) { + cpusBefore := runtime.GOMAXPROCS(0) + + t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) }) + + for _, tc := range []struct{ cpus, want int }{ + {1, 1}, {2, 1}, {3, 1}, {8, 4}, + } { + runtime.GOMAXPROCS(tc.cpus) + + got := cap(New(Params{}).processingSemaphore) + t.Logf("GOMAXPROCS %d: %d slots", tc.cpus, got) + + if got != tc.want { + t.Errorf("GOMAXPROCS %d: %d slots, want %d", tc.cpus, got, tc.want) + } + } +} + // TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls // than MaxConcurrentProcessing allows and holds the first ones inside // Process until the test lets them go. No more than the limit may be -- 2.54.0 From ebdecf9abada1725f3bbb7c18c84d566e7b15128 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 09:36:15 +0000 Subject: [PATCH 03/10] Default max_concurrent_processing to half the CPUs pixa sees (closes #242) The default was one image per CPU Go uses, and a container with no CPU limit sees every CPU of the host. It is now half of runtime.GOMAXPROCS(0), rounded down and at least 1, from imageprocessor.DefaultMaxConcurrentProcessing, which both the config and imageprocessor.New use. The two existing tests of the default expect the new one, as the owner allowed on the issue. "Running under upaas" gives the worst-case memory, with a per-image peak of about 1.6 GiB measured on pixad for an 8192x8192 source in each output format, and tells the operator to set PIXA_MAX_CONCURRENT_PROCESSING and PIXA_UPSTREAM_CONNECTIONS to fit the container's memory. Model: opus-5-5 --- README.md | 23 +++++++++++++++---- TODO.md | 10 ++++++++ configs/config.example.yml | 9 ++++---- .../concurrency_limits_internal_test.go | 10 ++++---- internal/config/config.go | 8 +++---- internal/imageprocessor/imageprocessor.go | 23 +++++++++++++------ ...max_concurrent_processing_internal_test.go | 8 +++---- internal/imgcache/service.go | 2 +- 8 files changed, 62 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 6c46fe4..3baf0d4 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,17 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: the cache can grow to three quarters or more of the space they all have left, and to all of it when that is 500 MiB or less. - 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 @@ -566,7 +577,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 | @@ -652,10 +663,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 22289af..3b0805a 100644 --- a/TODO.md +++ b/TODO.md @@ -154,6 +154,16 @@ P2: security: per-origin rate limiting not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a `Referer` is blocked whatever its port. A URL is signed over its host as it writes it, in lower case, with any IPv6 brackets, `:` and port kept. +- 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 2d61e03..51406e3 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 2eaf698..3a60d4c 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 ca033e7..dcdae0c 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,19 @@ 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 { + const cpusPerImage = 2 + + return max(runtime.GOMAXPROCS(0)/cpusPerImage, 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 +204,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 30f3fc5..2dbbbbf 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 1806a42..212e479 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -53,7 +53,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 -- 2.54.0 From 5b89f928d7b1748dae2cb12c77b1010243c3c53a Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 13:07:16 +0000 Subject: [PATCH 04/10] Define the default max_concurrent_processing in internal/config The default is configuration, so it now sits with the other defaults as config.DefaultMaxConcurrentProcessing, and imageprocessor.New reads it from there for a value below 1. internal/config no longer imports internal/imageprocessor, so the packages that import internal/config do not build against libvips. Model: opus-5-5 --- TODO.md | 19 +++++++++---------- internal/config/config.go | 16 +++++++++++++--- internal/imageprocessor/imageprocessor.go | 15 +++------------ 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/TODO.md b/TODO.md index 3b0805a..32fbe22 100644 --- a/TODO.md +++ b/TODO.md @@ -122,6 +122,15 @@ P2: security: per-origin rate limiting 405, or a CORS preflight. The metrics library takes the label before the request is routed, so the metrics middleware asks the router which route matches. +- 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. + `config.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 expired negative cache rows are removed on a schedule (closes #244): the row of a failed fetch, which makes pixa answer 502 for that source URL for 5 minutes, was deleted only when the same URL was requested again @@ -154,16 +163,6 @@ P2: security: per-origin rate limiting not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a `Referer` is blocked whatever its port. A URL is signed over its host as it writes it, in lower case, with any IPv6 brackets, `:` and port kept. -- 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/internal/config/config.go b/internal/config/config.go index 3a60d4c..0ff1263 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" "regexp" + "runtime" "sort" "strconv" "strings" @@ -20,7 +21,6 @@ import ( "git.eeqj.de/sneak/smartconfig" "go.uber.org/fx" - "sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/logger" ) @@ -37,6 +37,16 @@ const ( DefaultImageRequestsPerMinute = 600 ) +// DefaultMaxConcurrentProcessing returns the most images processed at once +// when max_concurrent_processing is omitted: half the CPUs Go uses +// (runtime.GOMAXPROCS(0), which follows a container's CPU limit), rounded +// down, and at least 1. +func DefaultMaxConcurrentProcessing() int { + const cpusPerImage = 2 + + return max(runtime.GOMAXPROCS(0)/cpusPerImage, 1) +} + // Configuration key names. const ( keyDebug = "debug" @@ -297,8 +307,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), UpstreamConnections: loader.intVal( keyUpstreamConnections, DefaultUpstreamConnections), - MaxConcurrentProcessing: loader.intVal(keyMaxConcurrentProcessing, - imageprocessor.DefaultMaxConcurrentProcessing()), + MaxConcurrentProcessing: loader.intVal( + keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()), UpstreamFetchTimeout: loader.durationVal( keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), UpstreamMaxResponseSize: loader.int64Val( diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index dcdae0c..b4be6d6 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -7,11 +7,11 @@ import ( "errors" "fmt" "io" - "runtime" "sync" "time" "github.com/davidbyttow/govips/v2/vips" + "sneak.berlin/go/pixa/internal/config" ) // vipsOnce ensures vips is initialized exactly once. @@ -179,19 +179,10 @@ type Params struct { // If <= 0, DefaultMaxInputBytes is used. MaxInputBytes int64 // MaxConcurrentProcessing is the most images processed at once. - // If <= 0, DefaultMaxConcurrentProcessing() is used. + // If <= 0, config.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 { - const cpusPerImage = 2 - - return max(runtime.GOMAXPROCS(0)/cpusPerImage, 1) -} - // New creates a new image processor with the given parameters. // A zero-value Params{} uses sensible defaults. func New(params Params) *ImageProcessor { @@ -204,7 +195,7 @@ func New(params Params) *ImageProcessor { maxConcurrentProcessing := params.MaxConcurrentProcessing if maxConcurrentProcessing <= 0 { - maxConcurrentProcessing = DefaultMaxConcurrentProcessing() + maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing() } return &ImageProcessor{ -- 2.54.0 From e8fdd49c801372428e7714b0235c53c351b133fe Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 13:07:22 +0000 Subject: [PATCH 05/10] Claim only one libvips worker thread per image in initVips The comment said the images processed at once use no more CPUs than max_concurrent_processing, which the review found does not hold for GIF output. It now says only that libvips runs one worker thread per image. Model: opus-5-5 --- internal/imageprocessor/imageprocessor.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/imageprocessor/imageprocessor.go b/internal/imageprocessor/imageprocessor.go index b4be6d6..2a1fbf5 100644 --- a/internal/imageprocessor/imageprocessor.go +++ b/internal/imageprocessor/imageprocessor.go @@ -20,11 +20,10 @@ import ( var vipsOnce sync.Once // initVips initializes libvips with quiet logging, one worker thread per -// 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. +// image and no operation cache. 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) -- 2.54.0 From 64d96aab87e2f9a92004198321c08c123a8a5328 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 13:27:29 +0000 Subject: [PATCH 06/10] Give the size of libvips' temporary file in "Running under upaas" While pixa processes a large source, libvips writes its decoded pixels to a file in /tmp, outside /var/lib/pixa and cache_max_bytes, and not in the 1.6 GiB of resident memory. Measured on pixad for 8192x8192 sources, it is 192 MiB for an 8-bit RGB JPEG, 512 MiB for 16-bit RGBA, and 1 GiB for a floating-point RGBA JPEG XL. The section now says so, and that the file is memory where /tmp is a tmpfs. Model: opus-5-5 --- README.md | 10 ++++++++++ TODO.md | 5 ++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3baf0d4..efdfdeb 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,16 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: 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. +- **Temporary files:** while pixa processes a large source, libvips writes its + decoded pixels to a file in `/tmp` and deletes the file when the image is + done. For an 8192x8192 source the file was measured on pixad at 192 MiB for an + 8-bit RGB JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG + XL of 32-bit floating-point RGBA, the largest measured: up to 1 GiB per image + processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs. + This `/tmp` is in the container's own filesystem, not on the volume at + `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 1.6 GiB above + does not include these files; where `/tmp` is a tmpfs, they are memory on top + of it. - **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 diff --git a/TODO.md b/TODO.md index 32fbe22..e2072f4 100644 --- a/TODO.md +++ b/TODO.md @@ -130,7 +130,10 @@ P2: security: per-origin rate limiting `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. + `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. It also gives the + file libvips writes to `/tmp` for each large source, up to 1 GiB, measured, + which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where + `/tmp` is a tmpfs. - 2026-10-15 expired negative cache rows are removed on a schedule (closes #244): the row of a failed fetch, which makes pixa answer 502 for that source URL for 5 minutes, was deleted only when the same URL was requested again -- 2.54.0 From 80e336c937e6399d7c0b88869ce31355ff1bd6b2 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 17:19:47 +0000 Subject: [PATCH 07/10] Say half the CPUs plainly in DefaultMaxConcurrentProcessing The constant cpusPerImage gave a reason nothing else states, that each image uses two CPUs. The function now divides by 2, with the nolint comment the repo uses for such numbers. Model: opus-5-5 --- internal/config/config.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0ff1263..ff4ac6c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -42,9 +42,7 @@ const ( // (runtime.GOMAXPROCS(0), which follows a container's CPU limit), rounded // down, and at least 1. func DefaultMaxConcurrentProcessing() int { - const cpusPerImage = 2 - - return max(runtime.GOMAXPROCS(0)/cpusPerImage, 1) + return max(runtime.GOMAXPROCS(0)/2, 1) //nolint:mnd // half the CPUs } // Configuration key names. -- 2.54.0 From b12cb4565ce868840e08844ae8b0cf97ed350600 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 17:19:47 +0000 Subject: [PATCH 08/10] Give the largest measured per-image memory in "Running under upaas" The per-image figure is now the largest peak measured over one 8192x8192 source of each source format, with 16-bit PNG, 12-bit AVIF, and 16-bit and 32-bit floating-point JPEG XL sources too, each saved at full size to every output format with pixad's libvips settings: 4.2 GiB, for the 16-bit JPEG XL saved as WebP. Temporary files says that for a GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in memory as well as written to /tmp. Model: opus-5-5 --- README.md | 33 +++++++++++++++++++-------------- TODO.md | 24 ++++++++++++------------ 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index efdfdeb..f72f415 100644 --- a/README.md +++ b/README.md @@ -131,25 +131,30 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: the cache can grow to three quarters or more of the space they all have left, and to all of it when that is 500 MiB or less. - the rest are in the table under Configuration below -- **Memory:** pixa can use up to `max_concurrent_processing` times 1.6 GiB for +- **Memory:** pixa can use up to `max_concurrent_processing` times 4.2 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 + for processing. 4.2 GiB is the most resident memory was measured to grow while + pixa, with pixad's libvips settings, processed one 8192x8192 source, the + largest it accepts, saving it at full size to each output format in turn, one + at a time. The sources were a JPEG, a GIF, a WebP, an 8-bit and a 16-bit PNG, + an 8-bit and a 12-bit AVIF, and an 8-bit, a 16-bit and a 32-bit floating-point + JPEG XL; memory grew most for the 16-bit JPEG XL, of random pixels, saved as + WebP. At the defaults that is half the CPUs pixa sees times 4.2 GiB, plus 64 + times 50 MiB: on a host where pixa sees 48 CPUs, 24 times 4.2 GiB plus 3.1 + GiB, about 104 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING` and + `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may use. - **Temporary files:** while pixa processes a large source, libvips writes its decoded pixels to a file in `/tmp` and deletes the file when the image is - done. For an 8192x8192 source the file was measured on pixad at 192 MiB for an - 8-bit RGB JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG - XL of 32-bit floating-point RGBA, the largest measured: up to 1 GiB per image - processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs. - This `/tmp` is in the container's own filesystem, not on the volume at - `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 1.6 GiB above + done. For a GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in + memory as well as written to that file, and the 4.2 GiB above includes that + memory. For an 8192x8192 source the file was measured on pixad at 192 MiB for + an 8-bit RGB JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a + JPEG XL of 32-bit floating-point RGBA, the largest measured: up to 1 GiB per + image processed at once, 24 GiB at the defaults on a host where pixa sees 48 + CPUs. This `/tmp` is in the container's own filesystem, not on the volume at + `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 4.2 GiB above does not include these files; where `/tmp` is a tmpfs, they are memory on top of it. - **Health check:** the image's `HEALTHCHECK` requests diff --git a/TODO.md b/TODO.md index e2072f4..8950d27 100644 --- a/TODO.md +++ b/TODO.md @@ -76,6 +76,18 @@ P2: security: per-origin rate limiting the cookie, as anyone can take a CSRF token from the login page. A logout that cannot be saved answers 500. `session.NewManager`, which the tests use, keeps the value in memory only; pixad uses `session.NewManagerWithDatabase`. +- 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 4.2 GiB. + `config.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. It also gives the + file libvips writes to `/tmp` for each large source, up to 1 GiB, measured, + which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where + `/tmp` is a tmpfs. - 2026-10-15 a cached file left empty or partial is no longer served, or fetched again on every request (closes #252). Source and transformed image files, and each transformed image's `.meta` file, are synced to disk before they are @@ -122,18 +134,6 @@ P2: security: per-origin rate limiting 405, or a CORS preflight. The metrics library takes the label before the request is routed, so the metrics middleware asks the router which route matches. -- 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. - `config.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. It also gives the - file libvips writes to `/tmp` for each large source, up to 1 GiB, measured, - which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where - `/tmp` is a tmpfs. - 2026-10-15 expired negative cache rows are removed on a schedule (closes #244): the row of a failed fetch, which makes pixa answer 502 for that source URL for 5 minutes, was deleted only when the same URL was requested again -- 2.54.0 From cead3d57bf233e62932e48ca7349be853fdc2d64 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 19:16:44 +0000 Subject: [PATCH 09/10] Say the libvips temporary file leaves /tmp but keeps its space libvips removes the file's name from /tmp as soon as it has written the decoded pixels, and the file keeps its space, on disk or in memory where /tmp is a tmpfs, until the image is done. "Running under upaas" said the file is deleted when the image is done, so an operator looking in /tmp for it would find nothing. Model: opus-5-5 --- README.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index f72f415..2666d03 100644 --- a/README.md +++ b/README.md @@ -146,14 +146,16 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may use. - **Temporary files:** while pixa processes a large source, libvips writes its - decoded pixels to a file in `/tmp` and deletes the file when the image is - done. For a GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in - memory as well as written to that file, and the 4.2 GiB above includes that - memory. For an 8192x8192 source the file was measured on pixad at 192 MiB for - an 8-bit RGB JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a - JPEG XL of 32-bit floating-point RGBA, the largest measured: up to 1 GiB per - image processed at once, 24 GiB at the defaults on a host where pixa sees 48 - CPUs. This `/tmp` is in the container's own filesystem, not on the volume at + decoded pixels to a file in `/tmp`. The file disappears from `/tmp` once it is + written, so a listing of `/tmp` does not show it, but it keeps its space, on + disk or, where `/tmp` is a tmpfs, in memory, until the image is done. For a + GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in memory as + well as written to that file, and the 4.2 GiB above includes that memory. For + an 8192x8192 source the file was measured on pixad at 192 MiB for an 8-bit RGB + JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG XL of + 32-bit floating-point RGBA, the largest measured: up to 1 GiB per image + processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs. + This `/tmp` is in the container's own filesystem, not on the volume at `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 4.2 GiB above does not include these files; where `/tmp` is a tmpfs, they are memory on top of it. -- 2.54.0 From a9e4d76b5f767bb37b684de6fb1a398b53fe6d7f Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 19:16:44 +0000 Subject: [PATCH 10/10] Keep this issue's TODO.md entry at the top of Completed Steps The rebase onto next put the newer entry above it. Model: opus-5-5 --- TODO.md | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index 8950d27..1b59abf 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,18 @@ 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 4.2 GiB. + `config.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. It also gives the + file libvips writes to `/tmp` for each large source, up to 1 GiB, measured, + which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where + `/tmp` is a tmpfs. - 2026-10-15 the canonical files are re-vendored from `sneak/prompts` commit `e3f79c4` (closes #221): `.dockerignore`, `.editorconfig`, `.gitea/workflows/check.yml`, `.gitignore`, `.golangci.yml`, @@ -76,18 +88,6 @@ P2: security: per-origin rate limiting the cookie, as anyone can take a CSRF token from the login page. A logout that cannot be saved answers 500. `session.NewManager`, which the tests use, keeps the value in memory only; pixad uses `session.NewManagerWithDatabase`. -- 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 4.2 GiB. - `config.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. It also gives the - file libvips writes to `/tmp` for each large source, up to 1 GiB, measured, - which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where - `/tmp` is a tmpfs. - 2026-10-15 a cached file left empty or partial is no longer served, or fetched again on every request (closes #252). Source and transformed image files, and each transformed image's `.meta` file, are synced to disk before they are -- 2.54.0