Check the negative cache only before a fetch from the upstream (closes #245)
check / check (push) Waiting to run
check / check (push) Waiting to run
The check moves from Get to the start of fetchAndProcess, so a variant not in the cache is still made from a cached source while the URL is in the negative cache. A refusal now counts as a miss. Model: opus-5-5
This commit is contained in:
@@ -200,10 +200,10 @@ path under `/v1/` answers 200, in maintenance mode too.
|
||||
`image_requests_per_minute` (below); 415 when the upstream answers with a
|
||||
`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
|
||||
answered with an error status, and for 5 minutes after that for the same
|
||||
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.
|
||||
answered with an error status, and for 5 minutes after that for an image of
|
||||
the same source URL that pixa can neither serve from its cache nor make from
|
||||
the source image still in its cache; 503 when pixa is busy or in maintenance
|
||||
mode; 500 for any other failure.
|
||||
- `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
|
||||
`If-None-Match` matches the image's `ETag`; 400 for a token that does not
|
||||
|
||||
@@ -33,10 +33,13 @@ P2: security: per-origin rate limiting
|
||||
- 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/`.
|
||||
misses after the source was evicted and the upstream is down, every variant of
|
||||
that source answered 502 for 5 minutes, also one on disk or one that could be
|
||||
made from the cached source. The negative cache is now checked only just
|
||||
before a fetch from the upstream, so a cached variant is served and a variant
|
||||
not in the cache is made from the cached source when there is one. `README.md`
|
||||
says so under `/v1/image/`. A refusal from the negative cache now counts as a
|
||||
miss, as any other request that misses the variant cache does.
|
||||
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
|
||||
needs a signature (closes #246): with `cdn.example.com` allowlisted,
|
||||
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
|
||||
|
||||
@@ -129,9 +129,10 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
||||
// ErrNegativeCached is returned when a URL is in the 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. A
|
||||
// cached variant is served even while its source URL is in the negative
|
||||
// cache, which refuses only a variant that is not cached.
|
||||
// Get retrieves a processed image, fetching and processing if necessary.
|
||||
// While its source URL is in the negative cache, a cached variant is still
|
||||
// served and a variant not in the cache is still made from a cached source;
|
||||
// only a request that needs the source fetched is refused.
|
||||
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
|
||||
// Propagate AllowHTTP setting to the request
|
||||
req.AllowHTTP = s.allowHTTP
|
||||
@@ -162,21 +163,6 @@ 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
|
||||
// miss it at the same time, then count this request's miss, also when it
|
||||
// failed or the request context has ended meanwhile
|
||||
@@ -435,12 +421,28 @@ func (s *Service) processFromSourceOrFetch(
|
||||
|
||||
// fetchAndProcess fetches from upstream, processes, and caches the result.
|
||||
// It counts the fetch with the bytes read from upstream, including when
|
||||
// reading the response or a later step fails.
|
||||
// reading the response or a later step fails. It fetches nothing while the
|
||||
// source URL is in the negative cache.
|
||||
func (s *Service) fetchAndProcess(
|
||||
ctx context.Context,
|
||||
req *ImageRequest,
|
||||
cacheKey VariantKey,
|
||||
) (*processedVariant, error) {
|
||||
// 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)
|
||||
}
|
||||
|
||||
// Fetch from upstream
|
||||
sourceURL := req.SourceURL()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user