README.md

@@ -164,11 +164,13 @@ name.
 Multiple source paths may reference the same content blob; the database tracks
 references rather than using filesystem refcounting.
 
-Source and transformed image files are written under a temporary name, synced to
+Source and transformed image files, and the `.meta` file beside each transformed
-disk, and only then renamed into place, so a crash leaves no file under its own
+image, are written under a temporary name, synced to disk, and only then renamed
-name with part of its data. A request that finds a cached source or transformed
+into place, so a crash leaves no file under its own name with part of its data.
-image whose file is empty, or not the size the database records for it, removes
+When pixa starts, and every five minutes after, it removes each cached source or
-it and fetches the source or converts the image again.
+transformed image whose file is empty, or not the size the database records for
+it, with a warning naming the file; the next request for it fetches the source
+or converts the image again.
 
 pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
 Test). Toward it, pixa keeps in memory the content types of the 10,000

TODO.md

@@ -30,6 +30,18 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
+  again on every request (closes #252). Source and transformed image files, and
+  each transformed image's `.meta` file, are synced to disk before they are
+  renamed into place, and their directory after; the `.meta` file is written
+  before the image, and a failed directory sync removes the renamed file. The
+  reconciliation pass, at startup and every five minutes, removes a cached
+  source or transformed image whose file is empty, or not the size the database
+  records for it, with its rows and a warning naming the file, so the next
+  request fetches the source or converts the image again; requests compare no
+  sizes. Storing a source over a file of another size replaces that file. A
+  source that fails to decode is not removed for that alone: a damaged upstream
+  image would then be fetched on every request.
 - 2026-10-15 a transcode stops once its request's `downstream_timeout` has
   ended, unless it has begun encoding (closes #243): the image processor looked
   at its context only while it waited for a processing slot, so an image was
@@ -121,15 +133,6 @@ P2: security: per-origin rate limiting
   answered 500. Once the limit is used up, the fetcher now reads one more byte:
   if the body has ended it is accepted, and if there is more it is refused with
   `ErrResponseTooLarge`, as before.
-- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
-  again on every request (closes #252). Source and transformed image files are
-  synced to disk before they are renamed into place, and their directory after.
-  A request that finds a cached source or transformed image whose file is empty,
-  or not the size the database records for it, removes it with a log line naming
-  it, and fetches the source or converts the image again. Storing a source over
-  a file of another size replaces that file. A source that fails to decode is
-  not removed for that alone: a damaged upstream image would then be fetched on
-  every request.
 - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
   (closes #257): it scales the image to cover the requested size, and the side
   scaled past that size was held to no maximum, so `1x8192?fit=outside` of a

internal/imgcache/cache.go

@@ -229,13 +229,13 @@ type LookupResult struct {
 	CacheStatus CacheStatus
 }
 
-// Lookup checks if a processed variant is on disk whole (see
+// Lookup checks if a processed variant exists on disk. Hits touch the
-// variantFileIsWhole). Hits touch the variant's LRU timestamp; a disabled
+// variant's LRU timestamp; a disabled cache always misses.
-// cache always misses.
 func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) {
 	cacheKey := CacheKey(req)
 
-	if !c.disabled && c.variantFileIsWhole(ctx, cacheKey) {
+	// Check variant storage directly - no DB needed for cache hits
+	if !c.disabled && c.variants.Exists(cacheKey) {
 		c.touchVariant(ctx, cacheKey)
 
 		return &LookupResult{
@@ -400,11 +400,8 @@ func (c *Cache) StoreVariant(
 
 // LookupSource checks if we have cached source content for a request.
 // Returns the content hash and content type if found, or empty values
-// if not. A content file that is empty, or not the size its
+// if not. Hits touch the blob's LRU timestamp; a disabled cache always
-// source_content row records, is damaged: it is removed with its rows,
+// reports no cached source.
-// and no cached source is reported, so the source is fetched again. Hits
-// touch the blob's LRU timestamp; a disabled cache always reports no
-// cached source.
 func (c *Cache) LookupSource(
 	ctx context.Context, req *ImageRequest,
 ) (ContentHash, string, error) {
@@ -412,18 +409,12 @@ func (c *Cache) LookupSource(
 		return "", "", nil
 	}
 
-	var (
+	var hashStr, contentType string
-		hashStr, contentType string
-		recordedSize         int64
-	)
 
 	err := c.db.QueryRowContext(ctx, `
-		SELECT source_metadata.content_hash, source_metadata.content_type,
+		SELECT content_hash, content_type FROM source_metadata
-		       source_content.size_bytes
-		FROM source_metadata JOIN source_content USING (content_hash)
 		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.SourceHost, req.SourcePath, req.SourceQuery).
+	`, req.SourceHost, req.SourcePath, req.SourceQuery).Scan(&hashStr, &contentType)
-		Scan(&hashStr, &contentType, &recordedSize)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return "", "", nil
@@ -435,24 +426,8 @@ func (c *Cache) LookupSource(
 
 	contentHash := ContentHash(hashStr)
 
-	fileSize, err := c.srcContent.Size(contentHash)
+	// Verify the content file exists
-	if errors.Is(err, ErrNotFound) {
+	if !c.srcContent.Exists(contentHash) {
-		return "", "", nil
-	}
-
-	if err != nil {
-		return "", "", err
-	}
-
-	if fileSize == 0 || fileSize != recordedSize {
-		c.log.Warn("removing damaged cached source", "content_hash", contentHash,
-			"file_bytes", fileSize, "recorded_bytes", recordedSize)
-
-		err = c.evictSourceBlob(ctx, contentHash)
-		if err != nil {
-			return "", "", fmt.Errorf("failed to remove damaged source: %w", err)
-		}
-
 		return "", "", nil
 	}
 
@@ -685,10 +660,10 @@ func (c *Cache) readHitAndMissCounts(ctx context.Context, stats *CacheStats) err
 // hold: it reads the content type from the variant's .meta file and
 // keeps it in metaCache, unless a StoreVariant has put one there
 // meanwhile, as the store's is newer. A read that finds no .meta file,
-// as one can between a store's writing of the variant file and of its
+// as one can when the store's .meta write failed or an eviction removed
-// .meta file, serves application/octet-stream and keeps nothing, so
+// the file after the read opened the variant, serves
-// metaCache only ever holds a type read from a .meta file or passed to
+// application/octet-stream and keeps nothing, so metaCache only ever
-// StoreVariant.
+// holds a type read from a .meta file or passed to StoreVariant.
 func (c *Cache) loadVariantWithMeta(
 	cacheKey VariantKey,
 ) (io.ReadCloser, int64, string, error) {
@@ -736,43 +711,6 @@ func (c *Cache) writeMetadataSidecar(
 	}
 }
 
-// variantFileIsWhole reports whether the variant file of cacheKey is on
-// disk with the size its variant_content row records. A file that is
-// empty or of another size is damaged: it is removed with its row, so the
-// variant is converted again. A variant whose row cannot be read, as
-// between StoreVariant's writing of its file and of its row, is judged
-// by its file alone.
-func (c *Cache) variantFileIsWhole(ctx context.Context, cacheKey VariantKey) bool {
-	fileSize, err := c.variants.Size(cacheKey)
-	if err != nil {
-		return false
-	}
-
-	var recordedSize int64
-
-	err = c.db.QueryRowContext(ctx,
-		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`,
-		string(cacheKey)).Scan(&recordedSize)
-	if err != nil {
-		recordedSize = fileSize
-	}
-
-	if fileSize > 0 && fileSize == recordedSize {
-		return true
-	}
-
-	c.log.Warn("removing damaged cached variant", "cache_key", cacheKey,
-		"file_bytes", fileSize, "recorded_bytes", recordedSize)
-
-	err = c.evictVariant(ctx, cacheKey)
-	if err != nil {
-		c.log.Warn("failed to remove damaged cached variant",
-			"cache_key", cacheKey, "error", err)
-	}
-
-	return false
-}
-
 // touchVariant updates the LRU timestamp of a variant, best-effort:
 // a failed touch only makes the entry look colder to eviction.
 func (c *Cache) touchVariant(ctx context.Context, cacheKey VariantKey) {

internal/imgcache/eviction.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -560,10 +561,11 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 // (or whose accounting insert failed, e.g. StoreVariant's best-effort
 // insert under transient DB contention), drops accounting rows whose
 // files are missing, removes source blob files the database does not
-// know (and rows whose files are gone), sweeps stale temp files left
+// know (and rows whose files are gone), removes source and variant files
-// behind by crashed writes, and last checks the total cache usage
+// that are empty or not the size their rows record, sweeps stale temp
-// against the tables. It reads the tables a page at a time. Running it
+// files left behind by crashed writes, and last checks the total cache
-// periodically, not just once, bounds how long such drift can
+// usage against the tables. It reads the tables a page at a time.
+// Running it periodically, not just once, bounds how long such drift can
 // accumulate unaccounted for on a long-running process to one eviction
 // interval. Once ctx is cancelled, it stops at the next file or row and
 // returns ctx's error.
@@ -595,8 +597,9 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	return c.reconcileUsageTotal(ctx)
 }
 
-// reconcileVariantFiles walks the variant storage directory, adopting
+// reconcileVariantFiles walks the variant storage directory, checking
-// files without accounting rows and sweeping stale temp files.
+// each variant file against its accounting row (reconcileVariantFile)
+// and sweeping stale temp files.
 func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.variants.baseDir,
@@ -621,38 +624,55 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
+			return c.reconcileVariantFile(ctx, path, entry, VariantKey(name))
 		},
 	)
 }
 
-// adoptVariantFile inserts an accounting row for a variant file that
+// reconcileVariantFile checks one variant file against its accounting
-// has none, using the file's size and modification time.
+// row. A file without a row is adopted into the accounting, unless it is
-func (c *Cache) adoptVariantFile(
+// empty. A file that is empty, or not the size its row records, as a
+// crash or a restored snapshot can leave it, is removed with its row and
+// a warning naming it, so the variant is converted again.
+func (c *Cache) reconcileVariantFile(
 	ctx context.Context, path string, entry fs.DirEntry, cacheKey VariantKey,
 ) error {
-	var rowExists int
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, string(cacheKey),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check variant accounting row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
 	info, err := entry.Info()
 	if err != nil {
 		return fmt.Errorf("failed to stat variant file: %w", err)
 	}
 
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`, string(cacheKey),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows) && info.Size() > 0:
+		return c.adoptVariantFile(ctx, path, info, cacheKey)
+	case err != nil && !errors.Is(err, sql.ErrNoRows):
+		return fmt.Errorf("failed to check variant accounting row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached variant file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictVariant(ctx, cacheKey)
+}
+
+// adoptVariantFile inserts an accounting row for a variant file that
+// has none, using the file's size and modification time.
+func (c *Cache) adoptVariantFile(
+	ctx context.Context, path string, info fs.FileInfo, cacheKey VariantKey,
+) error {
 	modTime := info.ModTime().UTC().Format(sqliteTimestampLayout)
 	contentType := c.variantContentTypeFromSidecar(path)
 
-	_, err = c.db.ExecContext(ctx, `
+	_, err := c.db.ExecContext(ctx, `
 		INSERT INTO variant_content
 			(cache_key, size_bytes, content_type, created_at, last_accessed_at)
 		VALUES (?, ?, ?, ?, ?)
@@ -771,10 +791,9 @@ func queryStringColumn[T ~string](
 	return values, nil
 }
 
-// reconcileSourceFiles walks the source content directory, removing
+// reconcileSourceFiles walks the source content directory, checking
-// blob files the database does not track (they are unreachable: source
+// each blob file against its source_content row (reconcileSourceFile)
-// lookups always go through source_metadata) and sweeping stale temp
+// and sweeping stale temp files.
-// files.
 func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.srcContent.baseDir,
@@ -795,31 +814,55 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
+			return c.reconcileSourceFile(ctx, path, entry, ContentHash(name))
 		},
 	)
 }
 
+// reconcileSourceFile checks one source blob file against its
+// source_content row. A file without a row is removed: it is
+// unreachable, as source lookups always go through source_metadata. A
+// file that is empty, or not the size its row records, as a crash or a
+// restored snapshot can leave it, is removed with its rows and a warning
+// naming it, so the source is fetched again.
+func (c *Cache) reconcileSourceFile(
+	ctx context.Context, path string, entry fs.DirEntry, contentHash ContentHash,
+) error {
+	info, err := entry.Info()
+	if err != nil {
+		return fmt.Errorf("failed to stat source content file: %w", err)
+	}
+
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM source_content WHERE content_hash = ?`,
+		string(contentHash),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows):
+		return c.removeUntrackedSourceFile(ctx, path, contentHash)
+	case err != nil:
+		return fmt.Errorf("failed to check source content row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached source file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictSourceBlob(ctx, contentHash)
+}
+
 // removeUntrackedSourceFile deletes a source blob file that has no
 // source_content row. Any source_metadata rows referencing the hash
 // are removed first so no row ever points at a deleted file.
 func (c *Cache) removeUntrackedSourceFile(
 	ctx context.Context, path string, contentHash ContentHash,
 ) error {
-	var rowExists int
+	_, err := c.db.ExecContext(ctx,
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM source_content WHERE content_hash = ?`, string(contentHash),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check source content row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
-	_, err = c.db.ExecContext(ctx,
 		`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
 	if err != nil {
 		return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)

internal/imgcache/service.go

@@ -143,7 +143,7 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
-	// Check variant cache first
+	// Check variant cache first (disk only, no DB)
 	result, err := s.cache.Lookup(ctx, req)
 	if err != nil {
 		s.log.Warn("cache lookup failed", "error", err)

internal/imgcache/storage.go

@@ -152,24 +152,8 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
 	return err == nil
 }
 
-// Size returns the size in bytes of the content with the given hash.
+// writeIfAbsent writes data with writeFileSynced to the path derived
-func (s *ContentStorage) Size(hash ContentHash) (int64, error) {
+// from hash, unless a file of the data's size is already there.
-	info, err := os.Stat(s.hashToPath(hash))
-	if err != nil {
-		if os.IsNotExist(err) {
-			return 0, ErrNotFound
-		}
-
-		return 0, fmt.Errorf("failed to stat content: %w", err)
-	}
-
-	return info.Size(), nil
-}
-
-// writeIfAbsent writes data to the path derived from hash, unless a
-// file of the data's size is already there. It writes a temp file,
-// syncs it to disk and renames it into place, so neither a concurrent
-// reader nor a crash finds a partial file under that path.
 func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 	// Build path: <basedir>/<ab>/<cd>/<hash>
 	path := s.hashToPath(hash)
@@ -180,15 +164,23 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return nil
 	}
 
-	// Create directory structure
+	err := os.MkdirAll(filepath.Dir(path), StorageDirPerm)
-	dir := filepath.Dir(path)
-
-	err := os.MkdirAll(dir, StorageDirPerm)
 	if err != nil {
 		return fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write to temp file first, then rename for atomicity
+	return writeFileSynced(path, data)
+}
+
+// writeFileSynced writes data to path through a temp file in the same
+// directory: the temp file is synced to disk, renamed to path, and the
+// directory synced after, so that neither a concurrent reader nor a
+// crash finds a partial file under path. When the directory sync fails,
+// the file is removed from path, so that a failed write leaves no file
+// there.
+func writeFileSynced(path string, data []byte) error {
+	dir := filepath.Dir(path)
+
 	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
 	if err != nil {
 		return fmt.Errorf("failed to create temp file: %w", err)
@@ -196,18 +188,12 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 
 	tmpPath := tmpFile.Name()
 
-	// Each failure path below unlinks the temp file explicitly. This
-	// replaces a deferred cleanup that read a named result, which the
-	// canonical config does not permit; the set of paths that remove
-	// tmpPath, and the order relative to Close, is unchanged. This
-	// mirrors how MetadataStorage.Store and VariantStorage.Store below
-	// already express the same cleanup.
 	_, err = tmpFile.Write(data)
 	if err != nil {
 		_ = tmpFile.Close()
 		_ = os.Remove(tmpPath)
 
-		return fmt.Errorf("failed to write content: %w", err)
+		return fmt.Errorf("failed to write temp file: %w", err)
 	}
 
 	err = tmpFile.Sync()
@@ -225,7 +211,6 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to close temp file: %w", err)
 	}
 
-	// Atomic rename
 	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
 	if err != nil {
 		_ = os.Remove(tmpPath)
@@ -233,7 +218,14 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to rename temp file: %w", err)
 	}
 
-	return syncDirectory(dir)
+	err = syncDirectory(dir)
+	if err != nil {
+		_ = os.Remove(path)
+
+		return err
+	}
+
+	return nil
 }
 
 // syncDirectory syncs the directory dir to disk, so that a file just
@@ -468,7 +460,9 @@ func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error
 	return &VariantStorage{baseDir: baseDir, log: log}, nil
 }
 
-// Store writes content and metadata to storage at the given key.
+// Store writes content and metadata to storage at the given key, each
+// with writeFileSynced. The .meta file is written first, so that a crash
+// cannot leave the content in place without it.
 func (s *VariantStorage) Store(
 	key VariantKey, r io.Reader, contentType string,
 ) (int64, error) {
@@ -481,61 +475,11 @@ func (s *VariantStorage) Store(
 	path := s.keyToPath(key)
 	metaPath := path + ".meta"
 
-	// Create directory structure
+	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
-	dir := filepath.Dir(path)
-
-	err = os.MkdirAll(dir, StorageDirPerm)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write content to temp file first, sync it to disk, then rename for
-	// atomicity, so neither a concurrent reader nor a crash finds a
-	// partial file under the key's path
-	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
-	if err != nil {
-		return 0, fmt.Errorf("failed to create temp file: %w", err)
-	}
-
-	tmpPath := tmpFile.Name()
-
-	_, err = tmpFile.Write(data)
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to write content: %w", err)
-	}
-
-	err = tmpFile.Sync()
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to sync temp file: %w", err)
-	}
-
-	err = tmpFile.Close()
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to close temp file: %w", err)
-	}
-
-	// Atomic rename content
-	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to rename temp file: %w", err)
-	}
-
-	err = syncDirectory(dir)
-	if err != nil {
-		return 0, err
-	}
-
-	// Write metadata
 	meta := VariantMeta{
 		ContentType: contentType,
 		Size:        size,
@@ -547,13 +491,19 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// Metadata write failure is non-fatal; content is already stored.
+	// Metadata write failure is non-fatal: the content is then served as
-	err = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	// application/octet-stream.
+	err = writeFileSynced(metaPath, metaData)
 	if err != nil {
 		s.log.Warn("failed to write variant metadata sidecar",
 			"path", metaPath, "error", err)
 	}
 
+	err = writeFileSynced(path, data)
+	if err != nil {
+		return 0, err
+	}
+
 	return size, nil
 }
 
@@ -631,20 +581,6 @@ func (s *VariantStorage) Exists(key VariantKey) bool {
 	return err == nil
 }
 
-// Size returns the size in bytes of the content at the given key.
-func (s *VariantStorage) Size(key VariantKey) (int64, error) {
-	info, err := os.Stat(s.keyToPath(key))
-	if err != nil {
-		if os.IsNotExist(err) {
-			return 0, ErrNotFound
-		}
-
-		return 0, fmt.Errorf("failed to stat content: %w", err)
-	}
-
-	return info.Size(), nil
-}
-
 // Delete removes the content at the given key together with its .meta
 // sidecar file. A missing file is not an error.
 func (s *VariantStorage) Delete(key VariantKey) error {