Where: internal/config/config.go line 303 and internal/imageprocessor/imageprocessor.go line 188 (default max_concurrent_processing is the CPU count pixa sees); internal/imgcache/service.go lines 453-460 (a whole source stays in memory while its request waits for a slot); README.md "Running under upaas" (line 106) and max_concurrent_processing (line 586), on v1.0.0-rc1.
What is wrong: one transcode of an 8192x8192 source, the documented input cap, decodes to 200-540 MB, and WebP, AVIF and JPEG XL hold the whole output in memory too, so one slot can take about 0.5-1.2 GB. The default slot count is the number of CPUs pixa sees, and containers on the shared host run without CPU or memory flags (#158): on a 48-CPU host that is 48 slots, tens of GB in the worst case, plus up to upstream_connections (64) sources of upstream_max_response_size (50 MiB) each waiting for a slot. None of this is in "Running under upaas", which lists the port, the volume, three variables and the health check.
How it fails: with a host carrying user content allowlisted (the example config allowlists one), an outsider uploads a few 8192x8192 PNGs of a few KB each and requests them at distinct sizes; each request costs about a GB while it runs, and the shared host runs out of memory before it runs out of CPU.
Severity: high
Definition of done:
README.md "Running under upaas" gives the worst-case memory (slots times the per-image bound, plus connections times upstream_max_response_size) and tells the operator to set PIXA_MAX_CONCURRENT_PROCESSING and PIXA_UPSTREAM_CONNECTIONS to fit the memory the container may use.
The default slot count no longer follows the host's CPU count when no CPU limit is set: a small fixed default, or one from a memory budget; the implementer proposes, the reviewer and owner decide.
make check green.
Model: fable-5-1
Where: `internal/config/config.go` line 303 and `internal/imageprocessor/imageprocessor.go` line 188 (default `max_concurrent_processing` is the CPU count pixa sees); `internal/imgcache/service.go` lines 453-460 (a whole source stays in memory while its request waits for a slot); `README.md` "Running under upaas" (line 106) and `max_concurrent_processing` (line 586), on `v1.0.0-rc1`.
What is wrong: one transcode of an 8192x8192 source, the documented input cap, decodes to 200-540 MB, and WebP, AVIF and JPEG XL hold the whole output in memory too, so one slot can take about 0.5-1.2 GB. The default slot count is the number of CPUs pixa sees, and containers on the shared host run without CPU or memory flags (https://git.eeqj.de/sneak/pixa/issues/158): on a 48-CPU host that is 48 slots, tens of GB in the worst case, plus up to `upstream_connections` (64) sources of `upstream_max_response_size` (50 MiB) each waiting for a slot. None of this is in "Running under upaas", which lists the port, the volume, three variables and the health check.
How it fails: with a host carrying user content allowlisted (the example config allowlists one), an outsider uploads a few 8192x8192 PNGs of a few KB each and requests them at distinct sizes; each request costs about a GB while it runs, and the shared host runs out of memory before it runs out of CPU.
Severity: high
Definition of done:
1. `README.md` "Running under upaas" gives the worst-case memory (slots times the per-image bound, plus connections times `upstream_max_response_size`) and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and `PIXA_UPSTREAM_CONNECTIONS` to fit the memory the container may use.
2. The default slot count no longer follows the host's CPU count when no CPU limit is set: a small fixed default, or one from a memory budget; the implementer proposes, the reviewer and owner decide.
3. `make check` green.
Model: fable-5-1
clawbot
self-assigned this 2026-10-13 08:46:09 +02:00
The default max_concurrent_processing becomes the smaller of runtime.GOMAXPROCS(0) (which follows a container CPU limit) and 4, defined once and used by both internal/config and the fallback in imageprocessor.New.
"Running under upaas" gives the worst-case memory as slots times the per-image bound plus upstream_connections times upstream_max_response_size, with the numbers at the defaults, and tells the operator to set PIXA_MAX_CONCURRENT_PROCESSING and PIXA_UPSTREAM_CONNECTIONS to fit the container's memory. The Configuration table states the new default.
The per-image bound is measured (one 8192x8192 source per output format, one at a time, peak memory), not copied from this issue; the README says it was measured.
Package: internal/imageprocessor and internal/config; no new file. Test first for the default.
Model: opus-5-5
Plan, taking the fixed-default option:
- The default `max_concurrent_processing` becomes the smaller of `runtime.GOMAXPROCS(0)` (which follows a container CPU limit) and 4, defined once and used by both `internal/config` and the fallback in `imageprocessor.New`.
- "Running under upaas" gives the worst-case memory as slots times the per-image bound plus `upstream_connections` times `upstream_max_response_size`, with the numbers at the defaults, and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. The Configuration table states the new default.
- The per-image bound is measured (one 8192x8192 source per output format, one at a time, peak memory), not copied from this issue; the README says it was measured.
Package: `internal/imageprocessor` and `internal/config`; no new file. Test first for the default.
Model: opus-5-5
Question: may two existing tests change? CLAUDE.md needs your approval to change an existing test. Both expect an omitted max_concurrent_processing to be runtime.GOMAXPROCS(0), the default this issue removes, so any fix fails them on the 48-CPU host the tests run on: TestOmittedConcurrencyLimitsUseDefaults (internal/config) and TestNewDefaultsMaxConcurrentProcessingToCPUs (internal/imageprocessor). Details in #261 (comment).
Yes, and the same for any existing test that expects the behaviour a filed bug says is wrong (recommended): each test's expected default becomes the new one, the smaller of the CPU count and 4; nothing else in them changes.
Yes, for these two only.
No: the default keeps following the CPU count, and this issue ends with the README's memory figures and advice only.
Model: opus-5-5
Question: may two existing tests change? `CLAUDE.md` needs your approval to change an existing test. Both expect an omitted `max_concurrent_processing` to be `runtime.GOMAXPROCS(0)`, the default this issue removes, so any fix fails them on the 48-CPU host the tests run on: `TestOmittedConcurrencyLimitsUseDefaults` (`internal/config`) and `TestNewDefaultsMaxConcurrentProcessingToCPUs` (`internal/imageprocessor`). Details in https://git.eeqj.de/sneak/pixa/pulls/261#issuecomment-141688.
- Yes, and the same for any existing test that expects the behaviour a filed bug says is wrong (recommended): each test's expected default becomes the new one, the smaller of the CPU count and 4; nothing else in them changes.
- Yes, for these two only.
- No: the default keeps following the CPU count, and this issue ends with the README's memory figures and advice only.
Model: opus-5-5
clawbot
removed their assignment 2026-10-15 04:26:41 +02:00
sneak
was assigned by clawbot2026-10-15 04:26:41 +02:00
i don't like capping max_concurrent_processing. perhaps default to 50% of the cpus and allow a user to override it in an env var.
you are allowed to update the tests accordingly.
i don't like capping max_concurrent_processing. perhaps default to 50% of the cpus and allow a user to override it in an env var.
you are allowed to update the tests accordingly.
sneak
was unassigned by clawbot2026-10-15 10:43:24 +02:00
clawbot
self-assigned this 2026-10-15 10:43:24 +02:00
Plan after sneak's ruling (#242 (comment)), replacing the cap of 4 in #261:
The default max_concurrent_processing is half of runtime.GOMAXPROCS(0), the CPUs pixa sees (it follows a container CPU limit), and at least 1. It is defined once and used by internal/config and by the fallback in imageprocessor.New.
PIXA_MAX_CONCURRENT_PROCESSING (and the config file key) stays the override. A set value that is not a whole number of at least 1 stops pixa at startup with an error naming the setting, never a fallback to the default. Tests first for the default and for an unparseable value.
TestOmittedConcurrencyLimitsUseDefaults and TestNewDefaultsMaxConcurrentProcessingToCPUs expect the new default in place of the CPU count, as sneak allowed.
"Running under upaas" keeps the memory guidance of the definition of done: the worst case as slots times the measured per-image bound plus upstream_connections times upstream_max_response_size, and an instruction to set PIXA_MAX_CONCURRENT_PROCESSING and PIXA_UPSTREAM_CONNECTIONS to fit the container's memory. The Configuration table gives the new default.
Model: opus-5-5
Plan after sneak's ruling (https://git.eeqj.de/sneak/pixa/issues/242#issuecomment-142377), replacing the cap of 4 in https://git.eeqj.de/sneak/pixa/pulls/261:
- The default `max_concurrent_processing` is half of `runtime.GOMAXPROCS(0)`, the CPUs pixa sees (it follows a container CPU limit), and at least 1. It is defined once and used by `internal/config` and by the fallback in `imageprocessor.New`.
- `PIXA_MAX_CONCURRENT_PROCESSING` (and the config file key) stays the override. A set value that is not a whole number of at least 1 stops pixa at startup with an error naming the setting, never a fallback to the default. Tests first for the default and for an unparseable value.
- `TestOmittedConcurrencyLimitsUseDefaults` and `TestNewDefaultsMaxConcurrentProcessingToCPUs` expect the new default in place of the CPU count, as sneak allowed.
- "Running under upaas" keeps the memory guidance of the definition of done: the worst case as slots times the measured per-image bound plus `upstream_connections` times `upstream_max_response_size`, and an instruction to set `PIXA_MAX_CONCURRENT_PROCESSING` and `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. The Configuration table gives the new default.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Where:
internal/config/config.goline 303 andinternal/imageprocessor/imageprocessor.goline 188 (defaultmax_concurrent_processingis the CPU count pixa sees);internal/imgcache/service.golines 453-460 (a whole source stays in memory while its request waits for a slot);README.md"Running under upaas" (line 106) andmax_concurrent_processing(line 586), onv1.0.0-rc1.What is wrong: one transcode of an 8192x8192 source, the documented input cap, decodes to 200-540 MB, and WebP, AVIF and JPEG XL hold the whole output in memory too, so one slot can take about 0.5-1.2 GB. The default slot count is the number of CPUs pixa sees, and containers on the shared host run without CPU or memory flags (#158): on a 48-CPU host that is 48 slots, tens of GB in the worst case, plus up to
upstream_connections(64) sources ofupstream_max_response_size(50 MiB) each waiting for a slot. None of this is in "Running under upaas", which lists the port, the volume, three variables and the health check.How it fails: with a host carrying user content allowlisted (the example config allowlists one), an outsider uploads a few 8192x8192 PNGs of a few KB each and requests them at distinct sizes; each request costs about a GB while it runs, and the shared host runs out of memory before it runs out of CPU.
Severity: high
Definition of done:
README.md"Running under upaas" gives the worst-case memory (slots times the per-image bound, plus connections timesupstream_max_response_size) and tells the operator to setPIXA_MAX_CONCURRENT_PROCESSINGandPIXA_UPSTREAM_CONNECTIONSto fit the memory the container may use.make checkgreen.Model: fable-5-1
Plan, taking the fixed-default option:
max_concurrent_processingbecomes the smaller ofruntime.GOMAXPROCS(0)(which follows a container CPU limit) and 4, defined once and used by bothinternal/configand the fallback inimageprocessor.New.upstream_connectionstimesupstream_max_response_size, with the numbers at the defaults, and tells the operator to setPIXA_MAX_CONCURRENT_PROCESSINGandPIXA_UPSTREAM_CONNECTIONSto fit the container's memory. The Configuration table states the new default.Package:
internal/imageprocessorandinternal/config; no new file. Test first for the default.Model: opus-5-5
Question: may two existing tests change?
CLAUDE.mdneeds your approval to change an existing test. Both expect an omittedmax_concurrent_processingto beruntime.GOMAXPROCS(0), the default this issue removes, so any fix fails them on the 48-CPU host the tests run on:TestOmittedConcurrencyLimitsUseDefaults(internal/config) andTestNewDefaultsMaxConcurrentProcessingToCPUs(internal/imageprocessor). Details in #261 (comment).Model: opus-5-5
i don't like capping max_concurrent_processing. perhaps default to 50% of the cpus and allow a user to override it in an env var.
you are allowed to update the tests accordingly.
Plan after sneak's ruling (#242 (comment)), replacing the cap of 4 in #261:
max_concurrent_processingis half ofruntime.GOMAXPROCS(0), the CPUs pixa sees (it follows a container CPU limit), and at least 1. It is defined once and used byinternal/configand by the fallback inimageprocessor.New.PIXA_MAX_CONCURRENT_PROCESSING(and the config file key) stays the override. A set value that is not a whole number of at least 1 stops pixa at startup with an error naming the setting, never a fallback to the default. Tests first for the default and for an unparseable value.TestOmittedConcurrencyLimitsUseDefaultsandTestNewDefaultsMaxConcurrentProcessingToCPUsexpect the new default in place of the CPU count, as sneak allowed.upstream_connectionstimesupstream_max_response_size, and an instruction to setPIXA_MAX_CONCURRENT_PROCESSINGandPIXA_UPSTREAM_CONNECTIONSto fit the container's memory. The Configuration table gives the new default.Model: opus-5-5