Serve a cached variant while its source URL is negative-cached (closes #245)
Get checked the negative cache before it looked for the variant on disk, so after one failed fetch of a source every cached variant of it answered 502 for 5 minutes. Get now serves a cached variant first and checks the negative cache only on a miss, before processing. The 502 line under /v1/image/ in README.md and TODO.md say so. Model: opus-5-5
This commit is contained in:
@@ -201,7 +201,8 @@ path under `/v1/` answers 200, in maintenance mode too.
|
|||||||
`Content-Type` that is not one of the source formats (below), or with an image
|
`Content-Type` that is not one of the source formats (below), or with an image
|
||||||
whose first bytes libvips reads as another format; 502 when the upstream
|
whose first bytes libvips reads as another format; 502 when the upstream
|
||||||
answered with an error status, and for 5 minutes after that for the same
|
answered with an error status, and for 5 minutes after that for the same
|
||||||
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
|
source URL, though an image of it that is already in the cache is still
|
||||||
|
served; 503 when pixa is busy or in maintenance mode; 500 for any other
|
||||||
failure.
|
failure.
|
||||||
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
|
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
|
||||||
(see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
|
(see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
|
||||||
|
|||||||
@@ -30,6 +30,13 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-15 a cached variant is served while its source URL is in the negative
|
||||||
|
cache (closes #245): `Get` checked the negative cache before the variant
|
||||||
|
cache, so once a fetch of the source failed, as one does when another variant
|
||||||
|
misses after the source was evicted and the upstream is down, every cached
|
||||||
|
variant of that source answered 502 for 5 minutes. `Get` now serves a cached
|
||||||
|
variant first and checks the negative cache only on a miss, before processing.
|
||||||
|
`README.md` says so under `/v1/image/`.
|
||||||
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
|
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
|
||||||
`github.com` and `user-images.githubusercontent.com`, where anyone can put an
|
`github.com` and `user-images.githubusercontent.com`, where anyone can put an
|
||||||
image, so a deployment set up as Getting Started says served those images to
|
image, so a deployment set up as Getting Started says served those images to
|
||||||
|
|||||||
@@ -128,26 +128,13 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
|||||||
// ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
|
// ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
|
||||||
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
||||||
|
|
||||||
// Get retrieves a processed image, fetching and processing if necessary.
|
// Get retrieves a processed image, fetching and processing if necessary. A
|
||||||
|
// cached variant is served even while its source URL is in the negative
|
||||||
|
// cache, which refuses only a variant that is not cached.
|
||||||
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
|
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
|
||||||
// Propagate AllowHTTP setting to the request
|
// Propagate AllowHTTP setting to the request
|
||||||
req.AllowHTTP = s.allowHTTP
|
req.AllowHTTP = s.allowHTTP
|
||||||
|
|
||||||
// Check negative cache first - skip fetching for recently-failed URLs
|
|
||||||
negHit, err := s.cache.checkNegativeCache(ctx, req)
|
|
||||||
if err != nil {
|
|
||||||
s.log.Warn("negative cache check failed", "error", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
if negHit {
|
|
||||||
s.log.Debug("negative cache hit",
|
|
||||||
"host", req.SourceHost,
|
|
||||||
"path", req.SourcePath,
|
|
||||||
)
|
|
||||||
|
|
||||||
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check variant cache first (disk only, no DB)
|
// Check variant cache first (disk only, no DB)
|
||||||
result, err := s.cache.Lookup(ctx, req)
|
result, err := s.cache.Lookup(ctx, req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -174,6 +161,21 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check negative cache - skip fetching for recently-failed URLs
|
||||||
|
negHit, err := s.cache.checkNegativeCache(ctx, req)
|
||||||
|
if err != nil {
|
||||||
|
s.log.Warn("negative cache check failed", "error", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if negHit {
|
||||||
|
s.log.Debug("negative cache hit",
|
||||||
|
"host", req.SourceHost,
|
||||||
|
"path", req.SourcePath,
|
||||||
|
)
|
||||||
|
|
||||||
|
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
|
||||||
|
}
|
||||||
|
|
||||||
// Cache miss - get the variant, processed once for all the requests that
|
// Cache miss - get the variant, processed once for all the requests that
|
||||||
// miss it at the same time, then count this request's miss, also when it
|
// miss it at the same time, then count this request's miss, also when it
|
||||||
// failed or the request context has ended meanwhile
|
// failed or the request context has ended meanwhile
|
||||||
|
|||||||
Reference in New Issue
Block a user