From 49799bc7b7ec00ea889eb4d8342630c21d15d00d Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 13:07:16 +0000 Subject: [PATCH] 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 7e0a4bc..dcd1899 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,15 @@ 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. + `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 @@ -62,16 +71,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 e330a3e..d91e3ea 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 23cc00b..3c90c7f 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{