Serve a cached variant while its source URL is in the negative cache (closes #245) #268

Merged
clawbot merged 4 commits from issue-245-serve-cached-before-negative-cache into next 2026-10-15 13:45:26 +02:00
4 changed files with 99 additions and 19 deletions
+4 -3
View File
@@ -200,9 +200,10 @@ path under `/v1/` answers 200, in maintenance mode too.
`image_requests_per_minute` (below); 415 when the upstream answers with a `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 `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 an image of
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other the same source URL that pixa can neither serve from its cache nor make from
failure. 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 - `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
`If-None-Match` matches the image's `ETag`; 400 for a token that does not `If-None-Match` matches the image's `ETag`; 400 for a token that does not
+10
View File
@@ -30,6 +30,16 @@ 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 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 - 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, needs a signature (closes #246): with `cdn.example.com` allowlisted,
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
@@ -94,6 +94,71 @@ func TestNegativeCache_Expired(t *testing.T) {
} }
} }
// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
// the test photo, then keeps a failed fetch of the photo's URL in the negative
// cache, as a failed fetch for another variant does once the source has been
// evicted. The cached variant is still served, and a variant not in the cache
// is made from the cached source. Once the source is evicted, a variant not in
// the cache needs a fetch and is refused.
func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
_ = resp.Content.Close()
err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
httpStatusBadGateway, "upstream answered 503")
if err != nil {
t.Fatalf("StoreNegative() error = %v", err)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
if err != nil {
t.Fatalf("Get() of the cached variant error = %v", err)
}
_ = resp.Content.Close()
if resp.CacheStatus != CacheHit {
t.Errorf("cached variant CacheStatus = %q, want %q",
resp.CacheStatus, CacheHit)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
t.Logf("Get() of a variant made from the cached source: error = %v", err)
if err != nil {
t.Fatalf("Get() of a variant made from the cached source error = %v", err)
}
_ = resp.Content.Close()
contentHash, _, err := svc.cache.LookupSource(t.Context(),
photoVariant(fixtures, 85, FitCover))
if err != nil || contentHash == "" {
t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
}
err = svc.cache.evictSourceBlob(t.Context(), contentHash)
if err != nil {
t.Fatalf("evictSourceBlob() error = %v", err)
}
_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
t.Logf("Get() of a variant whose source was evicted: error = %v", err)
if !errors.Is(err, ErrNegativeCached) {
t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
err, ErrNegativeCached)
}
}
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) { func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
t.Parallel() t.Parallel()
+20 -16
View File
@@ -130,25 +130,13 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
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.
// 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) { 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 {
@@ -433,12 +421,28 @@ func (s *Service) processFromSourceOrFetch(
// fetchAndProcess fetches from upstream, processes, and caches the result. // fetchAndProcess fetches from upstream, processes, and caches the result.
// It counts the fetch with the bytes read from upstream, including when // 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( func (s *Service) fetchAndProcess(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*processedVariant, error) { ) (*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 // Fetch from upstream
sourceURL := req.SourceURL() sourceURL := req.SourceURL()