cache: a multi tier error discards a successful hit from a later tier and 500s the request

[Michad]

When a multi cache has a tier that errors and a later tier that hits, the tile is found successfully and then thrown away, and the user gets an error response instead.

Multi.Lookup is the one cache that can return a non-nil image together with a non-nil error. internal/caches/multi.go:78-94:

for _, cache := range c.Tiers {
	img, err := cache.Lookup(ctx, t)
	if err != nil {
		allErrors = errors.Join(allErrors, err)
	}

	if img != nil {
		return img, allErrors
	}
}

RenderTile forwards the pair verbatim on a hit, pkg/entities/layer/layergroup.go:277-286:

img, err = l.Cache.Lookup(ctx, cacheTileRequest)

if img != nil {
	slog.DebugContext(ctx, "Cache hit")
	lg.cacheHitCounter.Add(ctx, 1)
	if cached, ok := pkg.CachedFromContext(ctx); ok && cached != nil {
		*cached = true
	}
	return img, err
}

and the handler checks the error before it ever looks at the image, internal/server/tile_handler.go:205-215:

img, err := cur.layerGroup().RenderTile(ctx, tileReq)

if err != nil {
	h.tileErrorCounter.Add(ctx, 1)
	span.RecordError(err)
	span.SetStatus(codes.Error, "Rendering error")
	writeError(ctx, w, &cur.errCfg, err, dataType)
	return
}

if img == nil {

With cache: {name: multi, tiers: [redis, disk]}, any redis blip (timeout, connection refused, auth failure) while the tile is present on disk means tier 1 records an error, tier 2 returns the tile, and every request for that tile gets an error response until redis recovers.

This defeats the stated purpose of the tiered cache. docs/operation/modules/ROOT/pages/configuration/cache/multi.adoc describes lookup as:

When looking up cache entries each cache is tried in order.

Falling through to the next tier is the whole point, and a degraded fast tier is the case the fallthrough exists for.

Note the metrics disagree with what the user receives: cacheHitCounter is incremented and the cached context flag is set to true before the error is returned, so telemetry records a cache hit for a request that ends in a 500. tileErrorCounter is also incremented, so a single request counts as both.

The Cache interface in pkg/entities/cache/cache.go:28-33 documents the bool on Remove ("a miss is not an error") but says nothing about Lookup returning a non-nil image alongside an error, so the current behaviour is unspecified rather than intended. multi_test.go has no case covering degraded-tier-plus-hit.

Two candidate fixes:

• Drop (and log) the error at layergroup.go:285 when img != nil. Keeps the multi tier's accumulated errors available for logging without failing the request.
• Have Multi.Lookup return img, nil on a hit, and log the tier errors internally. Keeps the odd pair from escaping the cache layer at all.

The second is probably the better boundary, but the first also protects against any future cache that returns the same pair. A regression test for the degraded-tier-plus-hit case should come with it, and the hit/error counter double-count is worth resolving at the same time.