1 Commits
Author SHA1 Message Date
sneak 410c167c70 Keep counts not written by the request's deadline in memory (closes #224)
check / check (push) Waiting to run
TestService_Get_ReturnsByItsDeadline failed on a busy host because its
request, past its deadline, still waited to write its miss count, a write
with no deadline; in pixad that write waits for the one database
connection every request shares. Each count write now keeps the request's
deadline but not its cancellation. A count not written by then is kept in
memory, where Stats includes it, and one goroutine of the cache writes
those counts one UPDATE at a time, and once more at shutdown before the
database closes. The test phase also runs go test with -parallel 4: on a
busy host, as many tests at once as there are CPUs wait so long to be
scheduled that a timed request can fail.

Model: opus-5-5
2026-10-08 15:44:39 +00:00
8 changed files with 584 additions and 29 deletions
+6 -3
View File
@@ -32,10 +32,13 @@ RUN script/bootstrap --cgo
COPY . . COPY . .
# Without -v first; on a failure, again with -v for the details, and # Without -v first; on a failure, again with -v for the details, and
# the step fails even if the second run passes. # the step fails even if the second run passes. -parallel 4: by default
RUN go test -count=1 -timeout 90s -race -cover ./... || \ # a package runs as many of its tests at once as the host has CPUs, and
# on a busy host they then wait so long to be scheduled that a test
# that times a request can see it return a second late.
RUN go test -count=1 -timeout 90s -race -parallel 4 -cover ./... || \
{ echo "--- Rerunning with -v for details ---"; \ { echo "--- Rerunning with -v for details ---"; \
go test -count=1 -timeout 90s -race -v ./...; exit 1; } go test -count=1 -timeout 90s -race -parallel 4 -v ./...; exit 1; }
# Build stage. Nothing is wanted from the two phases above: these copies # Build stage. Nothing is wanted from the two phases above: these copies
# make BuildKit build them first, so this stage runs only when lint and # make BuildKit build them first, so this stage runs only when lint and
+6 -4
View File
@@ -83,10 +83,12 @@ which answers 200 whenever pixa is running, in maintenance mode too (see
`maintenance_mode`). `maintenance_mode`).
On SIGTERM or SIGINT pixa stops accepting connections, gives the requests in On SIGTERM or SIGINT pixa stops accepting connections, gives the requests in
progress and the images being processed 5 seconds to finish, and exits: with 0, progress and the images being processed 5 seconds to finish, then writes to the
or with 1 when images were still being processed after those 5 seconds or database the counts of cache hits, misses, fetches and conversions that requests
another part of pixa failed to stop. A request not finished by then is cut off. could not write by their deadline, and exits: with 0, or with 1 when images were
`docker stop` waits 10 seconds before it kills the container. still being processed after those 5 seconds, some of those counts could not be
written, or another part of pixa failed to stop. A request not finished by then
is cut off. `docker stop` waits 10 seconds before it kills the container.
Outside Docker, pixa needs libvips (the image has 8.16) and libheif to run, as Outside Docker, pixa needs libvips (the image has 8.16) and libheif to run, as
it uses libvips through CGO. pixad does not start unless libvips has its JPEG XL it uses libvips through CGO. pixad does not start unless libvips has its JPEG XL
+13
View File
@@ -30,6 +30,19 @@ P2: security: per-IP rate limiting on the image routes
# Completed Steps # Completed Steps
- 2026-10-08 a request past its deadline no longer waits for the database to
count it (closes #224). On a busy host, `TestService_Get_ReturnsByItsDeadline`
failed because its request, past its deadline, still waited for its miss count
to be written, a write with no deadline, which in pixad waits for the one
database connection every request shares. Each count write now keeps the
request's deadline but not its cancellation. A count not written by then is
kept in memory, where `Stats` sees it, and one goroutine of the cache writes
those counts, one write at a time, and once more at shutdown before the
database closes. The test phase of the `Dockerfile` also runs at most 4 tests
of a package at once (`go test -parallel 4`), where it ran as many as the host
has CPUs: on a busy host they then wait so long to be scheduled that a test
that times a request can fail. `-p`, how many packages are tested at once, is
unchanged, as capping it also slows compiling.
- 2026-10-08 every output format is saved with settings pixa sets on purpose - 2026-10-08 every output format is saved with settings pixa sets on purpose
(closes #232): each format has its own govips export, as JPEG XL does, in (closes #232): each format has its own govips export, as JPEG XL does, in
place of govips' generic `Export`, which sent libvips a zero for some settings place of govips' generic `Export`, which sent libvips a zero for some settings
+11 -2
View File
@@ -4,6 +4,7 @@ package handlers
import ( import (
"context" "context"
"encoding/json" "encoding/json"
"errors"
"log/slog" "log/slog"
"net/http" "net/http"
"time" "time"
@@ -71,16 +72,21 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
} }
lc.Append(fx.Hook{ lc.Append(fx.Hook{
//nolint:contextcheck // the eviction loop outlives OnStart; OnStop cancels it //nolint:contextcheck // the cache's goroutines outlive OnStart; OnStop stops them
OnStart: func(_ context.Context) error { OnStart: func(_ context.Context) error {
return s.initImageService() return s.initImageService()
}, },
// The pending counts are written here, before the database's stop
// hook closes it.
OnStop: func(ctx context.Context) error { OnStop: func(ctx context.Context) error {
if s.imgCache == nil { if s.imgCache == nil {
return nil return nil
} }
return s.imgCache.StopEviction(ctx) return errors.Join(
s.imgCache.StopEviction(ctx),
s.imgCache.StopPendingCountWrites(ctx),
)
}, },
}) })
@@ -122,6 +128,9 @@ func (s *Handlers) initImageService() error {
// write-pressure passes. No-op when the disk cache is disabled. // write-pressure passes. No-op when the disk cache is disabled.
cache.StartEviction(imgcache.DefaultEvictionInterval) cache.StartEviction(imgcache.DefaultEvictionInterval)
// Writes the counts requests could not write by their deadline
cache.StartPendingCountWrites()
// Create the fetcher config // Create the fetcher config
fetcherCfg := httpfetcher.DefaultConfig() fetcherCfg := httpfetcher.DefaultConfig()
fetcherCfg.AllowHTTP = s.config.AllowHTTP fetcherCfg.AllowHTTP = s.config.AllowHTTP
+71 -8
View File
@@ -11,6 +11,8 @@ import (
"io" "io"
"log/slog" "log/slog"
"path/filepath" "path/filepath"
"sync"
"sync/atomic"
"time" "time"
lru "github.com/hashicorp/golang-lru/v2" lru "github.com/hashicorp/golang-lru/v2"
@@ -79,6 +81,26 @@ type Cache struct {
evictionDone chan struct{} evictionDone chan struct{}
evictionCancel context.CancelFunc evictionCancel context.CancelFunc
// The pending counts: hits, misses, upstream fetches and transforms
// whose write to the database missed its request's deadline, kept
// here until they are written by the goroutine that
// StartPendingCountWrites starts. As for eviction, the channels are
// created in NewCache: pendingCountsAdded wakes that goroutine, and
// pendingCountsDone is closed when it returns. pendingCountsCancel,
// set by StartPendingCountWrites, cancels its context, which tells it
// to finish.
pendingHits atomic.Int64
pendingMisses atomic.Int64
pendingUpstreamFetches atomic.Int64
pendingUpstreamFetchBytes atomic.Int64
pendingTransforms atomic.Int64
pendingCountsAdded chan struct{}
pendingCountsDone chan struct{}
pendingCountsCancel context.CancelFunc
// Held through each write of the pending counts, and by Stats to read them
pendingCountsWriteMutex sync.Mutex
// metaCache holds the content types of the variants most recently // metaCache holds the content types of the variants most recently
// stored or served, so a hit does not read the variant's .meta file. // stored or served, so a hit does not read the variant's .meta file.
// It never stands in for the variant file, which is always opened. // It never stands in for the variant file, which is always opened.
@@ -139,6 +161,9 @@ func newCache(
metaCache: metaCache, metaCache: metaCache,
contentLocks: newContentLock(), contentLocks: newContentLock(),
pendingCountsAdded: make(chan struct{}, 1),
pendingCountsDone: make(chan struct{}),
reconciliationPageSize: defaultReconciliationPageSize, reconciliationPageSize: defaultReconciliationPageSize,
} }
@@ -474,12 +499,21 @@ func (c *Cache) CleanExpired(ctx context.Context) error {
func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) { func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
var stats CacheStats var stats CacheStats
// So that no write of the pending counts falls between the two reads
c.pendingCountsWriteMutex.Lock()
// Fetch hit/miss counts from the stats table // Fetch hit/miss counts from the stats table
err := c.db.QueryRowContext(ctx, ` err := c.db.QueryRowContext(ctx, `
SELECT hit_count, miss_count SELECT hit_count, miss_count
FROM cache_stats WHERE id = 1 FROM cache_stats WHERE id = 1
`).Scan(&stats.HitCount, &stats.MissCount) `).Scan(&stats.HitCount, &stats.MissCount)
// Hits and misses not yet written count too (see StartPendingCountWrites)
stats.HitCount += c.pendingHits.Load()
stats.MissCount += c.pendingMisses.Load()
c.pendingCountsWriteMutex.Unlock()
if err != nil && !errors.Is(err, sql.ErrNoRows) { if err != nil && !errors.Is(err, sql.ErrNoRows) {
return nil, fmt.Errorf("failed to get cache stats: %w", err) return nil, fmt.Errorf("failed to get cache stats: %w", err)
} }
@@ -511,19 +545,30 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
} }
// IncrementStats counts a cache hit or miss, and an upstream fetch that read // IncrementStats counts a cache hit or miss, and an upstream fetch that read
// fetchBytes bytes, as IncrementUpstreamFetch does. // fetchBytes bytes, as IncrementUpstreamFetch does. Like the other Increment
// methods, it writes to the database with ctx's deadline but not its
// cancellation, so an ended request is still counted but does not wait for
// the database past its deadline; a count not written by then becomes a
// pending count (see StartPendingCountWrites).
func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) { func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
countCtx, cancel := withoutCancelKeepingDeadline(ctx)
defer cancel()
var err error var err error
pendingCount := &c.pendingMisses
if hit { if hit {
_, err = c.db.ExecContext(ctx, ` pendingCount = &c.pendingHits
_, err = c.db.ExecContext(countCtx, `
UPDATE cache_stats UPDATE cache_stats
SET hit_count = hit_count + 1, SET hit_count = hit_count + 1,
last_updated_at = CURRENT_TIMESTAMP last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1 WHERE id = 1
`) `)
} else { } else {
_, err = c.db.ExecContext(ctx, ` _, err = c.db.ExecContext(countCtx, `
UPDATE cache_stats UPDATE cache_stats
SET miss_count = miss_count + 1, SET miss_count = miss_count + 1,
last_updated_at = CURRENT_TIMESTAMP last_updated_at = CURRENT_TIMESTAMP
@@ -531,7 +576,10 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
`) `)
} }
if err != nil { switch {
case errors.Is(err, context.DeadlineExceeded):
c.addPendingCount(pendingCount, 1)
case err != nil:
c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err) c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err)
} }
@@ -545,14 +593,22 @@ func (c *Cache) IncrementUpstreamFetch(ctx context.Context, fetchBytes int64) {
return return
} }
_, err := c.db.ExecContext(ctx, ` countCtx, cancel := withoutCancelKeepingDeadline(ctx)
defer cancel()
_, err := c.db.ExecContext(countCtx, `
UPDATE cache_stats UPDATE cache_stats
SET upstream_fetch_count = upstream_fetch_count + 1, SET upstream_fetch_count = upstream_fetch_count + 1,
upstream_fetch_bytes = upstream_fetch_bytes + ?, upstream_fetch_bytes = upstream_fetch_bytes + ?,
last_updated_at = CURRENT_TIMESTAMP last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1 WHERE id = 1
`, fetchBytes) `, fetchBytes)
if err != nil {
switch {
case errors.Is(err, context.DeadlineExceeded):
c.addPendingCount(&c.pendingUpstreamFetches, 1)
c.addPendingCount(&c.pendingUpstreamFetchBytes, fetchBytes)
case err != nil:
c.log.Warn("failed to count upstream fetch", c.log.Warn("failed to count upstream fetch",
"fetch_bytes", fetchBytes, "error", err) "fetch_bytes", fetchBytes, "error", err)
} }
@@ -560,13 +616,20 @@ func (c *Cache) IncrementUpstreamFetch(ctx context.Context, fetchBytes int64) {
// IncrementTransformCount counts one image transcoded by the image processor. // IncrementTransformCount counts one image transcoded by the image processor.
func (c *Cache) IncrementTransformCount(ctx context.Context) { func (c *Cache) IncrementTransformCount(ctx context.Context) {
_, err := c.db.ExecContext(ctx, ` countCtx, cancel := withoutCancelKeepingDeadline(ctx)
defer cancel()
_, err := c.db.ExecContext(countCtx, `
UPDATE cache_stats UPDATE cache_stats
SET transform_count = transform_count + 1, SET transform_count = transform_count + 1,
last_updated_at = CURRENT_TIMESTAMP last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1 WHERE id = 1
`) `)
if err != nil {
switch {
case errors.Is(err, context.DeadlineExceeded):
c.addPendingCount(&c.pendingTransforms, 1)
case err != nil:
c.log.Warn("failed to count transform", "error", err) c.log.Warn("failed to count transform", "error", err)
} }
} }
+141
View File
@@ -0,0 +1,141 @@
package imgcache
import (
"context"
"fmt"
"sync/atomic"
)
// StartPendingCountWrites starts the goroutine that writes the pending
// counts to the database whenever there are some, one UPDATE at a time:
// however many requests pass their deadline before their counts are
// written, at most this one write waits for the database. It is a no-op
// when already started. The goroutine outlives the caller, so it runs with
// its own context, which StopPendingCountWrites cancels to tell it to
// finish.
func (c *Cache) StartPendingCountWrites() {
if c.pendingCountsCancel != nil {
return
}
ctx, cancel := context.WithCancel(context.Background())
c.pendingCountsCancel = cancel
go func() {
c.pendingCountWriteLoop(ctx)
}()
}
// StopPendingCountWrites tells the goroutine StartPendingCountWrites
// started, if it did, to finish, and waits for its write under way to end
// and for it to return. It then writes the pending counts left, so that
// they are written before the database closes. It waits for all of this at
// most until ctx ends, then logs the counts not written and returns an
// error.
func (c *Cache) StopPendingCountWrites(ctx context.Context) error {
var err error
if c.pendingCountsCancel != nil {
c.pendingCountsCancel()
select {
case <-c.pendingCountsDone:
case <-ctx.Done():
// The write under way is left to finish: the database's
// close waits for a query under way.
err = fmt.Errorf("pending counts still being written: %w", ctx.Err())
}
}
if err == nil {
err = c.writePendingCounts(ctx)
}
if err != nil {
c.log.Error("counts not written at shutdown",
"hits", c.pendingHits.Load(),
"misses", c.pendingMisses.Load(),
"upstream_fetches", c.pendingUpstreamFetches.Load(),
"upstream_fetch_bytes", c.pendingUpstreamFetchBytes.Load(),
"transforms", c.pendingTransforms.Load(),
"error", err,
)
return err
}
return nil
}
// pendingCountWriteLoop is the body of the goroutine
// StartPendingCountWrites starts. It returns when ctx is cancelled, but
// does not cut off a write under way then. A write that fails leaves the
// counts pending, for the next write.
func (c *Cache) pendingCountWriteLoop(ctx context.Context) {
defer close(c.pendingCountsDone)
for {
select {
case <-ctx.Done():
return
case <-c.pendingCountsAdded:
}
err := c.writePendingCounts(context.WithoutCancel(ctx))
if err != nil {
c.log.Warn("failed to write pending counts", "error", err)
}
}
}
// addPendingCount adds n to pendingCount, one of the pending counts, and
// wakes the goroutine that writes them. pendingCountsAdded has capacity one,
// so a wakeup already waiting is enough.
func (c *Cache) addPendingCount(pendingCount *atomic.Int64, n int64) {
pendingCount.Add(n)
select {
case c.pendingCountsAdded <- struct{}{}:
default:
}
}
// writePendingCounts adds the pending counts to the cache_stats row in one
// UPDATE, then takes what it wrote out of them, leaving any added
// meanwhile. When the UPDATE fails, they all stay pending.
func (c *Cache) writePendingCounts(ctx context.Context) error {
c.pendingCountsWriteMutex.Lock()
defer c.pendingCountsWriteMutex.Unlock()
hits := c.pendingHits.Load()
misses := c.pendingMisses.Load()
upstreamFetches := c.pendingUpstreamFetches.Load()
upstreamFetchBytes := c.pendingUpstreamFetchBytes.Load()
transforms := c.pendingTransforms.Load()
if hits+misses+upstreamFetches+upstreamFetchBytes+transforms == 0 {
return nil
}
_, err := c.db.ExecContext(ctx, `
UPDATE cache_stats
SET hit_count = hit_count + ?,
miss_count = miss_count + ?,
upstream_fetch_count = upstream_fetch_count + ?,
upstream_fetch_bytes = upstream_fetch_bytes + ?,
transform_count = transform_count + ?,
last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1
`, hits, misses, upstreamFetches, upstreamFetchBytes, transforms)
if err != nil {
return fmt.Errorf("failed to write pending counts: %w", err)
}
c.pendingHits.Add(-hits)
c.pendingMisses.Add(-misses)
c.pendingUpstreamFetches.Add(-upstreamFetches)
c.pendingUpstreamFetchBytes.Add(-upstreamFetchBytes)
c.pendingTransforms.Add(-transforms)
return nil
}
@@ -0,0 +1,316 @@
package imgcache
import (
"context"
"database/sql"
"errors"
"testing"
"time"
)
// pendingCountsWait is how long a test waits for the database's connection,
// for a count to reach the database, for a write to start waiting for the
// connection, or for StopPendingCountWrites to return.
const pendingCountsWait = 5 * time.Second
// holdDatabase takes the one connection of cache's database, so that every
// other query waits for it, and returns the func that frees it.
func holdDatabase(t *testing.T, cache *Cache) func() {
t.Helper()
ctx, cancel := context.WithTimeout(t.Context(), pendingCountsWait)
defer cancel()
conn, err := cache.db.Conn(ctx)
if err != nil {
t.Fatalf("failed to take the database connection: %v", err)
}
return func() { _ = conn.Close() }
}
// waitForConnectionWaits waits until db.Stats().WaitCount, the number of
// times a caller has waited for a connection, reaches waits.
func waitForConnectionWaits(t *testing.T, db *sql.DB, waits int64) {
t.Helper()
deadline := time.Now().Add(pendingCountsWait)
for db.Stats().WaitCount < waits {
if time.Now().After(deadline) {
t.Fatalf("callers waited for the database connection %d times, want %d",
db.Stats().WaitCount, waits)
}
time.Sleep(10 * time.Millisecond)
}
}
// waitForCounters waits until the cache_stats row holds want.
func waitForCounters(t *testing.T, cache *Cache, want cacheStatsCounters) {
t.Helper()
deadline := time.Now().Add(pendingCountsWait)
for {
got := readCacheStatsCounters(t, cache)
if got == want {
return
}
if time.Now().After(deadline) {
t.Fatalf("counters = %+v, want %+v", got, want)
}
time.Sleep(10 * time.Millisecond)
}
}
// TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy holds the
// database's one connection while a request whose fetch is held reaches its
// deadline. The request must still return by its deadline with the
// deadline's error, and its miss must reach the database once the
// connection is free.
func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
t.Parallel()
svc, fixtures, fetcher := setupHeldFetchService(t)
svc.cache.StartPendingCountWrites()
defer func() { _ = svc.cache.StopPendingCountWrites(t.Context()) }()
// Room for the request to reach its fetch on a busy host
const timeout = 2 * time.Second
ctx, cancel := context.WithTimeout(t.Context(), timeout)
defer cancel()
deadline, _ := ctx.Deadline()
results := startGet(ctx, svc, photoVariant(fixtures, 85, FitCover))
// The request has made its database reads by the time it fetches.
select {
case <-fetcher.started:
case <-time.After(timeout):
t.Fatal("request did not reach its fetch by its deadline")
}
releaseDatabase := holdDatabase(t, svc.cache)
defer releaseDatabase()
select {
case got := <-results:
t.Logf("Get() returned %v after its deadline, error = %v",
time.Since(deadline), got.err)
if !errors.Is(got.err, context.DeadlineExceeded) {
t.Errorf("Get() error = %v, want %v", got.err, context.DeadlineExceeded)
}
case <-time.After(time.Until(deadline) + time.Second):
t.Fatal("request did not return by its deadline while the database was busy")
}
releaseDatabase()
waitForCounters(t, svc.cache, cacheStatsCounters{missCount: 1})
}
// TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting holds the
// database's one connection while many misses are counted past their
// deadline. At most one write may then wait for the connection, and every
// miss must reach the database once it is free.
func TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
cache.StartPendingCountWrites()
defer func() { _ = cache.StopPendingCountWrites(t.Context()) }()
releaseDatabase := holdDatabase(t, cache)
defer releaseDatabase()
waitsBefore := cache.db.Stats().WaitCount
// Writes given a context past its deadline fail at once.
ended, cancel := context.WithDeadline(t.Context(), time.Now())
defer cancel()
const misses = 20
for range misses {
cache.IncrementStats(ended, false, 0)
}
// Once one write waits for the connection, any others start waiting too.
waitForConnectionWaits(t, cache.db, waitsBefore+1)
time.Sleep(arrivalWait)
waits := cache.db.Stats().WaitCount - waitsBefore
t.Logf("%d writes waited for the database", waits)
if waits > 1 {
t.Errorf("%d writes waited for the database, want at most 1", waits)
}
releaseDatabase()
waitForCounters(t, cache, cacheStatsCounters{missCount: misses})
}
// TestStats_IncludesPendingCounts counts hits and misses whose writes miss
// their deadline, and checks that Stats adds them to the database's counts
// before they are written.
func TestStats_IncludesPendingCounts(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
_, err := cache.db.ExecContext(t.Context(),
`UPDATE cache_stats SET hit_count = 75, miss_count = 25 WHERE id = 1`)
if err != nil {
t.Fatal(err)
}
// Writes given a context past its deadline fail at once.
ended, cancel := context.WithDeadline(t.Context(), time.Now())
defer cancel()
cache.IncrementStats(ended, true, 0)
cache.IncrementStats(ended, true, 0)
cache.IncrementStats(ended, false, 0)
stats, err := cache.Stats(t.Context())
if err != nil {
t.Fatalf("Stats() error = %v", err)
}
if stats.HitCount != 77 || stats.MissCount != 26 {
t.Errorf("HitCount = %d, MissCount = %d, want 77 and 26",
stats.HitCount, stats.MissCount)
}
want := cacheStatsCounters{hitCount: 75, missCount: 25}
if got := readCacheStatsCounters(t, cache); got != want {
t.Errorf("counters in the database = %+v, want %+v", got, want)
}
}
// TestStopPendingCountWrites_WritesPendingCounts holds the database's one
// connection while the goroutine that writes the pending counts waits for it,
// counts more, then stops that goroutine. The stop must leave the write under
// way to finish rather than cut it off, and every count must reach the
// database once the connection is free.
func TestStopPendingCountWrites_WritesPendingCounts(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
cache.StartPendingCountWrites()
defer func() { _ = cache.StopPendingCountWrites(t.Context()) }()
releaseDatabase := holdDatabase(t, cache)
defer releaseDatabase()
waitsBefore := cache.db.Stats().WaitCount
// Writes given a context past its deadline fail at once.
ended, cancel := context.WithDeadline(t.Context(), time.Now())
defer cancel()
cache.IncrementStats(ended, true, 0)
// The goroutine's write waits for the connection.
waitForConnectionWaits(t, cache.db, waitsBefore+1)
// Counted after that write read the pending counts
cache.IncrementStats(ended, false, 0)
cache.IncrementUpstreamFetch(ended, 1024)
cache.IncrementTransformCount(ended)
stopped := make(chan error, 1)
go func() {
stopped <- cache.StopPendingCountWrites(t.Context())
}()
// Had the stop cut off the write under way, its own write would wait for
// the connection too.
time.Sleep(arrivalWait)
waits := cache.db.Stats().WaitCount - waitsBefore
t.Logf("%d writes waited for the database", waits)
if waits > 1 {
t.Errorf("%d writes waited for the database, want 1: "+
"the stop cut off the write under way", waits)
}
releaseDatabase()
select {
case err := <-stopped:
if err != nil {
t.Fatalf("StopPendingCountWrites() error = %v", err)
}
case <-time.After(pendingCountsWait):
t.Fatal("StopPendingCountWrites() did not return once the database was free")
}
want := cacheStatsCounters{1, 1, 1, 1024, 1}
if got := readCacheStatsCounters(t, cache); got != want {
t.Errorf("counters = %+v, want %+v", got, want)
}
}
// TestStopPendingCountWrites_ReturnsWhenItsContextEnds holds the database's
// one connection while the goroutine that writes the pending counts waits for
// it, then stops that goroutine with a context that ends first. The stop must
// return the context's error, and the write under way must still reach the
// database once the connection is free.
func TestStopPendingCountWrites_ReturnsWhenItsContextEnds(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
cache.StartPendingCountWrites()
defer func() { _ = cache.StopPendingCountWrites(t.Context()) }()
releaseDatabase := holdDatabase(t, cache)
defer releaseDatabase()
waitsBefore := cache.db.Stats().WaitCount
// Writes given a context past its deadline fail at once.
ended, cancel := context.WithDeadline(t.Context(), time.Now())
defer cancel()
cache.IncrementStats(ended, true, 0)
// The goroutine's write waits for the connection.
waitForConnectionWaits(t, cache.db, waitsBefore+1)
stopCtx, cancelStop := context.WithCancel(t.Context())
cancelStop()
stopped := make(chan error, 1)
go func() {
stopped <- cache.StopPendingCountWrites(stopCtx)
}()
select {
case err := <-stopped:
if !errors.Is(err, context.Canceled) {
t.Errorf("StopPendingCountWrites() error = %v, want %v",
err, context.Canceled)
}
case <-time.After(pendingCountsWait):
t.Fatal("StopPendingCountWrites() did not return once its context ended")
}
releaseDatabase()
waitForCounters(t, cache, cacheStatsCounters{hitCount: 1})
}
+19 -11
View File
@@ -162,7 +162,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
// Fall through to re-process // Fall through to re-process
} else { } else {
// Counted also when the request context has ended meanwhile // Counted also when the request context has ended meanwhile
s.cache.IncrementStats(context.WithoutCancel(ctx), true, 0) s.cache.IncrementStats(ctx, true, 0)
return &ImageResponse{ return &ImageResponse{
Content: reader, Content: reader,
@@ -179,7 +179,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
// failed or the request context has ended meanwhile // failed or the request context has ended meanwhile
response, err := s.processOrWait(ctx, req) response, err := s.processOrWait(ctx, req)
s.cache.IncrementStats(context.WithoutCancel(ctx), false, 0) s.cache.IncrementStats(ctx, false, 0)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -300,14 +300,8 @@ func (s *Service) processOrWait(
} }
}() }()
processingCtx := context.WithoutCancel(ctx) processingCtx, cancel := withoutCancelKeepingDeadline(ctx)
if deadline, ok := ctx.Deadline(); ok {
var cancel context.CancelFunc
processingCtx, cancel = context.WithDeadline(processingCtx, deadline)
defer cancel() defer cancel()
}
return s.processFromSourceOrFetch(processingCtx, req, cacheKey) return s.processFromSourceOrFetch(processingCtx, req, cacheKey)
}) })
@@ -344,6 +338,20 @@ func (s *Service) processOrWait(
}, nil }, nil
} }
// withoutCancelKeepingDeadline returns ctx without its cancellation but with
// its deadline, if it has one, and the func that releases the returned
// context.
func withoutCancelKeepingDeadline(
ctx context.Context,
) (context.Context, context.CancelFunc) {
deadline, hasDeadline := ctx.Deadline()
if !hasDeadline {
return context.WithoutCancel(ctx), func() {}
}
return context.WithDeadline(context.WithoutCancel(ctx), deadline)
}
// loadCachedSource opens source content from cache, without reading it, and // loadCachedSource opens source content from cache, without reading it, and
// returns it with its size; nil if the cached data is unavailable, empty or // returns it with its size; nil if the cached data is unavailable, empty or
// exceeds maxResponseSize. // exceeds maxResponseSize.
@@ -453,7 +461,7 @@ func (s *Service) fetchAndProcess(
fetchBytes := int64(len(sourceData)) fetchBytes := int64(len(sourceData))
// Counted also when the request context has ended meanwhile // Counted also when the request context has ended meanwhile
s.cache.IncrementUpstreamFetch(context.WithoutCancel(ctx), fetchBytes) s.cache.IncrementUpstreamFetch(ctx, fetchBytes)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read upstream response: %w", err) return nil, fmt.Errorf("failed to read upstream response: %w", err)
@@ -535,7 +543,7 @@ func (s *Service) processAndStore(
processDuration := time.Since(processStart) processDuration := time.Since(processStart)
// Counted also when the request context has ended meanwhile // Counted also when the request context has ended meanwhile
s.cache.IncrementTransformCount(context.WithoutCancel(ctx)) s.cache.IncrementTransformCount(ctx)
// Read processed content // Read processed content
processedData, err := io.ReadAll(processResult.Content) processedData, err := io.ReadAll(processResult.Content)