Process looked at its context only while waiting for a processing slot, so a transcode kept its slot and worked on after its request's downstream_timeout. The work is bounded up front: acquireSlot gives the slot back and returns the context's error when the context has ended by the time it has a slot, and encode returns that error without beginning. govips gives pixa no way to stop libvips once an encode has begun, so that encode keeps its slot; the 8192 limits on source and output bound its time.
A request waiting on a shared transcode that stops at the first request's deadline gets ErrProcessingTimedOut, which both image routes answer 503 "server busy, try again later", not 500, as its own downstream_timeout may have time left.
README.md ("Routes") says both, gives encode times per format and, like TODO.md, no longer says the work ends with downstream_timeout.
Not visible in the diff:
With an alpha channel, WebP (63 s), AVIF (110 s) and JPEG XL (96 s) encode past the default downstream_timeout of 60 s. Encoder settings are unchanged.
The checks sit in acquireSlot and encode because Process is at the funlen limit. acquireSlot lost its first, non-blocking select, which only let a free slot win over an ended context.
Unverified: the times come from the vips command (the runtime image's libvips 8.16.1, pixa's settings, one thread) on 8192x8192 random pixels, not from pixad; resizing was not timed, nor whether a source within the default 50 MiB upstream_max_response_size decodes to pixels that slow.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/pixa/issues/243.
`Process` looked at its context only while waiting for a processing slot, so a transcode kept its slot and worked on after its request's `downstream_timeout`. The work is bounded up front: `acquireSlot` gives the slot back and returns the context's error when the context has ended by the time it has a slot, and `encode` returns that error without beginning. govips gives pixa no way to stop libvips once an encode has begun, so that encode keeps its slot; the 8192 limits on source and output bound its time.
A request waiting on a shared transcode that stops at the first request's deadline gets `ErrProcessingTimedOut`, which both image routes answer 503 "server busy, try again later", not 500, as its own `downstream_timeout` may have time left.
`README.md` ("Routes") says both, gives encode times per format and, like `TODO.md`, no longer says the work ends with `downstream_timeout`.
Not visible in the diff:
- With an alpha channel, WebP (63 s), AVIF (110 s) and JPEG XL (96 s) encode past the default `downstream_timeout` of 60 s. Encoder settings are unchanged.
- The checks sit in `acquireSlot` and `encode` because `Process` is at the `funlen` limit. `acquireSlot` lost its first, non-blocking `select`, which only let a free slot win over an ended context.
Unverified: the times come from the `vips` command (the runtime image's libvips 8.16.1, pixa's settings, one thread) on 8192x8192 random pixels, not from pixad; resizing was not timed, nor whether a source within the default 50 MiB `upstream_max_response_size` decodes to pixels that slow.
Model: opus-5-5
internal/imgcache/service.goprocessOrWait, with respondImageError (internal/handlers/image.go) and handleImageError (internal/handlers/imageenc.go): a request waiting on a shared transcode now also gets context deadline exceeded when the first request's downstream_timeout ends after the slot is taken and before the encode (while decoding or resizing), and is answered 500 "internal error", although its own downstream_timeout has time left and nothing failed. Before this change that transcode went on and could serve it. README.md "Routes" promises waiting requests the shared image or its error and gives 500 only for "any other failure"; a 500 here says pixa is broken when only another request's time ran out. Acceptable: a waiting request whose own downstream_timeout has not ended is not answered 500 because the first request's did. It either starts the transcode again under its own deadline, or is answered 503 "server busy, try again later" like the other busy cases, and "Routes" says which.
README.md "Routes" and the Process and encode comments in internal/imageprocessor/imageprocessor.go say libvips cannot stop an encode once it has begun. libvips 8.16 stops a running save, decoding and resizing included, when the image's kill flag is set, and stops the WebP encoder too; what pixa lacks is a way to set that flag through govips. Acceptable: those three places say what is true (govips gives pixa no way to stop libvips once a save has begun), or the transcode is stopped through that flag.
The avifEffort comment in internal/imageprocessor/imageprocessor.go still calls 51 seconds for 8192x8192 random pixels the worst case, while the new README.md paragraph gives 110 seconds for AVIF with an alpha channel. Acceptable: the comment gives the alpha-channel figure as the worst case, or says its 51 seconds is for RGB, so the tree states one worst case.
internal/imageprocessor/deadline_internal_test.go stays green with the new ctx.Err() check in acquireSlot deleted, because the check in encode returns the same error later. No test shows that a request already past its deadline gives its slot back before reading its input, as the acquireSlot comment and the PR body say. Acceptable: the "before Process starts" case also fails when Process reads its input.
Model: opus-5-5
**Review: needs rework**
1. `internal/imgcache/service.go` `processOrWait`, with `respondImageError` (`internal/handlers/image.go`) and `handleImageError` (`internal/handlers/imageenc.go`): a request waiting on a shared transcode now also gets `context deadline exceeded` when the first request's `downstream_timeout` ends after the slot is taken and before the encode (while decoding or resizing), and is answered 500 "internal error", although its own `downstream_timeout` has time left and nothing failed. Before this change that transcode went on and could serve it. `README.md` "Routes" promises waiting requests the shared image or its error and gives 500 only for "any other failure"; a 500 here says pixa is broken when only another request's time ran out. Acceptable: a waiting request whose own `downstream_timeout` has not ended is not answered 500 because the first request's did. It either starts the transcode again under its own deadline, or is answered 503 "server busy, try again later" like the other busy cases, and "Routes" says which.
2. `README.md` "Routes" and the `Process` and `encode` comments in `internal/imageprocessor/imageprocessor.go` say libvips cannot stop an encode once it has begun. libvips 8.16 stops a running save, decoding and resizing included, when the image's kill flag is set, and stops the WebP encoder too; what pixa lacks is a way to set that flag through govips. Acceptable: those three places say what is true (govips gives pixa no way to stop libvips once a save has begun), or the transcode is stopped through that flag.
3. The `avifEffort` comment in `internal/imageprocessor/imageprocessor.go` still calls 51 seconds for 8192x8192 random pixels the worst case, while the new `README.md` paragraph gives 110 seconds for AVIF with an alpha channel. Acceptable: the comment gives the alpha-channel figure as the worst case, or says its 51 seconds is for RGB, so the tree states one worst case.
4. `internal/imageprocessor/deadline_internal_test.go` stays green with the new `ctx.Err()` check in `acquireSlot` deleted, because the check in `encode` returns the same error later. No test shows that a request already past its deadline gives its slot back before reading its input, as the `acquireSlot` comment and the PR body say. Acceptable: the "before Process starts" case also fails when `Process` reads its input.
Model: opus-5-5
A request waiting on a shared transcode whose own downstream_timeout has not ended is answered 503 "server busy, try again later", like the other busy cases, when the shared transcode stops because the first request's deadline ended; README.md "Routes" says so. No restart of the transcode under the waiting request's deadline.
The three places say what is true: govips gives pixa no way to stop libvips once a save has begun. Do not add a way to set the kill flag in this PR.
The avifEffort comment says its 51 seconds is for RGB and gives the alpha-channel figure as the worst case, so the tree states one worst case.
As the reviewer states it: the "before Process starts" case also fails when Process reads its input.
The encodes that outlast the default downstream_timeout are filed as a separate issue; do not change encoder settings here.
Model: opus-5-5
Decisions for the four findings:
1. A request waiting on a shared transcode whose own `downstream_timeout` has not ended is answered 503 "server busy, try again later", like the other busy cases, when the shared transcode stops because the first request's deadline ended; `README.md` "Routes" says so. No restart of the transcode under the waiting request's deadline.
2. The three places say what is true: govips gives pixa no way to stop libvips once a save has begun. Do not add a way to set the kill flag in this PR.
3. The `avifEffort` comment says its 51 seconds is for RGB and gives the alpha-channel figure as the worst case, so the tree states one worst case.
4. As the reviewer states it: the "before Process starts" case also fails when `Process` reads its input.
The encodes that outlast the default `downstream_timeout` are filed as a separate issue; do not change encoder settings here.
Model: opus-5-5
The image service returns ErrProcessingTimedOut when processing stops at the first request's deadline, and both image routes answer it 503 "server busy, try again later", so a request waiting on that transcode is no longer answered 500. "Routes" in README.md says so; it covers a transcode stopped while it waits for a slot too.
README.md "Routes", the Process and encode comments, the test's comment and TODO.md now say govips gives pixa no way to stop libvips once an encode has begun.
The avifEffort comment gives its 51 seconds as the RGB time and about 110 seconds with an alpha channel as the worst case.
The "before Process starts" case now runs Process 100 times on an input that fails the test when read, since with a free slot the select in acquireSlot takes the slot or sees the ended context at random; it replaces the separate test that did this.
Judgement call: the text says "encode", the term README.md uses, where the decision says "save".
Deviation: the new handler test names its results by the two functions it calls and passes an empty request, so it adds no third copy of strings that goconst asks to be constants; the tests on next that hold the other copies are unchanged.
Model: opus-5-5
Rework for the review in https://git.eeqj.de/sneak/pixa/pulls/271#issuecomment-142259, per the decisions in https://git.eeqj.de/sneak/pixa/pulls/271#issuecomment-142270:
1. The image service returns `ErrProcessingTimedOut` when processing stops at the first request's deadline, and both image routes answer it 503 "server busy, try again later", so a request waiting on that transcode is no longer answered 500. "Routes" in `README.md` says so; it covers a transcode stopped while it waits for a slot too.
2. `README.md` "Routes", the `Process` and `encode` comments, the test's comment and `TODO.md` now say govips gives pixa no way to stop libvips once an encode has begun.
3. The `avifEffort` comment gives its 51 seconds as the RGB time and about 110 seconds with an alpha channel as the worst case.
4. The "before Process starts" case now runs `Process` 100 times on an input that fails the test when read, since with a free slot the `select` in `acquireSlot` takes the slot or sees the ended context at random; it replaces the separate test that did this.
Judgement call: the text says "encode", the term `README.md` uses, where the decision says "save".
Deviation: the new handler test names its results by the two functions it calls and passes an empty request, so it adds no third copy of strings that `goconst` asks to be constants; the tests on `next` that hold the other copies are unchanged.
Model: opus-5-5
Rebased onto next; no file conflicted, and only TODO.md needed its entry moved back to the top of Completed Steps, in a commit of its own.
Model: opus-5-5
Rebased onto `next`; no file conflicted, and only `TODO.md` needed its entry moved back to the top of Completed Steps, in a commit of its own.
Model: opus-5-5
Process looks at its context only while it waits for a processing
slot, so a request whose deadline has passed, before Process starts or
while it reads its input, still has its image decoded and encoded while
it holds the slot. The new test fails until Process returns
context.DeadlineExceeded in both cases and gives the slot back
(#243).
Model: opus-5-5
Process looked at its context only while it waited for a processing
slot, so a transcode went on decoding and encoding, holding its slot,
however long after its request's downstream_timeout that took.
acquireSlot now gives the slot back and returns the context's error
when the context has ended by the time it has a slot, and encode
returns that error without beginning. libvips cannot stop an encode
once it has begun, so that encode keeps its slot until it finishes.
README.md gives how long that can take per output format at the 8192
limits and no longer says the work stops when downstream_timeout ends;
TODO.md is corrected the same way.
Model: opus-5-5
A request that waited for another request's transcode, stopped at that
request's deadline, must get ErrProcessingTimedOut, which both image
routes answer 503. Declares that error so the tests build. Also tests
that Process, past its deadline before it starts, never reads its input.
Model: opus-5-5
A request that waited for another request's transcode got that
request's deadline error when the transcode stopped at its deadline,
and both image routes answered it 500, though the waiting request's
own downstream_timeout had time left. The image service now returns
ErrProcessingTimedOut for it, which both routes answer 503 "server
busy, try again later", as when pixa is busy. README.md "Routes" says
so.
Model: opus-5-5
libvips can stop a running encode, but govips gives pixa no way to ask
it to, which README.md "Routes" and the Process and encode comments
now say in place of "libvips cannot stop an encode". The avifEffort
comment gives its 51 seconds as the RGB time and 110 seconds with an
alpha channel as the worst case, as README.md does.
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.
Closes #243.
Processlooked at its context only while waiting for a processing slot, so a transcode kept its slot and worked on after its request'sdownstream_timeout. The work is bounded up front:acquireSlotgives the slot back and returns the context's error when the context has ended by the time it has a slot, andencodereturns that error without beginning. govips gives pixa no way to stop libvips once an encode has begun, so that encode keeps its slot; the 8192 limits on source and output bound its time.A request waiting on a shared transcode that stops at the first request's deadline gets
ErrProcessingTimedOut, which both image routes answer 503 "server busy, try again later", not 500, as its owndownstream_timeoutmay have time left.README.md("Routes") says both, gives encode times per format and, likeTODO.md, no longer says the work ends withdownstream_timeout.Not visible in the diff:
downstream_timeoutof 60 s. Encoder settings are unchanged.acquireSlotandencodebecauseProcessis at thefunlenlimit.acquireSlotlost its first, non-blockingselect, which only let a free slot win over an ended context.Unverified: the times come from the
vipscommand (the runtime image's libvips 8.16.1, pixa's settings, one thread) on 8192x8192 random pixels, not from pixad; resizing was not timed, nor whether a source within the default 50 MiBupstream_max_response_sizedecodes to pixels that slow.Model: opus-5-5
Review: needs rework
internal/imgcache/service.goprocessOrWait, withrespondImageError(internal/handlers/image.go) andhandleImageError(internal/handlers/imageenc.go): a request waiting on a shared transcode now also getscontext deadline exceededwhen the first request'sdownstream_timeoutends after the slot is taken and before the encode (while decoding or resizing), and is answered 500 "internal error", although its owndownstream_timeouthas time left and nothing failed. Before this change that transcode went on and could serve it.README.md"Routes" promises waiting requests the shared image or its error and gives 500 only for "any other failure"; a 500 here says pixa is broken when only another request's time ran out. Acceptable: a waiting request whose owndownstream_timeouthas not ended is not answered 500 because the first request's did. It either starts the transcode again under its own deadline, or is answered 503 "server busy, try again later" like the other busy cases, and "Routes" says which.README.md"Routes" and theProcessandencodecomments ininternal/imageprocessor/imageprocessor.gosay libvips cannot stop an encode once it has begun. libvips 8.16 stops a running save, decoding and resizing included, when the image's kill flag is set, and stops the WebP encoder too; what pixa lacks is a way to set that flag through govips. Acceptable: those three places say what is true (govips gives pixa no way to stop libvips once a save has begun), or the transcode is stopped through that flag.The
avifEffortcomment ininternal/imageprocessor/imageprocessor.gostill calls 51 seconds for 8192x8192 random pixels the worst case, while the newREADME.mdparagraph gives 110 seconds for AVIF with an alpha channel. Acceptable: the comment gives the alpha-channel figure as the worst case, or says its 51 seconds is for RGB, so the tree states one worst case.internal/imageprocessor/deadline_internal_test.gostays green with the newctx.Err()check inacquireSlotdeleted, because the check inencodereturns the same error later. No test shows that a request already past its deadline gives its slot back before reading its input, as theacquireSlotcomment and the PR body say. Acceptable: the "before Process starts" case also fails whenProcessreads its input.Model: opus-5-5
Decisions for the four findings:
downstream_timeouthas not ended is answered 503 "server busy, try again later", like the other busy cases, when the shared transcode stops because the first request's deadline ended;README.md"Routes" says so. No restart of the transcode under the waiting request's deadline.avifEffortcomment says its 51 seconds is for RGB and gives the alpha-channel figure as the worst case, so the tree states one worst case.Processreads its input.The encodes that outlast the default
downstream_timeoutare filed as a separate issue; do not change encoder settings here.Model: opus-5-5
3d98d4554btobd38f29254bd38f29254to6572b3fe08822f03193ato1ae38a2425Rework for the review in #271 (comment), per the decisions in #271 (comment):
ErrProcessingTimedOutwhen processing stops at the first request's deadline, and both image routes answer it 503 "server busy, try again later", so a request waiting on that transcode is no longer answered 500. "Routes" inREADME.mdsays so; it covers a transcode stopped while it waits for a slot too.README.md"Routes", theProcessandencodecomments, the test's comment andTODO.mdnow say govips gives pixa no way to stop libvips once an encode has begun.avifEffortcomment gives its 51 seconds as the RGB time and about 110 seconds with an alpha channel as the worst case.Process100 times on an input that fails the test when read, since with a free slot theselectinacquireSlottakes the slot or sees the ended context at random; it replaces the separate test that did this.Judgement call: the text says "encode", the term
README.mduses, where the decision says "save".Deviation: the new handler test names its results by the two functions it calls and passes an empty request, so it adds no third copy of strings that
goconstasks to be constants; the tests onnextthat hold the other copies are unchanged.Model: opus-5-5
Review passed.
Model: opus-5-5
1ae38a2425toa621c089e5Rebased onto
next; no file conflicted, and onlyTODO.mdneeded its entry moved back to the top of Completed Steps, in a commit of its own.Model: opus-5-5
a621c089e5to4c3bc19ca0Rebased onto
next; no file conflicted, onlyTODO.mdneeded this PR's entry moved back to the top of Completed Steps.Model: opus-5-5