diff --git a/.claude/rules/distributed-cache.md b/.claude/rules/distributed-cache.md index e40c3a6d..fb10ffc3 100644 --- a/.claude/rules/distributed-cache.md +++ b/.claude/rules/distributed-cache.md @@ -40,17 +40,29 @@ retry the same bytes with a fresh token. Meta invalidation/populate ordering is a strategy selected once at construction (`cache/coordinator.go`), never per-call if/else: -- **Legacy (default)**: `cache.legacy_coordination: true` — the pre-v1.21 - tombstone mechanism (plain Get/Put/Delete + timestamp tombstones). Safe for - any cluster mix, including nodes upgrading straight from ≤v1.20. -- **CAS**: `legacy_coordination: false` — the fenced CAS pattern above. - Requires every node in the cluster to be CAS-capable (≥v1.21) BEFORE the - flip; flip via config + a brisk rolling restart (mixed-mechanism window is - bounded by the restart). - -The proxy layer is mode-blind: it carries an opaque uint64 token (store version -in CAS mode, wall-clock stamp in legacy mode) from decision to commit. Never -branch on the mode outside the coordinator implementations. +- **Legacy (default)**: `cache.legacy_coordination: true` keeps the v1.20 + metadata key and eight-byte tombstone protocol for wire-compatible rollouts; + it is not a fleet-wide freshness guarantee while v1.20 readers remain. + Current TAG also CAS-updates a separate `meta-gen|...` sidecar and records + its generation in new metadata; current reads require a match, so old rows + without a generation are misses until repopulated. A decision read creates a + TTL-bounded publish marker when none exists; eviction before commit makes the + captured token fail closed. If a cache owner lacks the sidecar CAS RPC, + current TAG fails closed and bypasses that cache entry. Older TAG readers + ignore the sidecar and can still serve a late plain metadata Put during a + mixed-version rollout. The v1.20 reader reads only the plain metadata key, and + its tombstone check precedes a separate plain Put; a writer that passed the + check can publish after a later delete. This old check-to-Put limitation is + outside the sidecar-aware current-reader guarantee. +- **CAS**: `legacy_coordination: false` — the fenced CAS pattern above on the + metadata key. Requires every node in the cluster to be CAS-capable (≥v1.21) + BEFORE the flip; flip via config + a brisk rolling restart (mixed-mechanism + window is bounded by the restart). + +The proxy layer is mode-blind: it carries an opaque comparable token from +decision to commit. CAS mode uses the metadata-key version; legacy mode carries +the generation-sidecar observation and v1.20 tombstone timestamp. Never branch +on the mode outside the coordinator implementations. ## Stream Multiplexing > Batching diff --git a/CLAUDE.md b/CLAUDE.md index ad89c574..e14b1758 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ All dependencies are public, including the Tigris `ocache` modules (`github.com/ - **Optional Clustering**: Memberlist gossip for discovery, gRPC for cache key routing between nodes - **Broadcast/Subscriber Pattern**: Request coalescing via `proxy/broadcast/` - streams chunks to multiple listeners simultaneously - **Two-Key Cache Storage**: Objects stored as `meta|bucket|key` (headers/ETag) and `body|bucket|key` (raw bytes) -- **Meta Coordination Strategy**: Invalidation/populate ordering is a construction-time strategy (`cache/coordinator.go`, `cache.legacy_coordination`). Default is the legacy tombstone mechanism (safe for any cluster mix, including direct ≤v1.20 upgrades); flipping the flag off selects fenced CAS ops (ocache v1.13.0, ocache#267) where every populate commits under a decision-time version token and writes that observed pre-delete state lose atomically. The proxy layer is mode-blind — it carries an opaque token, never branches on the mode +- **Meta Coordination Strategy**: Invalidation/populate ordering is selected once at construction (`cache/coordinator.go`). The default legacy path keeps the v1.20 tombstone and plain metadata-key protocol for wire-compatible rollouts, not a fleet-wide freshness guarantee while v1.20 readers remain; current TAG tags new metadata with a separate CAS generation sidecar and validates it on reads. Rows without a generation are misses until repopulated. A decision read creates a TTL-bounded publish marker when none exists; if disk-cap eviction removes it before commit, that token refuses the late write. Older readers ignore the sidecar and can still serve a late plain Put during a mixed-version rollout. The v1.20 reader uses only the plain metadata key, and its tombstone check precedes a separate plain Put; a writer that passed the check can publish after a later delete. This check-to-Put limit is outside the sidecar-aware current-reader guarantee. If a cache owner lacks the sidecar CAS RPC, current TAG fails closed and bypasses that entry. Setting `cache.legacy_coordination: false` selects fenced CAS operations on the metadata key (ocache v1.13.0, ocache#267); enable it only after every node supports CAS. The proxy layer stays mode-blind and carries the coordinator's opaque decision token. - **Ingress Admission**: `server.max_inflight_requests` (default 1024) bounds concurrently-served S3 requests; excess is shed with 503 SlowDown. Operational endpoints (`/health`, `/metrics`, `/debug/pprof/*`) are exempt. `0`/unset = default, negative = disabled. - **Semaphore-Gated Cache Writes**: `cache.max_concurrent_writes` (default 256) bounds concurrent cache-populate operations; when saturated, objects are served from upstream without being cached. `0`/unset = default, negative = disabled. diff --git a/README.md b/README.md index c8d6fdb7..98e52dab 100644 --- a/README.md +++ b/README.md @@ -102,8 +102,9 @@ When configuring S3 clients, ensure path-style addressing is enabled. See [docs/ - Among cacheable objects (within `size_threshold`), those at or above `block_size` are cached as fixed-size blocks (block caching, on by default), so a range read fetches only the covering blocks; smaller objects are whole-cached - Set `block_caching_enabled: false` to cache whole objects instead — a range request then triggers a background fetch of the full object (if within threshold) - PUT/DELETE operations invalidate the cache entry +- In proxy modes, a lost DeleteObjects reply triggers a second invalidation for each requested key; a cacheable 200 revalidation updates the cache only after a HEAD confirms the same strong ETag and length -See [docs/cache-control.md](docs/cache-control.md) for detailed cache control and revalidation documentation. For tuning and sizing block caching in production, see [docs/deploy.md](docs/deploy.md#block-aligned-caching). +See [docs/cache-control.md](docs/cache-control.md) for detailed cache control, revalidation, and mixed-version cache behavior. For tuning and sizing block caching in production, see [docs/deploy.md](docs/deploy.md#block-aligned-caching). ## Configuration diff --git a/cache/block_cache_test.go b/cache/block_cache_test.go index dd854082..4e16ab82 100644 --- a/cache/block_cache_test.go +++ b/cache/block_cache_test.go @@ -385,7 +385,7 @@ func TestPutMetaIfVersion(t *testing.T) { meta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"v1"`, ContentLength: 100, StatusCode: 200, BlockSize: 4} // No tombstone → meta is written. - wrote, err := c.PutMetaIfVersion(ctx, bucket, key, meta, 60, VersionAny) + wrote, err := c.PutMetaIfVersion(ctx, bucket, key, meta, 60, AnyMetaVersionToken()) if err != nil || !wrote { t.Fatalf("PutMetaIfVersion = (%v, %v), want (true, nil)", wrote, err) } diff --git a/cache/cache.go b/cache/cache.go index d95d0cf5..68b371b1 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -43,8 +43,9 @@ type Cache struct { client cacheclient.CacheClient defaultTTL int64 // seconds enabled bool - closed bool // coord is the meta-key ordering strategy: legacy tombstones (default) - // or CAS fences/versions. Selected once at construction; see coordinator.go. + closed bool // coord is the meta-key strategy: legacy tombstones plus a + // current-reader generation sidecar (default), or CAS metadata-key fences. + // Selected once at construction; see coordinator.go. coord metaCoordinator } @@ -65,7 +66,12 @@ func NewCacheWithClient(client cacheclient.CacheClient, cfg *config.CacheConfig) } var coord metaCoordinator if legacy { - coord = &legacyCoordinator{client: client, tombstoneTTL: TombstoneTTLSeconds(sizeThreshold)} + tombstoneTTL := TombstoneTTLSeconds(sizeThreshold) + coord = &legacyCoordinator{ + client: client, + tombstoneTTL: tombstoneTTL, + generationTTL: ttl + tombstoneTTL, + } } else { coord = &casCoordinator{client: client} } @@ -94,17 +100,30 @@ func (c *Cache) IsEnabled() bool { // Two-Key Pattern: Metadata and Body stored separately // ============================================================================ -// VersionAny requests a version-BUMPING unconditional meta write: last-write- -// wins like a plain Put, but implemented as a CAS loop so the row stays -// version-stamped. This upholds the package invariant that NO meta key is ever -// written with a plain Put — a plain Put resets the row to the storage layer's -// legacy version, which blinds DeleteIfETag's version guard for every -// subsequent conditional operation on the key. +// MetaVersionToken is an opaque, comparable snapshot for a metadata commit. +// CAS coordination carries the metadata-key version; legacy coordination also +// carries the tombstone decision time and sidecar generation. Callers must pass +// the value returned by GetMetaWithVersion without reconstructing its contents. +type MetaVersionToken struct { + version uint64 + decisionTime int64 + unconditional bool +} + +// AnyMetaVersionToken requests an unconditional last-write-wins metadata update. +// The coordinator prepares any decision-time state required by its mode. +func AnyMetaVersionToken() MetaVersionToken { + return MetaVersionToken{unconditional: true} +} + +// VersionAny remains the numeric sentinel for APIs that compare raw store +// versions. Metadata writes use MetaVersionToken so legacy timestamp and sidecar +// state stay together. const VersionAny = ^uint64(0) // putMetaVersioned commits the meta bytes through the selected coordinator // (see coordinator.go for both mechanisms' semantics). -func (c *Cache) putMetaVersioned(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected uint64) (bool, error) { +func (c *Cache) putMetaVersioned(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected MetaVersionToken) (bool, error) { return c.coord.putMeta(ctx, bucket, key, metaKey, metaBytes, ttl, expected) } func (c *Cache) PutWithMeta(ctx context.Context, bucket, key string, meta *CachedObjectMeta, body []byte, ttl int) error { @@ -124,6 +143,11 @@ func (c *Cache) PutWithMeta(ctx context.Context, bucket, key string, meta *Cache ttl = int(c.defaultTTL) } + expected, err := c.coord.prepareMetaToken(ctx, bucket, key, AnyMetaVersionToken()) + if err != nil { + return err + } + metaKey := MakeMetaKey(bucket, key) bodyKey := MakeBodyKey(bucket, key, meta.ETag) @@ -141,9 +165,10 @@ func (c *Cache) PutWithMeta(ctx context.Context, bucket, key string, meta *Cache return err } - // Store metadata AFTER body is complete. Version-bumping unconditional: - // same last-write-wins semantics as before, but the row stays stamped. - if _, err := c.putMetaVersioned(ctx, bucket, key, metaKey, metaBytes, int64(ttl), VersionAny); err != nil { + // Store metadata AFTER the body is complete. CAS mode keeps the row + // version-stamped; legacy mode records its sidecar generation for current + // readers while retaining the plain metadata Put used by older peers. + if _, err := c.putMetaVersioned(ctx, bucket, key, metaKey, metaBytes, int64(ttl), expected); err != nil { log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Cache meta put error") // Leave the versioned body to age out via TTL rather than deleting it // synchronously: a concurrent populate of the same ETag could have a reader @@ -163,28 +188,31 @@ func (c *Cache) PutWithMeta(ctx context.Context, bucket, key string, meta *Cache } // PutWithMetaStreamIfVersion is like PutWithMetaStream but commits the -// metadata under the caller's decision-time version precondition (see -// putMetaVersioned): body first, then the meta write that makes the entry -// visible — refused atomically if the entry changed (including a fenced -// delete) since the caller's token was read. +// metadata under the caller's decision-time token (see putMetaVersioned): body +// first, then the metadata write that makes the entry visible. CAS mode refuses +// atomically if the metadata key changed. Legacy mode stores the separate +// generation in the row; a late plain Put may land, but current TAG readers +// will not serve it after that generation advances. Older readers do not check +// the sidecar. // -// The expected version is the meta write's precondition: a decision-time -// token (live version or absence token) for an ordered populate, VersionAny -// for last-write-wins. 0 is unordered put-if-absent under the CAS -// coordinator but is REFUSED by the legacy coordinator (nothing to order -// against) — production callers always carry a nonzero token. +// The expected token carries the metadata-key version in CAS mode. Legacy mode +// carries the sidecar version and decision-time stamp used by v1.20 tombstone +// checks. GetMetaWithVersion creates a decision marker when absent; if that +// marker is evicted before commit, the stale token is refused. AnyMetaVersionToken +// requests last-write-wins. A zero-value token is invalid for legacy mode and +// is refused. // -// Returns wrote=true only when the metadata was actually written (the entry -// is now visible). It is false when the write was skipped without error — the -// object was not cacheable (no ETag) or the precondition was lost — so -// callers can distinguish a no-op from a real write. +// Returns wrote=true when the metadata write completed. It is false when the +// write was skipped without error — the object was not cacheable (no ETag) or +// the precondition was lost. In legacy mode a late plain Put can complete but +// remain invisible to current TAG readers because its sidecar generation is stale. func (c *Cache) PutWithMetaStreamIfVersion( ctx context.Context, bucket, key string, meta *CachedObjectMeta, body io.Reader, ttl int, - expected uint64, // decision-time version precondition for the meta write + expected MetaVersionToken, // decision-time token for the metadata write ) (wrote bool, err error) { if !c.IsEnabled() { return false, nil @@ -204,6 +232,11 @@ func (c *Cache) PutWithMetaStreamIfVersion( if ttl == 0 { ttl = int(c.defaultTTL) } + expected, err = c.coord.prepareMetaToken(ctx, bucket, key, expected) + if err != nil { + _, _ = io.Copy(io.Discard, body) + return false, err + } metaKey := MakeMetaKey(bucket, key) bodyKey := MakeBodyKey(bucket, key, meta.ETag) @@ -221,16 +254,17 @@ func (c *Cache) PutWithMetaStreamIfVersion( return false, err } - // No tombstone gate: the version precondition below IS the invalidation - // guard — a fenced delete (or any write) landing after the caller's - // decision-time token makes this commit lose atomically. On a lost commit - // the just-written versioned body is left to age out via TTL rather than - // deleted synchronously: a concurrent populate of the same ETag could have - // a reader streaming this exact body key, and deleting it would truncate - // that reader. Without a visible meta entry the orphaned body is - // unreachable and harmless until it expires. + // The coordinator applies the decision-time token. CAS mode rejects a + // metadata write atomically after a fence; legacy mode tags the plain write + // with its sidecar generation so current readers reject a late commit. The + // old tombstone check remains for legacy compatibility but is not atomic. + // On a lost commit the just-written versioned body is left to age out via + // TTL rather than deleted synchronously: a concurrent populate of the same + // ETag could have a reader streaming this exact body key, and deleting it + // would truncate that reader. Without visible metadata the orphaned body is + // unreachable to current readers until it expires. - // Write metadata AFTER body (makes entry visible), under the version + // Write metadata AFTER body (makes entry visible to current readers), under the version // precondition. A lost precondition leaves the newer entry in place and the // just-written body to TTL. wrote, err = c.putMetaVersioned(ctx, bucket, key, metaKey, metaBytes, int64(ttl), expected) @@ -253,13 +287,14 @@ func (c *Cache) PutWithMetaStreamIfVersion( return true, nil } -// GetMetaWithVersion is GetMeta plus the decision-time TOKEN a subsequent -// commit must carry (semantics per coordinator: a store version in CAS mode, -// a wall-clock stamp in legacy mode). Absent entries return found=false with -// a valid token; callers test found, never token==0. -func (c *Cache) GetMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, uint64, bool, error) { +// GetMetaWithVersion is GetMeta plus the decision-time token a subsequent +// commit must carry. The token includes the metadata-key store version in CAS +// mode. Legacy mode retains the tombstone timestamp and generation version, and +// creates a marker at decision time when none exists. Callers use found for +// metadata presence. +func (c *Cache) GetMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, MetaVersionToken, bool, error) { if !c.IsEnabled() { - return nil, 0, false, nil + return nil, MetaVersionToken{}, false, nil } return c.coord.getMetaWithVersion(ctx, bucket, key) } @@ -267,36 +302,16 @@ func (c *Cache) GetMeta(ctx context.Context, bucket, key string) (*CachedObjectM if !c.IsEnabled() { return nil, false, nil } - - metaKey := MakeMetaKey(bucket, key) - - // Get metadata - metaBytes, err := c.client.Get(ctx, metaKey) + meta, found, err := c.coord.getMeta(ctx, bucket, key) if err != nil { - if isNotFoundError(err) { - log.Debug().Str("bucket", bucket).Str("key", key).Msg("Cache miss (meta only)") - return nil, false, nil - } log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Cache meta get error") return nil, false, err } - - if metaBytes == nil { + if !found || meta == nil { + log.Debug().Str("bucket", bucket).Str("key", key).Msg("Cache miss (meta only)") return nil, false, nil } - - // Decode metadata - meta, err := DecodeMeta(metaBytes) - if err != nil { - log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Cache meta decode error") - return nil, false, err - } - - log.Debug(). - Str("bucket", bucket). - Str("key", key). - Int("meta_size", len(metaBytes)). - Msg("Cache hit (meta only)") + log.Debug().Str("bucket", bucket).Str("key", key).Msg("Cache hit (meta only)") return meta, true, nil } @@ -359,14 +374,20 @@ func (c *Cache) GetBodyStream(ctx context.Context, bucket, key, etag string, w i return nil } -// DeleteWithMeta removes both metadata and body from cache via the fenced CAS -// delete: the fence it leaves is what stops an in-flight populate that -// observed pre-delete state (including pre-delete absence) from completing. +// DeleteWithMeta invalidates visible metadata through the selected +// coordinator. CAS deletes fence earlier metadata-key tokens. Legacy mode +// advances a separate CAS generation for current TAG readers, then retains the +// v1.20 tombstone and plain metadata operations. A late legacy Put remains +// invisible to current readers; older TAG readers do not consult the sidecar +// and can still observe that write during a mixed-version rollout. The body is +// left to age out via TTL rather than deleted synchronously, which could +// truncate an in-flight reader of that version. // -// Both steps are attempted even if the first fails (best-effort invalidation), but a -// genuine backend failure of either is returned so callers don't report a successful -// invalidation while stale metadata is still readable. A not-found metadata delete is -// success — the entry is already gone. +// Legacy mode attempts the sidecar advance, tombstone write, and metadata +// deletion even if one fails. +// A genuine backend failure is returned so callers don't report successful +// invalidation while stale metadata is still readable. A not-found metadata +// delete is success — the entry is already gone. func (c *Cache) DeleteWithMeta(ctx context.Context, bucket, key string) error { if !c.IsEnabled() { return nil @@ -374,17 +395,12 @@ func (c *Cache) DeleteWithMeta(ctx context.Context, bucket, key string) error { var errs []error - // Delete only the metadata. That is sufficient to make subsequent reads miss - // (a read resolves the body from meta.ETag, so with meta gone there is no body - // lookup), and the fence left by the CAS delete blocks any in-flight - // token-carrying repopulation. The - // versioned body is intentionally left to age out via TTL rather than deleted - // synchronously — deleting it could truncate an in-flight reader still - // streaming that exact version. - // - // FENCED (ocache v1.13.0, #267): the delete goes through the CAS op family - // so it leaves a fence, ordering every token-carrying populate that - // observed pre-delete state — the sole invalidation mechanism. + // Delete only the metadata. With no metadata, subsequent reads do not look + // up the body. CAS deletion leaves a version fence; legacy deletion advances + // the current-reader generation sidecar, then uses the compatibility + // tombstone and plain metadata delete. The versioned body is left to age out + // via TTL rather than deleted synchronously — deleting it could truncate an + // in-flight reader still streaming that version. if err := c.deleteMetaFenced(ctx, bucket, key); err != nil { log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Cache meta delete error") errs = append(errs, fmt.Errorf("delete meta: %w", err)) @@ -398,8 +414,8 @@ func (c *Cache) DeleteWithMeta(ctx context.Context, bucket, key string) error { return nil } -// deleteMetaFenced routes the unconditional invalidation through the selected -// coordinator (fenced CAS delete, or legacy tombstone + plain delete). +// deleteMetaFenced routes invalidation through the selected coordinator +// (CAS metadata fence, or legacy generation sidecar plus tombstone/plain delete). func (c *Cache) deleteMetaFenced(ctx context.Context, bucket, key string) error { return c.coord.deleteMeta(ctx, bucket, key) } @@ -417,11 +433,11 @@ func (c *Cache) Delete(ctx context.Context, bucket, key string) error { // even by identical content reusing the same MD5 ETag — refuses the delete. // Returns (false, nil) when replaced or absent. CAS-coordinator strength; the // legacy coordinator refuses (see coordinator.go). -func (c *Cache) DeleteMetaIfVersion(ctx context.Context, bucket, key string, version uint64) (bool, error) { +func (c *Cache) DeleteMetaIfVersion(ctx context.Context, bucket, key string, expected MetaVersionToken) (bool, error) { if !c.IsEnabled() { return false, nil } - return c.coord.deleteMetaIfVersion(ctx, bucket, key, version) + return c.coord.deleteMetaIfVersion(ctx, bucket, key, expected) } // DeleteIfETag invalidates the object's metadata only while it still carries @@ -720,7 +736,7 @@ func (c *Cache) PutMetaIfVersion( bucket, key string, meta *CachedObjectMeta, ttl int, - expected uint64, + expected MetaVersionToken, ) (wrote bool, err error) { if !c.IsEnabled() { return false, nil @@ -731,6 +747,10 @@ func (c *Cache) PutMetaIfVersion( if ttl == 0 { ttl = int(c.defaultTTL) } + expected, err = c.coord.prepareMetaToken(ctx, bucket, key, expected) + if err != nil { + return false, err + } metaKey := MakeMetaKey(bucket, key) metaBytes, err := meta.Encode() if err != nil { @@ -753,10 +773,8 @@ func (c *Cache) Has(ctx context.Context, bucket, key string) bool { ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() - // Check for metadata key - metaKey := MakeMetaKey(bucket, key) - metaBytes, err := c.client.Get(ctx, metaKey) - return err == nil && metaBytes != nil + _, found, err := c.GetMeta(ctx, bucket, key) + return err == nil && found } // ListKeys returns all keys matching the prefix. diff --git a/cache/coordinator.go b/cache/coordinator.go index 7c3baaac..8de3f981 100644 --- a/cache/coordinator.go +++ b/cache/coordinator.go @@ -7,11 +7,12 @@ package cache // proxy's RequestForwarder strategy pattern): // // - legacyCoordinator (cache.legacy_coordination: true, the DEFAULT): -// the pre-CAS mechanism, byte-faithful to v1.20 — timestamp tombstones -// written before deletes, checked before meta commits, plain Put/Delete on -// the meta key. Rolling upgrades from v1.20 are homogeneous under it: a -// mixed cluster runs one mechanism, so there is no cross-version ordering -// gap to reason about. +// preserves the v1.20 eight-byte tombstones and plain metadata Put/Delete +// so older peers keep their existing wire protocol. New TAG readers and +// writers also use a separate CAS generation sidecar: metadata rows carry +// its decision-time version, and a read rejects a late plain Put after the +// sidecar advances. Older readers do not consult that sidecar. If its CAS +// RPC is unavailable on an old cache owner, new TAG readers fail closed. // // - casCoordinator (legacy_coordination: false): ocache v1.13.0's fenced // CAS deletes and per-key versions carry all ordering; no tombstones @@ -19,10 +20,10 @@ package cache // release (standalone nodes qualify trivially). // // The caller-facing contract is identical either way: read a decision-time -// TOKEN with getMetaWithVersion before fetching, commit with putMeta under it. -// In CAS mode the token is a store version (absence included); in legacy mode -// it is the wall-clock stamp the old writeStartTime discipline used — which is -// why the proxy layer is mode-blind and contains no conditionals. +// token with getMetaWithVersion before fetching, commit with putMeta under it. +// CAS mode uses the metadata-key store version. Legacy mode carries both the +// sidecar version and the v1.20 tombstone timestamp in the opaque token. The +// proxy layer remains mode-blind. // // Flipping legacy → CAS on a live cluster: set the flag and rolling-restart. // Nodes in different modes during that restart window run different ordering @@ -44,15 +45,18 @@ import ( // metaCoordinator is the ordering seam for the mutable meta key. type metaCoordinator interface { - // getMetaWithVersion reads the metadata and the decision-time token a - // subsequent putMeta must carry. Absent entries return found=false with a - // still-valid token; callers test found, never token==0. - getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, uint64, bool, error) + // getMeta is the current-reader path. The legacy coordinator validates its + // sidecar generation before returning metadata; CAS reads the metadata key. + getMeta(ctx context.Context, bucket, key string) (*CachedObjectMeta, bool, error) + // getMetaWithVersion reads metadata plus the complete decision-time token. + getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, MetaVersionToken, bool, error) + // prepareMetaToken resolves an unconditional write token before body or + // upstream work when the selected coordinator needs a decision-time fence. + prepareMetaToken(ctx context.Context, bucket, key string, expected MetaVersionToken) (MetaVersionToken, error) // putMeta commits metaBytes under the caller's decision-time token. - // expected==VersionAny is the deliberately unordered last-write-wins used - // by tests and simple seeding. Returns wrote=false without error when the - // precondition was lost — the newer state wins. - putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected uint64) (bool, error) + // Unconditional tokens are used only for last-write-wins seeding. Returns + // wrote=false without error when the precondition was lost. + putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected MetaVersionToken) (bool, error) // deleteMeta is the unconditional invalidation of the meta key. deleteMeta(ctx context.Context, bucket, key string) error // deleteMetaIfETag invalidates only while the entry still carries @@ -64,7 +68,7 @@ type metaCoordinator interface { // (which identical bytes can reuse across entries and tiers). CAS- // coordinator strength: legacy coordination stores no versions to compare // and refuses with (false, nil). - deleteMetaIfVersion(ctx context.Context, bucket, key string, version uint64) (bool, error) + deleteMetaIfVersion(ctx context.Context, bucket, key string, expected MetaVersionToken) (bool, error) } // ============================================================================ @@ -75,7 +79,25 @@ type casCoordinator struct { client cacheclient.CacheClient } -func (c *casCoordinator) getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, uint64, bool, error) { +func (c *casCoordinator) getMeta(ctx context.Context, bucket, key string) (*CachedObjectMeta, bool, error) { + metaBytes, err := c.client.Get(ctx, MakeMetaKey(bucket, key)) + if err != nil { + if isNotFoundError(err) { + return nil, false, nil + } + return nil, false, err + } + if metaBytes == nil { + return nil, false, nil + } + meta, err := DecodeMeta(metaBytes) + if err != nil { + return nil, false, err + } + return meta, true, nil +} + +func (c *casCoordinator) getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, MetaVersionToken, bool, error) { metaBytes, version, found, err := c.client.GetWithVersion(ctx, MakeMetaKey(bucket, key)) if err != nil { if isNotFoundError(err) { @@ -83,32 +105,36 @@ func (c *casCoordinator) getMetaWithVersion(ctx context.Context, bucket, key str // token, not an error. Pass through whatever version accompanied // the error rather than squashing to 0 — 0 would opt the caller // into unordered put-if-absent that a fence exists to refuse. - return nil, version, false, nil + return nil, MetaVersionToken{version: version}, false, nil } - return nil, 0, false, err + return nil, MetaVersionToken{}, false, err } if !found || metaBytes == nil { // Absence carries a nonzero token (fence stamp or fresh observation); // it orders the caller's commit against a later fenced delete. - return nil, version, false, nil + return nil, MetaVersionToken{version: version}, false, nil } meta, err := DecodeMeta(metaBytes) if err != nil { - return nil, 0, false, err + return nil, MetaVersionToken{}, false, err } - return meta, version, true, nil + return meta, MetaVersionToken{version: version}, true, nil } -func (c *casCoordinator) putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected uint64) (bool, error) { - if expected != VersionAny { - if _, err := c.client.PutIfVersion(ctx, metaKey, metaBytes, ttl, expected); err != nil { +func (c *casCoordinator) prepareMetaToken(_ context.Context, _, _ string, expected MetaVersionToken) (MetaVersionToken, error) { + return expected, nil +} + +func (c *casCoordinator) putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected MetaVersionToken) (bool, error) { + if !expected.unconditional { + if _, err := c.client.PutIfVersion(ctx, metaKey, metaBytes, ttl, expected.version); err != nil { if _, mismatch := cacheclient.IsVersionMismatch(err); mismatch { // Debug + metric, per the repo's log policy: a lost // precondition replaces what was previously a SILENT lost // update, so a low rate is the feature working; growth means a // precondition was chosen wrong for its path. metrics.RecordCacheOperation("meta_put", "precondition_lost") - log.Debug().Str("bucket", bucket).Str("key", key).Uint64("expected", expected). + log.Debug().Str("bucket", bucket).Str("key", key).Uint64("expected", expected.version). Msg("Skipping meta write - version precondition lost to a newer write") return false, nil } @@ -201,8 +227,8 @@ func (c *casCoordinator) deleteMetaIfETag(ctx context.Context, bucket, key, stal return true, nil } -func (c *casCoordinator) deleteMetaIfVersion(ctx context.Context, bucket, key string, version uint64) (bool, error) { - if derr := c.client.DeleteIfVersion(ctx, MakeMetaKey(bucket, key), version); derr != nil { +func (c *casCoordinator) deleteMetaIfVersion(ctx context.Context, bucket, key string, expected MetaVersionToken) (bool, error) { + if derr := c.client.DeleteIfVersion(ctx, MakeMetaKey(bucket, key), expected.version); derr != nil { if _, mismatch := cacheclient.IsVersionMismatch(derr); mismatch { // Replaced (or removed) since the caller's read: the newer state // wins — exactly what the guard exists for. @@ -217,7 +243,8 @@ func (c *casCoordinator) deleteMetaIfVersion(ctx context.Context, bucket, key st } // ============================================================================ -// Legacy coordinator — timestamp tombstones, byte-faithful to v1.20 +// Legacy coordinator — v1.20 metadata/tombstone protocol plus the +// current-reader generation sidecar // ============================================================================ const ( @@ -250,54 +277,199 @@ func TombstoneTTLSeconds(sizeThreshold int64) int64 { } type legacyCoordinator struct { - client cacheclient.CacheClient - tombstoneTTL int64 // seconds; must outlive the longest racing cache-populate + client cacheclient.CacheClient + tombstoneTTL int64 // seconds; must outlive the longest racing cache-populate + generationTTL int64 // seconds; default cache TTL plus populate margin +} + +const legacyGenerationMarker byte = 1 + +// generationToken reads the separate CAS sidecar used to tag legacy metadata. +// Unsupported or failed CAS reads fail closed. +func (c *legacyCoordinator) generationToken(ctx context.Context, bucket, key string) (uint64, bool, error) { + marker, version, found, err := c.client.GetWithVersion(ctx, makeGenerationKey(bucket, key)) + if err != nil && !isNotFoundError(err) { + return 0, false, fmt.Errorf("read legacy generation for %s/%s: %w", bucket, key, err) + } + if found && (len(marker) != 1 || marker[0] != legacyGenerationMarker) { + return 0, false, fmt.Errorf("invalid legacy generation marker for %s/%s", bucket, key) + } + return version, found, nil +} + +// ensureDecisionGeneration creates a marker at token-capture time when a key +// has none. The populate token carries its version, so eviction before commit +// leaves no matching generation and makes the delayed writer fail closed. +func (c *legacyCoordinator) ensureDecisionGeneration(ctx context.Context, bucket, key string) (uint64, error) { + generationKey := makeGenerationKey(bucket, key) + const maxAttempts = 64 + for attempt := 0; attempt < maxAttempts; attempt++ { + if err := ctx.Err(); err != nil { + return 0, fmt.Errorf("initialize legacy generation for %s/%s: %w", bucket, key, err) + } + version, found, err := c.generationToken(ctx, bucket, key) + if err != nil { + return 0, err + } + if found { + return version, nil + } + version, err = c.client.PutIfVersion(ctx, generationKey, []byte{legacyGenerationMarker}, c.generationTTL, version) + if err == nil { + return version, nil + } + if _, mismatch := cacheclient.IsVersionMismatch(err); !mismatch { + return 0, fmt.Errorf("initialize legacy generation for %s/%s: %w", bucket, key, err) + } + } + return 0, fmt.Errorf("initialize legacy generation for %s/%s lost %d consecutive version races", bucket, key, maxAttempts) +} + +// advanceGeneration atomically moves the sidecar version before a legacy +// invalidation. The metadata key itself remains the plain v1.20 key so older +// peers can still read and write it; current readers use the sidecar version to +// reject late plain metadata puts. +func (c *legacyCoordinator) advanceGeneration(ctx context.Context, bucket, key string) error { + generationKey := makeGenerationKey(bucket, key) + const maxAttempts = 64 + for attempt := 0; attempt < maxAttempts; attempt++ { + if err := ctx.Err(); err != nil { + return fmt.Errorf("advance legacy generation for %s/%s: %w", bucket, key, err) + } + version, _, err := c.generationToken(ctx, bucket, key) + if err != nil { + return err + } + if _, err := c.client.PutIfVersion(ctx, generationKey, []byte{legacyGenerationMarker}, c.generationTTL, version); err == nil { + return nil + } else if _, mismatch := cacheclient.IsVersionMismatch(err); !mismatch { + return fmt.Errorf("write legacy generation for %s/%s: %w", bucket, key, err) + } + } + return fmt.Errorf("advance legacy generation for %s/%s lost %d consecutive version races", bucket, key, maxAttempts) } -// getMetaWithVersion reads via the plain API and hands out the wall-clock -// stamp the old writeStartTime discipline used as the decision-time token: -// callers capture it before fetching, and putMeta refuses the commit if an -// invalidation tombstone postdates it — the pre-CAS ordering, unchanged. -func (c *legacyCoordinator) getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, uint64, bool, error) { - stamp := uint64(time.Now().UnixNano()) +// getMetaWithVersion reads the metadata snapshot and captures the sidecar +// version plus v1.20 tombstone decision time. If no sidecar exists, it creates a +// decision marker before returning the token. Later marker eviction then makes +// the commit fail closed rather than looking like a fresh absence. +func (c *legacyCoordinator) getMetaWithVersion(ctx context.Context, bucket, key string) (*CachedObjectMeta, MetaVersionToken, bool, error) { + // Stamp before every read. An older v1.20 peer can write a tombstone without + // advancing this generation sidecar; the decision-time check must still see + // that invalidation if it lands while we read the metadata and generation. + decisionTime := time.Now().UnixNano() + if _, err := c.tombstoneTimestamp(ctx, bucket, key); err != nil { + return nil, MetaVersionToken{}, false, err + } + metaBytes, metaErr := c.client.Get(ctx, MakeMetaKey(bucket, key)) + if metaErr != nil && !isNotFoundError(metaErr) { + return nil, MetaVersionToken{}, false, metaErr + } + var meta *CachedObjectMeta + if metaErr == nil && metaBytes != nil { + var err error + meta, err = DecodeMeta(metaBytes) + if err != nil { + return nil, MetaVersionToken{}, false, err + } + } + tokenVersion, foundGeneration, err := c.generationToken(ctx, bucket, key) + if err != nil { + return nil, MetaVersionToken{}, false, err + } + if !foundGeneration { + // Keep a decision marker until this populate commits or its bounded + // generation TTL expires. If the disk-cap cleaner evicts it after a + // later invalidation, putMeta sees that the captured marker vanished and + // refuses the old writer instead of recreating a fresh generation. + tokenVersion, err = c.ensureDecisionGeneration(ctx, bucket, key) + if err != nil { + return nil, MetaVersionToken{}, false, err + } + } + token := MetaVersionToken{ + version: tokenVersion, + decisionTime: decisionTime, + } + if meta == nil || meta.cacheGeneration != tokenVersion { + return nil, token, false, nil + } + return meta, token, true, nil +} + +func (c *legacyCoordinator) getMeta(ctx context.Context, bucket, key string) (*CachedObjectMeta, bool, error) { metaBytes, err := c.client.Get(ctx, MakeMetaKey(bucket, key)) if err != nil { if isNotFoundError(err) { - return nil, stamp, false, nil + return nil, false, nil } - return nil, 0, false, err + return nil, false, err } if metaBytes == nil { - return nil, stamp, false, nil + return nil, false, nil } meta, err := DecodeMeta(metaBytes) if err != nil { - return nil, 0, false, err + return nil, false, err + } + generation, found, err := c.generationToken(ctx, bucket, key) + if err != nil { + return nil, false, err + } + if !found || meta.cacheGeneration != generation { + return nil, false, nil + } + return meta, true, nil +} + +func (c *legacyCoordinator) prepareMetaToken(ctx context.Context, bucket, key string, expected MetaVersionToken) (MetaVersionToken, error) { + if !expected.unconditional { + return expected, nil } - return meta, stamp, true, nil + _, token, _, err := c.getMetaWithVersion(ctx, bucket, key) + return token, err } -func (c *legacyCoordinator) putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected uint64) (bool, error) { - if expected == 0 { - // No decision-time token to order against. The CAS coordinator's - // expected==0 is unordered put-if-absent; that has no tombstone - // analogue, and writing here could resurrect an invalidated entry. - // Refuse — a skipped cache write is always safe. Production callers - // always carry a nonzero token (legacy stamps are wall-clock nanos). +func (c *legacyCoordinator) putMeta(ctx context.Context, bucket, key, metaKey string, metaBytes []byte, ttl int64, expected MetaVersionToken) (bool, error) { + if expected.decisionTime <= 0 { + // No decision-time token to order against. Refuse rather than publish + // metadata that current readers cannot validate. metrics.RecordCacheOperation("meta_put", "precondition_lost") return false, nil } - if expected != VersionAny { - // Tombstone gate right before the visibility-granting meta write: an - // invalidation at or after the caller's decision-time stamp blocks the - // commit, so a populate cannot resurrect deleted metadata. - if ts := c.tombstoneTimestamp(ctx, bucket, key); ts >= int64(expected) { - metrics.RecordCacheOperation("meta_put", "precondition_lost") - log.Debug().Str("bucket", bucket).Str("key", key). - Int64("tombstone_ts", ts).Uint64("write_start", expected). - Msg("Skipping meta write - tombstone detected") - return false, nil - } + // Retain the v1.20 decision-time tombstone check for mixed-version peers. + // The sidecar is a separate guard for current readers because the metadata + // key still uses a plain compatibility Put. + tombstone, err := c.tombstoneTimestamp(ctx, bucket, key) + if err != nil { + return false, err + } + if tombstone >= expected.decisionTime { + metrics.RecordCacheOperation("meta_put", "precondition_lost") + log.Debug().Str("bucket", bucket).Str("key", key).Int64("tombstone_ts", tombstone). + Int64("write_start", expected.decisionTime).Msg("Skipping meta write - tombstone detected") + return false, nil + } + current, found, err := c.generationToken(ctx, bucket, key) + if err != nil { + return false, err + } + if !found || current != expected.version { + // A decision token captured from an existing sidecar cannot be reused + // after that marker expires or is evicted; fail closed rather than + // reviving an old writer under a fresh generation. + metrics.RecordCacheOperation("meta_put", "precondition_lost") + return false, nil + } + generation := current + meta, err := DecodeMeta(metaBytes) + if err != nil { + return false, err + } + meta.cacheGeneration = generation + metaBytes, err = meta.Encode() + if err != nil { + return false, err } if err := c.client.Put(ctx, metaKey, metaBytes, ttl); err != nil { return false, err @@ -305,13 +477,18 @@ func (c *legacyCoordinator) putMeta(ctx context.Context, bucket, key, metaKey st return true, nil } -// deleteMeta is v1.20's invalidation, unchanged: tombstone FIRST (it blocks -// in-flight populates from completing), then the plain metadata delete. Both -// steps are attempted even if the first fails, and a genuine failure of -// either is returned so callers don't report a successful invalidation while -// stale metadata is still readable. +// deleteMeta advances the generation sidecar for current readers, then uses +// v1.20's tombstone-first, plain metadata delete for older peers. All three +// operations are attempted even if one fails, and a genuine failure is +// returned so callers don't report successful invalidation while stale +// metadata may remain readable. func (c *legacyCoordinator) deleteMeta(ctx context.Context, bucket, key string) error { var errs []error + if err := c.advanceGeneration(ctx, bucket, key); err != nil { + log.Debug().Err(err).Str("bucket", bucket).Str("key", key). + Msg("Failed to advance cache generation (continuing with legacy invalidation)") + errs = append(errs, fmt.Errorf("advance generation: %w", err)) + } if err := c.writeTombstone(ctx, bucket, key); err != nil { log.Debug().Err(err).Str("bucket", bucket).Str("key", key). Msg("Failed to write tombstone (continuing with delete)") @@ -330,20 +507,13 @@ func (c *legacyCoordinator) deleteMeta(ctx context.Context, bucket, key string) // unconditional delete at these call sites — a spurious miss and refetch, // never stale data. Closing the window takes CAS; that IS the other mode. func (c *legacyCoordinator) deleteMetaIfETag(ctx context.Context, bucket, key, staleETag string) (bool, error) { - metaBytes, err := c.client.Get(ctx, MakeMetaKey(bucket, key)) + meta, found, err := c.getMeta(ctx, bucket, key) if err != nil { - if isNotFoundError(err) { - return false, nil - } return false, err } - if metaBytes == nil { + if !found || meta == nil { return false, nil } - meta, err := DecodeMeta(metaBytes) - if err != nil { - return false, err - } if meta.ETag != staleETag { return false, nil } @@ -354,25 +524,30 @@ func (c *legacyCoordinator) deleteMetaIfETag(ctx context.Context, bucket, key, s } func (c *legacyCoordinator) writeTombstone(ctx context.Context, bucket, key string) error { - ts := time.Now().UnixNano() data := make([]byte, 8) - binary.BigEndian.PutUint64(data, uint64(ts)) + binary.BigEndian.PutUint64(data, uint64(time.Now().UnixNano())) return c.client.Put(ctx, MakeTombstoneKey(bucket, key), data, c.tombstoneTTL) } -func (c *legacyCoordinator) tombstoneTimestamp(ctx context.Context, bucket, key string) int64 { +func (c *legacyCoordinator) tombstoneTimestamp(ctx context.Context, bucket, key string) (int64, error) { data, err := c.client.Get(ctx, MakeTombstoneKey(bucket, key)) - if err != nil || len(data) != 8 { - return 0 + if err != nil { + if isNotFoundError(err) { + return 0, nil + } + return 0, err + } + if len(data) != 8 { + return 0, nil } - return int64(binary.BigEndian.Uint64(data)) + return int64(binary.BigEndian.Uint64(data)), nil } -// deleteMetaIfVersion is a CAS-strength identity delete: legacy coordination -// stores no versions (its tokens are wall-clock stamps never persisted with -// the row), so there is nothing to compare the caller's token against and the -// only safe answer is to refuse. Its sole caller (the tiered cleanup repair) +// deleteMetaIfVersion is a CAS-strength identity delete: legacy tokens carry a +// sidecar version and tombstone timestamp, but the plain metadata key has no +// per-row CAS version to compare. The only safe answer is to refuse. Its sole +// caller (the tiered cleanup repair) // runs under CAS coordination by construction — tiered mode rejects legacy. -func (c *legacyCoordinator) deleteMetaIfVersion(ctx context.Context, bucket, key string, version uint64) (bool, error) { +func (c *legacyCoordinator) deleteMetaIfVersion(ctx context.Context, bucket, key string, _ MetaVersionToken) (bool, error) { return false, nil } diff --git a/cache/coordinator_test.go b/cache/coordinator_test.go index 764e1af3..63a65334 100644 --- a/cache/coordinator_test.go +++ b/cache/coordinator_test.go @@ -1,7 +1,13 @@ package cache import ( + "bytes" "context" + "encoding/binary" + "errors" + "net/http" + "strings" + "sync/atomic" "testing" "time" @@ -44,8 +50,8 @@ func TestLegacyCoordinator_TombstoneBlocksStalePopulate(t *testing.T) { if err != nil || found { t.Fatalf("decision read: found=%v err=%v", found, err) } - if tok == 0 { - t.Fatal("legacy decision token must be a nonzero stamp") + if tok.decisionTime <= 0 { + t.Fatal("legacy decision token must carry a positive tombstone timestamp") } // ...the invalidation lands (tombstone + delete)... @@ -74,6 +80,72 @@ func TestLegacyCoordinator_TombstoneBlocksStalePopulate(t *testing.T) { } } +// Concurrent misses share the marker created at token capture. A first populate's +// publish generation is reusable by the second; only an invalidation advances it. +func TestLegacyConcurrentFirstPopulatesSharePublishGeneration(t *testing.T) { + ctx := context.Background() + c, _ := newLegacyTestCache() + const bucket, key = "b", "parallel-first-populates" + + _, firstToken, firstFound, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || firstFound { + t.Fatalf("first decision=(%+v, found=%t, err=%v), want decision-time publish marker", firstToken, firstFound, err) + } + _, secondToken, secondFound, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || secondFound || secondToken.version != firstToken.version { + t.Fatalf("second decision=(%+v, found=%t, err=%v), want the shared publish marker", secondToken, secondFound, err) + } + + first := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"first"`, StatusCode: http.StatusOK} + if wrote, err := c.PutMetaIfVersion(ctx, bucket, key, first, 60, firstToken); err != nil || !wrote { + t.Fatalf("first populate=(%t, %v), want success", wrote, err) + } + second := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"second"`, StatusCode: http.StatusOK} + if wrote, err := c.PutMetaIfVersion(ctx, bucket, key, second, 60, secondToken); err != nil || !wrote { + t.Fatalf("concurrent populate=(%t, %v), want legacy last-writer success", wrote, err) + } + meta, found, err := c.GetMeta(ctx, bucket, key) + if err != nil || !found || meta.ETag != second.ETag { + t.Fatalf("final legacy metadata=(%+v, found=%t, err=%v), want the later populate", meta, found, err) + } +} + +// Legacy commits retain the v1.20 timestamp guard in addition to the current +// reader sidecar, so a tombstone from an older peer still blocks stale writers. +func TestLegacyCoordinatorOldPeerTombstoneBlocksRefill(t *testing.T) { + ctx := context.Background() + c, store := newLegacyTestCache() + const bucket, key = "b", "old-peer" + + if err := c.DeleteWithMeta(ctx, bucket, key); err != nil { + t.Fatalf("current-code first invalidation: %v", err) + } + _, token, found, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || found || token.decisionTime <= 0 { + t.Fatalf("decision token=(%+v, found=%t, err=%v), want present generation and a timestamp", token, found, err) + } + + // A v1.20 peer has no generation-sidecar operation. It orders itself only + // through the shared eight-byte tombstone and plain metadata delete. + newerTombstone := uint64(token.decisionTime + 1) + encoded := make([]byte, 8) + binary.BigEndian.PutUint64(encoded, newerTombstone) + if err := store.Put(ctx, MakeTombstoneKey(bucket, key), encoded, c.defaultTTL); err != nil { + t.Fatalf("old-peer tombstone write: %v", err) + } + if err := store.Delete(ctx, MakeMetaKey(bucket, key)); err != nil && !isNotFoundError(err) { + t.Fatalf("old-peer metadata delete: %v", err) + } + + stale := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"stale"`, StatusCode: http.StatusOK} + if wrote, err := c.PutMetaIfVersion(ctx, bucket, key, stale, 60, token); err != nil || wrote { + t.Fatalf("late legacy refill=(wrote=%t, err=%v), want tombstone rejection", wrote, err) + } + if _, found, err := c.GetMeta(ctx, bucket, key); err != nil || found { + t.Fatalf("current reader found stale metadata=%t err=%v", found, err) + } +} + // Legacy DeleteIfETag is the pre-CAS compare-then-delete: removes a matching // entry (leaving a tombstone), spares a mismatched one. func TestLegacyCoordinator_DeleteIfETag(t *testing.T) { @@ -95,34 +167,47 @@ func TestLegacyCoordinator_DeleteIfETag(t *testing.T) { } } -// Legacy mode must never touch the CAS API: a client whose CAS methods panic -// proves the separation is total. -type noCASClient struct { +// Legacy mode keeps plain metadata and tombstone operations for v1.20 +// compatibility. Its CAS operations are isolated to the separate generation +// sidecar so new readers can reject a late plain metadata Put. +type generationSidecarCASClient struct { cacheclient.CacheClient } -func (n *noCASClient) GetWithVersion(context.Context, string) ([]byte, uint64, bool, error) { - panic("CAS op reached a legacy coordinator") +func (c *generationSidecarCASClient) checkGenerationKey(key string) { + if !strings.HasPrefix(key, "meta-gen|") { + panic("CAS operation reached a legacy metadata or tombstone key: " + key) + } +} + +func (c *generationSidecarCASClient) GetWithVersion(ctx context.Context, key string) ([]byte, uint64, bool, error) { + c.checkGenerationKey(key) + return c.CacheClient.GetWithVersion(ctx, key) } -func (n *noCASClient) PutIfVersion(context.Context, string, []byte, int64, uint64) (uint64, error) { - panic("CAS op reached a legacy coordinator") +func (c *generationSidecarCASClient) PutIfVersion(ctx context.Context, key string, data []byte, ttl int64, expected uint64) (uint64, error) { + c.checkGenerationKey(key) + return c.CacheClient.PutIfVersion(ctx, key, data, ttl, expected) } -func (n *noCASClient) DeleteIfVersion(context.Context, string, uint64) error { - panic("CAS op reached a legacy coordinator") +func (c *generationSidecarCASClient) DeleteIfVersion(ctx context.Context, key string, expected uint64) error { + c.checkGenerationKey(key) + return c.CacheClient.DeleteIfVersion(ctx, key, expected) } -func TestLegacyCoordinator_NeverCallsCASAPI(t *testing.T) { +func TestLegacyCoordinatorCASIsIsolatedToGenerationKey(t *testing.T) { cfg := config.NewDefault() - c := NewCacheWithClient(&noCASClient{CacheClient: cacheclient.NewMemoryCache()}, &cfg.Cache) + c := NewCacheWithClient(&generationSidecarCASClient{CacheClient: cacheclient.NewMemoryCache()}, &cfg.Cache) ctx := context.Background() meta := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v1"`, ContentLength: 2, StatusCode: 200} - _, tok, _, _ := c.GetMetaWithVersion(ctx, "b", "k") + _, tok, _, err := c.GetMetaWithVersion(ctx, "b", "k") + if err != nil { + t.Fatalf("legacy decision read: %v", err) + } if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", meta, 60, tok); err != nil || !wrote { t.Fatalf("legacy put: (%v, %v)", wrote, err) } - if _, _, found, _ := c.GetMetaWithVersion(ctx, "b", "k"); !found { - t.Fatal("legacy read: not found") + if _, _, found, err := c.GetMetaWithVersion(ctx, "b", "k"); err != nil || !found { + t.Fatalf("legacy read: found=%t err=%v", found, err) } if _, err := c.DeleteIfETag(ctx, "b", "k", `"v1"`); err != nil { t.Fatalf("legacy guarded delete: %v", err) @@ -130,7 +215,6 @@ func TestLegacyCoordinator_NeverCallsCASAPI(t *testing.T) { if err := c.DeleteWithMeta(ctx, "b", "k"); err != nil { t.Fatalf("legacy delete: %v", err) } - _ = time.Now() } // expected==0 carries no decision-time token, so the legacy coordinator must @@ -140,7 +224,7 @@ func TestLegacyCoordinator_ZeroExpectedRefused(t *testing.T) { ctx := context.Background() meta := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v1"`, StatusCode: 200} - wrote, err := c.PutMetaIfVersion(ctx, "b", "k", meta, 60, 0) + wrote, err := c.PutMetaIfVersion(ctx, "b", "k", meta, 60, MetaVersionToken{}) if err != nil { t.Fatalf("PutMetaIfVersion(expected=0): %v", err) } @@ -151,3 +235,315 @@ func TestLegacyCoordinator_ZeroExpectedRefused(t *testing.T) { t.Fatal("refused write still landed in the store") } } + +// A refill on a second legacy coordinator carries the shared sidecar +// generation, not its local wall clock. A later invalidation advances that +// generation and makes the old metadata write invisible to current readers. +func TestLegacyCoordinator_GenerationSidecarBlocksCrossCoordinatorRefill(t *testing.T) { + ctx := context.Background() + shared := cacheclient.NewMemoryCache() + cfg := config.NewDefault() + deleter := NewCacheWithClient(shared, &cfg.Cache) + refiller := NewCacheWithClient(shared, &cfg.Cache) + + old := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + if err := refiller.PutWithMeta(ctx, "b", "k", old, []byte("old"), 60); err != nil { + t.Fatalf("seed old object: %v", err) + } + if err := deleter.DeleteWithMeta(ctx, "b", "k"); err != nil { + t.Fatalf("first DeleteWithMeta: %v", err) + } + _, staleToken, found, err := refiller.GetMetaWithVersion(ctx, "b", "k") + if err != nil || found { + t.Fatalf("cross-coordinator decision read: found=%v err=%v", found, err) + } + if err := deleter.DeleteWithMeta(ctx, "b", "k"); err != nil { + t.Fatalf("second DeleteWithMeta: %v", err) + } + + stale := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"stale"`, StatusCode: http.StatusOK} + if wrote, err := refiller.PutMetaIfVersion(ctx, "b", "k", stale, 60, staleToken); err != nil || wrote { + t.Fatalf("stale PutMetaIfVersion=(%t,%v), want generation precondition loss", wrote, err) + } + if _, found, err := refiller.GetMeta(ctx, "b", "k"); err != nil || found { + t.Fatalf("current reader found stale metadata=%t err=%v", found, err) + } + + _, freshToken, _, err := refiller.GetMetaWithVersion(ctx, "b", "k") + if err != nil { + t.Fatalf("fresh decision read: %v", err) + } + fresh := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"fresh"`, StatusCode: http.StatusOK} + if wrote, err := refiller.PutMetaIfVersion(ctx, "b", "k", fresh, 60, freshToken); err != nil || !wrote { + t.Fatalf("fresh PutMetaIfVersion=(%t,%v), want successful generation commit", wrote, err) + } +} + +type legacyTombstoneReadFailureClient struct { + cacheclient.CacheClient + tombstoneKey string + err error + fail bool +} + +func (c *legacyTombstoneReadFailureClient) Get(ctx context.Context, key string) ([]byte, error) { + if c.fail && key == c.tombstoneKey { + return nil, c.err + } + return c.CacheClient.Get(ctx, key) +} + +func TestLegacyCoordinatorTombstoneReadFailuresFailClosed(t *testing.T) { + ctx := context.Background() + const bucket, key = "b", "k" + backendErr := errors.New("tombstone store unavailable") + + t.Run("decision token", func(t *testing.T) { + client := &legacyTombstoneReadFailureClient{ + CacheClient: cacheclient.NewMemoryCache(), + tombstoneKey: MakeTombstoneKey(bucket, key), + err: backendErr, + fail: true, + } + cfg := config.NewDefault() + c := NewCacheWithClient(client, &cfg.Cache) + if meta, token, found, err := c.GetMetaWithVersion(ctx, bucket, key); !errors.Is(err, backendErr) || meta != nil || token != (MetaVersionToken{}) || found { + t.Fatalf("GetMetaWithVersion = (%v, %+v, %v, %v), want no token and tombstone read error", meta, token, found, err) + } + }) + + t.Run("meta commit", func(t *testing.T) { + client := &legacyTombstoneReadFailureClient{ + CacheClient: cacheclient.NewMemoryCache(), + tombstoneKey: MakeTombstoneKey(bucket, key), + err: backendErr, + } + cfg := config.NewDefault() + c := NewCacheWithClient(client, &cfg.Cache) + _, token, found, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || found { + t.Fatalf("decision read: found=%v err=%v", found, err) + } + client.fail = true + meta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"stale"`, StatusCode: http.StatusOK} + wrote, err := c.PutMetaIfVersion(ctx, bucket, key, meta, 60, token) + if !errors.Is(err, backendErr) || wrote { + t.Fatalf("PutMetaIfVersion = (%v, %v), want the tombstone read error without a write", wrote, err) + } + if _, found, err := c.GetMeta(ctx, bucket, key); err != nil || found { + t.Fatalf("metadata found=%v err=%v after refused write", found, err) + } + }) + +} + +type generationCASUnavailableClient struct { + cacheclient.CacheClient + err error +} + +func (c *generationCASUnavailableClient) GetWithVersion(ctx context.Context, key string) ([]byte, uint64, bool, error) { + if strings.HasPrefix(key, "meta-gen|") { + return nil, 0, false, c.err + } + return c.CacheClient.GetWithVersion(ctx, key) +} + +func (c *generationCASUnavailableClient) PutIfVersion(ctx context.Context, key string, data []byte, ttl int64, expected uint64) (uint64, error) { + if strings.HasPrefix(key, "meta-gen|") { + return 0, c.err + } + return c.CacheClient.PutIfVersion(ctx, key, data, ttl, expected) +} + +func TestLegacyGenerationSidecarUnavailableFailsClosed(t *testing.T) { + ctx := context.Background() + const bucket, key = "b", "k" + backendErr := errors.New("generation sidecar CAS unsupported") + store := cacheclient.NewMemoryCache() + oldMeta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + oldBytes, err := oldMeta.Encode() // v1.20-style metadata has no generation field + if err != nil { + t.Fatal(err) + } + if err := store.Put(ctx, MakeMetaKey(bucket, key), oldBytes, 3600); err != nil { + t.Fatal(err) + } + if err := store.Put(ctx, MakeBodyKey(bucket, key, oldMeta.ETag), []byte("old"), 3600); err != nil { + t.Fatal(err) + } + + cfg := config.NewDefault() + client := &generationCASUnavailableClient{CacheClient: store, err: backendErr} + c := NewCacheWithClient(client, &cfg.Cache) + if meta, found, err := c.GetMeta(ctx, bucket, key); !errors.Is(err, backendErr) || meta != nil || found { + t.Fatalf("GetMeta = (%v, %v, %v), want fail-closed sidecar error", meta, found, err) + } + if meta, token, found, err := c.GetMetaWithVersion(ctx, bucket, key); !errors.Is(err, backendErr) || meta != nil || token != (MetaVersionToken{}) || found { + t.Fatalf("GetMetaWithVersion = (%v, %+v, %v, %v), want no token on sidecar error", meta, token, found, err) + } + if err := c.DeleteWithMeta(ctx, bucket, key); !errors.Is(err, backendErr) { + t.Fatalf("DeleteWithMeta error=%v, want sidecar invalidation error", err) + } + if _, err := store.Get(ctx, MakeMetaKey(bucket, key)); !isNotFoundError(err) { + t.Fatalf("legacy metadata delete did not continue after sidecar error: err=%v", err) + } +} + +func TestLegacyGenerationSidecarRejectsUntaggedPlainPut(t *testing.T) { + c, store := newLegacyTestCache() + ctx := context.Background() + const bucket, key = "b", "untagged" + + if err := c.DeleteWithMeta(ctx, bucket, key); err != nil { + t.Fatalf("advance generation: %v", err) + } + old := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + oldBytes, err := old.Encode() // an older writer knows only the legacy meta key + if err != nil { + t.Fatal(err) + } + if err := store.Put(ctx, MakeMetaKey(bucket, key), oldBytes, 3600); err != nil { + t.Fatalf("legacy plain metadata Put: %v", err) + } + if meta, found, err := c.GetMeta(ctx, bucket, key); err != nil || found || meta != nil { + t.Fatalf("new TAG reader accepted untagged metadata: (%v, found=%t, err=%v)", meta, found, err) + } + + _, token, found, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || found { + t.Fatalf("new decision read: found=%t err=%v", found, err) + } + fresh := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"fresh"`, ContentLength: 5, StatusCode: http.StatusOK} + if wrote, err := c.PutMetaIfVersion(ctx, bucket, key, fresh, 60, token); err != nil || !wrote { + t.Fatalf("new tagged PutMetaIfVersion=(%t,%v), want success", wrote, err) + } + if got, found, err := c.GetMeta(ctx, bucket, key); err != nil || !found || got.ETag != fresh.ETag { + t.Fatalf("tagged metadata read=(%v, found=%t, err=%v), want fresh entry", got, found, err) + } +} + +type generationCASReadBarrier struct { + cacheclient.CacheClient + generationKey string + armed atomic.Bool + reads atomic.Int32 + bothRead chan struct{} + release chan struct{} +} + +func (c *generationCASReadBarrier) GetWithVersion(ctx context.Context, key string) ([]byte, uint64, bool, error) { + data, version, found, err := c.CacheClient.GetWithVersion(ctx, key) + if key == c.generationKey && c.armed.Load() { + read := c.reads.Add(1) + if read <= 2 { + if read == 2 { + close(c.bothRead) + } + <-c.release + } + } + return data, version, found, err +} + +func TestLegacyGenerationSidecarConcurrentAdvances(t *testing.T) { + ctx := context.Background() + const bucket, key = "b", "concurrent" + shared := &generationCASReadBarrier{ + CacheClient: cacheclient.NewMemoryCache(), + generationKey: makeGenerationKey(bucket, key), + bothRead: make(chan struct{}), + release: make(chan struct{}), + } + cfg := config.NewDefault() + first := NewCacheWithClient(shared, &cfg.Cache) + second := NewCacheWithClient(shared, &cfg.Cache) + old := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + if err := first.PutWithMeta(ctx, bucket, key, old, []byte("old"), 60); err != nil { + t.Fatalf("seed old metadata: %v", err) + } + _, staleToken, found, err := first.GetMetaWithVersion(ctx, bucket, key) + if err != nil || !found { + t.Fatalf("pre-delete read: found=%t err=%v", found, err) + } + + shared.armed.Store(true) + deleteResults := make(chan error, 2) + go func() { deleteResults <- first.DeleteWithMeta(ctx, bucket, key) }() + go func() { deleteResults <- second.DeleteWithMeta(ctx, bucket, key) }() + select { + case <-shared.bothRead: + case <-time.After(5 * time.Second): + t.Fatal("both legacy coordinators did not read the same generation") + } + close(shared.release) + for i := 0; i < 2; i++ { + if err := <-deleteResults; err != nil { + t.Fatalf("concurrent DeleteWithMeta: %v", err) + } + } + + if wrote, err := first.PutMetaIfVersion(ctx, bucket, key, old, 60, staleToken); err != nil || wrote { + t.Fatalf("pre-delete metadata Put=(%t,%v), want generation precondition loss", wrote, err) + } + if meta, found, err := first.GetMeta(ctx, bucket, key); err != nil || found { + t.Fatalf("stale metadata after concurrent generation advances=(%v, found=%t, err=%v)", meta, found, err) + } +} + +func TestLegacyGenerationSidecarEvictionFailsClosed(t *testing.T) { + c, mem := newLegacyTestCache() + ctx := context.Background() + const bucket, key = "b", "evicted-generation" + + meta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + if err := c.PutWithMeta(ctx, bucket, key, meta, []byte("old"), 60); err != nil { + t.Fatalf("seed tagged metadata: %v", err) + } + _, staleToken, found, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil || !found { + t.Fatalf("read tagged metadata: found=%t err=%v", found, err) + } + if err := mem.Delete(ctx, makeGenerationKey(bucket, key)); err != nil { + t.Fatalf("evict generation sidecar: %v", err) + } + if _, found, err := c.GetMeta(ctx, bucket, key); err != nil || found { + t.Fatalf("reader found metadata without its sidecar: found=%t err=%v", found, err) + } + if wrote, err := c.PutMetaIfVersion(ctx, bucket, key, meta, 60, staleToken); err != nil || wrote { + t.Fatalf("writer with evicted generation=(%t,%v), want fail closed", wrote, err) + } +} + +func TestLegacyGenerationSidecarTreatsPreTagMetadataAsMiss(t *testing.T) { + ctx := context.Background() + const bucket, key = "b", "pre-tagged" + store := cacheclient.NewMemoryCache() + legacyMeta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, ContentLength: 3, StatusCode: http.StatusOK} + data, err := legacyMeta.Encode() + if err != nil { + t.Fatal(err) + } + if err := store.Put(ctx, MakeMetaKey(bucket, key), data, 60); err != nil { + t.Fatal(err) + } + + cfg := config.NewDefault() + current := NewCacheWithClient(store, &cfg.Cache) + if meta, found, err := current.GetMeta(ctx, bucket, key); err != nil || found || meta != nil { + t.Fatalf("untagged metadata read=(%v, found=%t, err=%v), want miss", meta, found, err) + } +} + +func TestLegacyGenerationDecisionInitializesSidecar(t *testing.T) { + c, store := newLegacyTestCache() + ctx := context.Background() + _, token, found, err := c.GetMetaWithVersion(ctx, "b", "uncacheable") + if err != nil || found || token.decisionTime <= 0 { + t.Fatalf("decision read=(token=%+v, found=%t, err=%v), want a present decision marker and metadata miss", token, found, err) + } + marker, err := store.Get(ctx, makeGenerationKey("b", "uncacheable")) + if err != nil || !bytes.Equal(marker, []byte{legacyGenerationMarker}) { + t.Fatalf("decision marker=%v err=%v, want an initialized publish marker", marker, err) + } +} diff --git a/cache/etag_versioning_test.go b/cache/etag_versioning_test.go index 8fa576d1..8b31e6f7 100644 --- a/cache/etag_versioning_test.go +++ b/cache/etag_versioning_test.go @@ -129,7 +129,7 @@ func TestETagVersionedBody_EmptyETagNotCachedViaStream(t *testing.T) { body := bytes.NewReader([]byte("body-bytes")) meta := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: "", ContentLength: 10, StatusCode: 200} - if _, err := c.PutWithMetaStreamIfVersion(ctx, bucket, key, meta, body, 60, VersionAny); err != nil { + if _, err := c.PutWithMetaStreamIfVersion(ctx, bucket, key, meta, body, 60, AnyMetaVersionToken()); err != nil { t.Fatalf("PutWithMetaStreamIfVersion: %v", err) } if _, found, _ := c.GetMeta(ctx, bucket, key); found { diff --git a/cache/fenced_delete_test.go b/cache/fenced_delete_test.go index 185055ec..9f56ed54 100644 --- a/cache/fenced_delete_test.go +++ b/cache/fenced_delete_test.go @@ -15,7 +15,7 @@ func TestGetMetaWithVersion_AbsentCarriesToken(t *testing.T) { if err != nil || found { t.Fatalf("absent read: found=%v err=%v", found, err) } - if tok == 0 { + if tok.version == 0 { t.Fatal("absent read returned token 0: the absence token was squashed") } } diff --git a/cache/legacy_fence_eviction_test.go b/cache/legacy_fence_eviction_test.go new file mode 100644 index 00000000..aff4d9e0 --- /dev/null +++ b/cache/legacy_fence_eviction_test.go @@ -0,0 +1,60 @@ +package cache + +import ( + "context" + "net/http" + "testing" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/config" +) + +func TestLegacyFenceRefusesLateCommitAfterMarkerEviction(t *testing.T) { + ctx := context.Background() + const bucket, key = "eviction-bucket", "deleted-key" + store := cacheclient.NewMemoryCache() + cfg := config.NewDefault() + cache := NewCacheWithClient(store, &cfg.Cache) + old := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, StatusCode: http.StatusOK} + if err := cache.PutWithMeta(ctx, bucket, key, old, []byte("old body"), 60); err != nil { + t.Fatalf("seed old metadata: %v", err) + } + + if err := cache.DeleteWithMeta(ctx, bucket, key); err != nil { + t.Fatalf("first invalidation: %v", err) + } + evict := func(key string) { + t.Helper() + if err := store.Delete(ctx, key); err != nil && !isNotFoundError(err) { + t.Fatalf("simulate disk-cap eviction of %q: %v", key, err) + } + } + generationKey := "meta-gen|" + bucket + "|" + key + evict(generationKey) + evict(MakeTombstoneKey(bucket, key)) + + // The GET takes its decision after the first successful invalidation, even + // though cap eviction removed both ordinary cache fence keys. + _, token, found, err := cache.GetMetaWithVersion(ctx, bucket, key) + if err != nil || found { + t.Fatalf("post-first-fence decision=(found=%t, err=%v), want metadata miss", found, err) + } + + if err := cache.DeleteWithMeta(ctx, bucket, key); err != nil { + t.Fatalf("second invalidation: %v", err) + } + evict(generationKey) + evict(MakeTombstoneKey(bucket, key)) + + stale := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old-refill"`, StatusCode: http.StatusOK} + wrote, err := cache.PutMetaIfVersion(ctx, bucket, key, stale, 60, token) + if err != nil { + t.Fatalf("late old metadata commit: %v", err) + } + if wrote { + t.Fatal("late old metadata survived sidecar eviction") + } + if meta, found, err := cache.GetMeta(ctx, bucket, key); err != nil || found || meta != nil { + t.Fatalf("current read=(%+v, found=%t, err=%v), want a miss", meta, found, err) + } +} diff --git a/cache/legacy_mixed_version_test.go b/cache/legacy_mixed_version_test.go new file mode 100644 index 00000000..23482b43 --- /dev/null +++ b/cache/legacy_mixed_version_test.go @@ -0,0 +1,119 @@ +package cache + +import ( + "context" + "encoding/binary" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/config" +) + +type legacyPeerDecisionGate struct { + cacheclient.CacheClient + metaKey string + armed atomic.Bool + readStarted chan struct{} + resumeRead chan struct{} + resumeOnce sync.Once +} + +func (g *legacyPeerDecisionGate) Get(ctx context.Context, key string) ([]byte, error) { + data, err := g.CacheClient.Get(ctx, key) + if key == g.metaKey && g.armed.Swap(false) { + close(g.readStarted) + <-g.resumeRead + } + return data, err +} + +func (g *legacyPeerDecisionGate) resume() { + g.resumeOnce.Do(func() { close(g.resumeRead) }) +} + +func TestLegacyDecisionTimePrecedesLegacyPeerInvalidation(t *testing.T) { + ctx := context.Background() + const bucket, key = "mixed-version-bucket", "deleted-key" + base := cacheclient.NewMemoryCache() + gate := &legacyPeerDecisionGate{ + CacheClient: base, + metaKey: MakeMetaKey(bucket, key), + readStarted: make(chan struct{}), + resumeRead: make(chan struct{}), + } + cfg := config.NewDefault() + cfg.Cache.SetLegacyCoordination(true) + c := NewCacheWithClient(gate, &cfg.Cache) + defer gate.resume() + + old := &CachedObjectMeta{Bucket: bucket, Key: key, ETag: `"old"`, StatusCode: 200} + if err := c.PutWithMeta(ctx, bucket, key, old, []byte("old body"), 60); err != nil { + t.Fatalf("seed old metadata: %v", err) + } + + type decision struct { + found bool + err error + publish func() (bool, error) + } + captured := make(chan decision, 1) + gate.armed.Store(true) + go func() { + _, token, found, err := c.GetMetaWithVersion(ctx, bucket, key) + captured <- decision{ + found: found, + err: err, + publish: func() (bool, error) { + return c.PutWithMetaStreamIfVersion(ctx, bucket, key, old, strings.NewReader("old body"), 60, token) + }, + } + }() + + select { + case <-gate.readStarted: + case <-time.After(5 * time.Second): + t.Fatal("decision read did not reach the metadata snapshot") + } + + // Simulate an older v1.20 peer: it writes the timestamp tombstone and plain + // metadata delete, but knows nothing about the new generation sidecar. + var tombstone [8]byte + binary.BigEndian.PutUint64(tombstone[:], uint64(time.Now().UnixNano())) + if err := base.Put(ctx, MakeTombstoneKey(bucket, key), tombstone[:], TombstoneTTLSeconds(cfg.Cache.SizeThreshold)); err != nil { + t.Fatalf("older peer tombstone: %v", err) + } + if err := base.Delete(ctx, MakeMetaKey(bucket, key)); err != nil { + t.Fatalf("older peer metadata delete: %v", err) + } + gate.resume() + + var result decision + select { + case result = <-captured: + case <-time.After(5 * time.Second): + t.Fatal("decision token read did not finish") + } + if result.err != nil { + t.Fatalf("capture decision token: %v", result.err) + } + if !result.found { + t.Fatal("decision snapshot did not include the old metadata") + } + + wrote, err := result.publish() + if err != nil { + t.Fatalf("late old-object publish: %v", err) + } + if wrote { + t.Fatal("late metadata commit crossed an older-peer tombstone") + } + if meta, found, err := c.GetMeta(ctx, bucket, key); err != nil { + t.Fatalf("read metadata after late publish: %v", err) + } else if found { + t.Fatalf("old metadata remained readable after the older-peer delete: %+v", meta) + } +} diff --git a/cache/object.go b/cache/object.go index 99dbe6d2..8ec15368 100644 --- a/cache/object.go +++ b/cache/object.go @@ -88,6 +88,11 @@ type CachedObjectMeta struct { // outlive its blocks. 0 (entries written before the field existed) means the age is // unknown and no lifetime-sensitive rewrite is allowed. CachedAt int64 `json:"cached_at,omitempty"` + + // cacheGeneration is the legacy coordinator's sidecar token. It is stored + // in cache JSON but never exposed as an HTTP header or public field; older + // TAG readers ignore the unknown JSON property. + cacheGeneration uint64 } // MetaFromHTTPHeaders builds CachedObjectMeta from S3 response headers. @@ -347,7 +352,15 @@ func etagKeyComponent(etag string) string { // having to remember the field exists. func (m *CachedObjectMeta) Encode() ([]byte, error) { m.ContentLengthKnown = m.ContentLength >= 0 - return json.Marshal(m) + type metadataAlias CachedObjectMeta + type wireMeta struct { + metadataAlias + CacheGeneration uint64 `json:"cache_generation,omitempty"` + } + return json.Marshal(wireMeta{ + metadataAlias: metadataAlias(*m), + CacheGeneration: m.cacheGeneration, + }) } // DecodeMeta deserializes JSON bytes to CachedObjectMeta. A row without @@ -355,10 +368,17 @@ func (m *CachedObjectMeta) Encode() ([]byte, error) { // sentinel, where 0 was ambiguous between "empty" and "unknown" — it decodes // as unknown so a non-empty cached object is never served as empty. func DecodeMeta(data []byte) (*CachedObjectMeta, error) { - var meta CachedObjectMeta - if err := json.Unmarshal(data, &meta); err != nil { + type metadataAlias CachedObjectMeta + type wireMeta struct { + metadataAlias + CacheGeneration uint64 `json:"cache_generation,omitempty"` + } + var wire wireMeta + if err := json.Unmarshal(data, &wire); err != nil { return nil, err } + meta := CachedObjectMeta(wire.metadataAlias) + meta.cacheGeneration = wire.CacheGeneration if !meta.ContentLengthKnown && meta.ContentLength == 0 { meta.ContentLength = -1 } @@ -371,6 +391,13 @@ func MakeTombstoneKey(bucket, key string) string { return "tomb|" + bucket + "|" + key } +// makeGenerationKey creates the independent CAS generation sidecar for a +// legacy metadata row. It intentionally uses a different key from the +// v1.20-compatible plain metadata protocol. +func makeGenerationKey(bucket, key string) string { + return "meta-gen|" + bucket + "|" + key +} + // MakeMetaKey creates the cache key for object metadata. func MakeMetaKey(bucket, key string) string { return metaKeyPrefix + bucket + "|" + key diff --git a/cache/object_test.go b/cache/object_test.go index c5725cea..5689eacb 100644 --- a/cache/object_test.go +++ b/cache/object_test.go @@ -450,7 +450,8 @@ func TestCachedObjectMeta_EncodeDecodeMeta(t *testing.T) { UserMetadata: map[string]string{ "X-Amz-Meta-Custom": "value", }, - StatusCode: http.StatusOK, + StatusCode: http.StatusOK, + cacheGeneration: 42, } // Encode @@ -484,6 +485,9 @@ func TestCachedObjectMeta_EncodeDecodeMeta(t *testing.T) { if decoded.StatusCode != original.StatusCode { t.Errorf("StatusCode = %d, want %d", decoded.StatusCode, original.StatusCode) } + if decoded.cacheGeneration != original.cacheGeneration { + t.Errorf("cache generation = %d, want %d", decoded.cacheGeneration, original.cacheGeneration) + } } func TestCachedObjectMeta_IsPublicRead(t *testing.T) { diff --git a/cache/versioned_populate_test.go b/cache/versioned_populate_test.go index fc8771c5..87ad09e4 100644 --- a/cache/versioned_populate_test.go +++ b/cache/versioned_populate_test.go @@ -22,12 +22,12 @@ func TestPutMetaIfVersion_PutIfAbsentRefusesExisting(t *testing.T) { ctx := context.Background() existing := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"racer"`, StatusCode: 200} - if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", existing, 60, 0); err != nil || !wrote { + if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", existing, 60, MetaVersionToken{version: 0}); err != nil || !wrote { t.Fatalf("seed via put-if-absent = (%v, %v), want (true, nil)", wrote, err) } late := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"late"`, StatusCode: 200} - wrote, err := c.PutMetaIfVersion(ctx, "b", "k", late, 60, 0) + wrote, err := c.PutMetaIfVersion(ctx, "b", "k", late, 60, MetaVersionToken{version: 0}) if err != nil { t.Fatalf("PutMetaIfVersion: %v", err) } @@ -46,7 +46,7 @@ func TestPutMetaIfVersion_StaleVersionRefused(t *testing.T) { ctx := context.Background() v1 := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v1"`, StatusCode: 200} - if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v1, 60, 0); err != nil || !wrote { + if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v1, 60, MetaVersionToken{version: 0}); err != nil || !wrote { t.Fatalf("seed: (%v, %v)", wrote, err) } _, version, found, err := c.GetMetaWithVersion(ctx, "b", "k") @@ -56,7 +56,7 @@ func TestPutMetaIfVersion_StaleVersionRefused(t *testing.T) { // The entry moves on (an unconditional refresh)... v2 := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v2"`, StatusCode: 200} - if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v2, 60, VersionAny); err != nil || !wrote { + if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v2, 60, AnyMetaVersionToken()); err != nil || !wrote { t.Fatalf("refresh: (%v, %v)", wrote, err) } @@ -83,13 +83,13 @@ func TestVersionAny_BumpsVersion(t *testing.T) { ctx := context.Background() v1 := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v1"`, StatusCode: 200} - if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v1, 60, VersionAny); err != nil || !wrote { + if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v1, 60, AnyMetaVersionToken()); err != nil || !wrote { t.Fatalf("first: (%v, %v)", wrote, err) } _, ver1, _, _ := mem.GetWithVersion(ctx, MakeMetaKey("b", "k")) v2 := &CachedObjectMeta{Bucket: "b", Key: "k", ETag: `"v2"`, StatusCode: 200} - if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v2, 60, VersionAny); err != nil || !wrote { + if wrote, err := c.PutMetaIfVersion(ctx, "b", "k", v2, 60, AnyMetaVersionToken()); err != nil || !wrote { t.Fatalf("second: (%v, %v)", wrote, err) } _, ver2, _, _ := mem.GetWithVersion(ctx, MakeMetaKey("b", "k")) diff --git a/config/config.go b/config/config.go index bdace42c..2b77c28e 100644 --- a/config/config.go +++ b/config/config.go @@ -315,17 +315,17 @@ type CacheConfig struct { // best-effort background GET (deduplicated and shed under the populate budget). // It costs one extra upstream GET per write, so it defaults to false. WarmOnWrite bool `yaml:"warm_on_write"` - // LegacyCoordination selects the cache's meta-key ordering mechanism - // (cache/coordinator.go). true (the DEFAULT when unset) runs the legacy - // timestamp-tombstone mechanism, byte-faithful to v1.20 — rolling upgrades - // from any earlier release are homogeneous under it. false runs the CAS - // coordinator (ocache fences + versions, no tombstones); set it only when - // EVERY node in the cluster runs a CAS-capable release, and flip via a - // brisk rolling restart (nodes in different modes order writes with - // different mechanisms during that window). Standalone nodes may flip - // immediately after upgrading. Tiered mode requires CAS and selects it - // automatically when this is unset; an explicit true there is rejected - // at startup (see validateMode). + // LegacyCoordination selects the cache's metadata-key protocol + // (cache/coordinator.go). true (the DEFAULT when unset) retains the v1.20 + // timestamp tombstones and plain metadata operations for rolling-upgrade + // compatibility. Current TAG readers also validate a separate CAS + // generation sidecar; they fail closed when its CAS RPC is unavailable, but + // older readers do not consult that sidecar. false uses CAS fences and + // versions on the metadata key; set it only when EVERY node in the cluster + // runs a CAS-capable release, and flip via a brisk rolling restart. Standalone + // nodes may flip immediately after upgrading. Tiered mode requires CAS and + // selects it automatically when this is unset; an explicit true there is + // rejected at startup (see validateMode). LegacyCoordination *bool `yaml:"legacy_coordination"` // BlockCachingEnabled turns on block-aligned caching for large objects (RFC 0001): @@ -362,9 +362,9 @@ func (c *CacheConfig) IsEnabled() bool { return *c.Enabled } -// IsLegacyCoordination returns whether the legacy tombstone coordinator is -// selected (default: true when nil — the safe choice for mixed-version -// clusters; see the field comment). +// IsLegacyCoordination returns whether the legacy-compatible metadata +// coordinator is selected (default: true when nil; see the field comment for +// its mixed-version reader limit). func (c *CacheConfig) IsLegacyCoordination() bool { if c.LegacyCoordination == nil { return true diff --git a/docs/cache-control.md b/docs/cache-control.md index 7bb2dfea..ec450ab1 100644 --- a/docs/cache-control.md +++ b/docs/cache-control.md @@ -14,7 +14,7 @@ TAG supports RFC 7234-compliant cache revalidation. Clients can control caching ## Force Revalidation -Send `Cache-Control: no-cache` or `Cache-Control: max-age=0` to force TAG to check with upstream before serving a cached object. TAG sends a conditional request using the cached ETag. If the object hasn't changed, upstream returns 304 and TAG serves from cache (`X-Cache: HIT`). If changed, TAG streams the new content (`X-Cache: REVALIDATED`). +Send `Cache-Control: no-cache` or `Cache-Control: max-age=0` to force TAG to check with upstream before serving a cached object. TAG sends a conditional request using the cached ETag. If the object hasn't changed, upstream returns 304 and TAG serves from cache (`X-Cache: HIT`). If changed, TAG streams the new content (`X-Cache: REVALIDATED`). In proxy modes, a cacheable 200 revalidation is stored only when a follow-up HEAD confirms the same strong ETag and content length; otherwise TAG still returns the 200 body but does not cache that response. ```bash # Force revalidation on GET @@ -47,6 +47,37 @@ TAG automatically invalidates cached objects when they are modified through TAG: - **DeleteObjects** (bulk) — Cache entries deleted for all keys in the request - **CopyObject** — Cache entry deleted for the destination key +In proxy modes, if TAG attempts a bulk delete but receives no upstream response, +it invalidates each requested key again because the origin may have applied the +delete before its reply was lost. An error before the HTTP request is attempted +does not trigger this second pass. A received 2xx response still preserves a +key's refill when every requested delete for that key reports ``. Tiered +mode removes local state only when its delete is confirmed successful; an error +without a response leaves it intact because the local copy may be the only copy. + +Default legacy mode retains the v1.20 tombstone and plain metadata-key +operations so older TAG peers can keep reading and writing the same entries. +Current TAG writers also stamp metadata with a separate CAS generation. Current +TAG reads compare that generation before serving the body, so a late legacy +plain Put after the lost-response invalidation is not served by an updated TAG +reader. Rows written before generation tagging are treated as misses and rebuilt. +If a cache owner does not support the sidecar CAS operation, updated TAG readers +bypass the cache rather than trust unguarded metadata. A legacy miss creates a +small, TTL-bounded decision marker before fetching; if disk-cap eviction removes +it before commit, the populate is refused rather than recreating an old fence. + +Older TAG readers do not check the generation sidecar. A v1.20 reader uses only +the plain metadata key, and its writer checks the timestamp tombstone before a +separate plain metadata Put. A writer that passed that check can publish after +a later delete, so an older reader can still serve a late plain Put during a +mixed-version rollout. This check-to-Put limit exists in the v1.20 protocol; +a newer TAG binary cannot change an older reader. The legacy setting preserves +wire compatibility for rollouts, not fleet-wide stale-read freedom while +unmodified v1.20 readers remain. The stale-refill guarantee here applies to +TAG readers running this source revision, not to unmodified v1.20 readers. +CAS mode fences metadata tokens atomically and should be enabled only after +every TAG peer in the cluster supports it. + Objects modified directly on Tigris (bypassing TAG) remain in cache until they expire (default TTL: 24 hours, configurable via `TAG_CACHE_TTL`) or are revalidated via `Cache-Control: no-cache`. ## Verifying Cache Behavior diff --git a/docs/configuration.md b/docs/configuration.md index 1fdfda8a..8e87f4ca 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -188,12 +188,14 @@ cache: # default; set false to cache whole objects instead. block_caching_enabled: true - # Meta invalidation/populate coordination. true (default) = legacy tombstone mechanism, - # compatible with any cluster mix — including nodes upgrading directly from <=v1.20. false = - # fenced-CAS coordination (stronger ordering; requires every node in the cluster to be - # CAS-capable BEFORE flipping, then a brisk rolling restart). Standalone deployments may flip - # immediately. Override with TAG_CACHE_LEGACY_COORDINATION (fail-safe: only an explicit - # "false"/"0" selects CAS). + # Meta invalidation/populate coordination. true (default) retains the v1.20 tombstone and + # plain metadata protocol for wire-compatible rollouts, not fleet-wide freshness for v1.20 + # readers. Current TAG readers also require a CAS generation sidecar; older readers ignore it, + # and current readers bypass cache entries when an owner lacks sidecar CAS support. false + # selects fenced-CAS metadata ordering, which requires every node in the cluster to be + # CAS-capable BEFORE flipping, then a brisk rolling restart. + # Standalone deployments may flip immediately. Override with TAG_CACHE_LEGACY_COORDINATION + # (fail-safe: only an explicit "false"/"0" selects CAS). legacy_coordination: true # Block granularity AND the whole-vs-block boundary for every populate path: an object @@ -404,7 +406,7 @@ Controls the embedded cache behavior. TAG uses an embedded OCache instance with | `ttl` | duration | `24h` | Default TTL for cached objects | | `size_threshold` | int64 | `1073741824` | Max object size to cache (bytes) | | `block_caching_enabled` | bool | `true` | Enable block-aligned caching for large objects (RFC 0001) | -| `legacy_coordination` | bool | `true` | Meta coordination mechanism: `true` = legacy tombstones (safe for any cluster mix, incl. direct ≤v1.20 upgrades); `false` = fenced CAS (all nodes must be CAS-capable before flipping) | +| `legacy_coordination` | bool | `true` | Meta coordination: `true` = v1.20 tombstones/plain metadata for wire-compatible rollouts plus a CAS generation sidecar for current readers (v1.20 readers may serve late plain Puts; current readers bypass owners without sidecar CAS); `false` = fenced CAS on metadata (all nodes must be CAS-capable before flipping) | | `block_size` | int64 | `1048576` | Block granularity **and** the read-side whole-vs-block boundary (must stay below ocache's 64 MB compaction threshold) | | `parquet_optimization` | bool | `false` | Prefetch a parquet object's metadata blocks on a tail read (only when the metadata spans more than the tail block) | | `disk_path` | string | `/var/cache/tag` | Path to cache data directory | diff --git a/handlers/block_cache_prefetch_benchmark_test.go b/handlers/block_cache_prefetch_benchmark_test.go index fc6c1b74..95115f63 100644 --- a/handlers/block_cache_prefetch_benchmark_test.go +++ b/handlers/block_cache_prefetch_benchmark_test.go @@ -400,7 +400,7 @@ func newHandlerPrefetchBenchmarkFixture(tb testing.TB, remote bool, blockCount i StatusCode: http.StatusOK, BlockSize: blockSize, } - if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.VersionAny); err != nil || !wrote { + if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.AnyMetaVersionToken()); err != nil || !wrote { tb.Fatalf("seed block metadata = (wrote=%t, err=%v)", wrote, err) } service := proxy.NewService(handlerPrefetchForwarder{}, store, cfg) diff --git a/handlers/remote_block_miss_benchmark_test.go b/handlers/remote_block_miss_benchmark_test.go index 42833191..509034df 100644 --- a/handlers/remote_block_miss_benchmark_test.go +++ b/handlers/remote_block_miss_benchmark_test.go @@ -678,7 +678,7 @@ func (f *remoteBlockMissBenchmarkFixture) seedMeta(tb testing.TB, bucket, key st StatusCode: http.StatusOK, BlockSize: f.blockLen, } - if wrote, err := f.cache.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.VersionAny); err != nil || !wrote { + if wrote, err := f.cache.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.AnyMetaVersionToken()); err != nil || !wrote { tb.Fatalf("seed block metadata = (wrote=%t, err=%v)", wrote, err) } return meta diff --git a/proxy/block_cache.go b/proxy/block_cache.go index 0581d568..c80625c7 100644 --- a/proxy/block_cache.go +++ b/proxy/block_cache.go @@ -792,7 +792,7 @@ func (s *Service) serveFullObjectFromBlockCache( // carrying the serve ETag — an absent or different-ETag entry means the serve-path copy is // already displaced, and its token could order the commit against the wrong history. A // failed or disqualified read just skips the promotion. - var promoToken uint64 + var promoToken cache.MetaVersionToken promoTokenOK := false if !meta.BlocksComplete { if cur, tok, found, terr := s.cache.GetMetaWithVersion(ctx, bucket, key); terr == nil && found && cur != nil && cur.ETag == meta.ETag { @@ -1481,7 +1481,7 @@ func (s *Service) buildBlockMeta(bucket, key string, respHeader http.Header, tot // truncated bytes under a committed length (and poison a later range-path populate that trusts // existing blocks). fetchOneBlock validates block length the same way; this keeps the two block // writers consistent. A body longer than Content-Length is likewise rejected before the meta. -func (s *Service) putBlocksFromStream(ctx context.Context, bucket, key string, meta *cache.CachedObjectMeta, r io.Reader, ttl int, expected uint64) (wrote bool, err error) { +func (s *Service) putBlocksFromStream(ctx context.Context, bucket, key string, meta *cache.CachedObjectMeta, r io.Reader, ttl int, expected cache.MetaVersionToken) (wrote bool, err error) { // On any early return, drain the rest of r. setupCacheListener feeds this from an io.Pipe; if // we stop reading with bytes still queued (a mid-object PutBlockStream error, or an oversize // body), the pipe writer goroutine blocks forever on Write, leaking it and never releasing the @@ -1542,9 +1542,10 @@ func (s *Service) triggerBlockModePopulate(bucket, key, accessKey, secretKey str ctx, cancel := context.WithTimeout(context.Background(), backgroundFetchTimeout) defer cancel() - // Decision-time token BEFORE fetching, so an invalidation that lands - // mid-populate bumps the fence past it and the meta commit loses - // (mirrors the whole-object paths). No token, no ordered commit: skip. + // Decision-time token BEFORE fetching. CAS rejects a raced metadata + // commit; legacy tags the row with the sidecar generation so current + // readers ignore it if invalidation lands mid-populate. No token, no + // ordered commit: skip. // An entry present at decision time does NOT skip: the blocks are // ETag-keyed and useful to it, and finalizeBlockModeMeta's own // re-check backs off the meta write in its favor. @@ -1567,9 +1568,10 @@ func (s *Service) triggerBlockModePopulate(bucket, key, accessKey, secretKey str // attribution there) share this logic instead of re-deriving its race handling. // // expected must be the caller's decision-time token, read BEFORE the blocks were -// fetched (or the HEAD made): an invalidation landing mid-populate bumps the -// fence past it and the meta commit loses. -func (s *Service) finalizeBlockModeMeta(ctx context.Context, bucket, key string, meta *cache.CachedObjectMeta, blockCount int, expected uint64) { +// fetched (or the HEAD made): CAS refuses a replaced metadata row; in legacy +// mode current readers reject metadata tagged with a sidecar generation that an +// invalidation advanced during the populate. +func (s *Service) finalizeBlockModeMeta(ctx context.Context, bucket, key string, meta *cache.CachedObjectMeta, blockCount int, expected cache.MetaVersionToken) { // Re-check that no entry was established concurrently before stamping block-mode meta. // The schedule-time !found gate can go stale during the block fetch: a racing // full-GET miss may have whole-cached the object. Overwriting that with block-mode meta @@ -1580,9 +1582,9 @@ func (s *Service) finalizeBlockModeMeta(ctx context.Context, bucket, key string, return } ttl := int(s.config.Cache.TTL.Seconds()) - // Committed under the decision-time absence token: a racer (a warm, another - // establish) or a fenced delete that landed since bumps the version and the - // lost precondition leaves the newer state in place. + // Committed under the decision-time token: CAS rejects a raced metadata + // version, while legacy tags the row with its sidecar generation so current + // readers ignore it if an invalidation advanced during the populate. wrote, err := s.cache.PutMetaIfVersion(ctx, bucket, key, meta, ttl, expected) if err != nil || !wrote { log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Bool("wrote", wrote).Msg("Block-mode meta not written (precondition lost or error)") diff --git a/proxy/block_cache_integration_test.go b/proxy/block_cache_integration_test.go index 0928c678..24293917 100644 --- a/proxy/block_cache_integration_test.go +++ b/proxy/block_cache_integration_test.go @@ -890,7 +890,7 @@ func TestPutBlocksFromStream_ExactMultipleNoPhantomBlock(t *testing.T) { meta := cache.MetaFromHTTPHeaders(wowBucket, wowKey, http.StatusOK, h) meta.BlockSize = 4 - if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEFGH"), 60, cache.VersionAny); err != nil { + if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEFGH"), 60, cache.AnyMetaVersionToken()); err != nil { t.Fatalf("putBlocksFromStream: %v", err) } for i := int64(0); i <= 1; i++ { @@ -920,7 +920,7 @@ func TestPutBlocksFromStream_MidStreamErrorLeavesMetaUnwritten(t *testing.T) { // One full block, then a read error before the second. r := &errAfterReader{data: []byte("ABCD")} - if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, r, 60, cache.VersionAny); err == nil { + if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, r, 60, cache.AnyMetaVersionToken()); err == nil { t.Fatal("expected an error from the mid-stream read failure") } if _, found, _ := c.GetMeta(context.Background(), wowBucket, wowKey); found { @@ -943,7 +943,7 @@ func TestPutBlocksFromStream_TruncatedStreamLeavesNoShortBlockOrMeta(t *testing. meta.BlockSize = 4 // ...but the body carries only 6 bytes, ending cleanly (no read error). - if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEF"), 60, cache.VersionAny); err == nil { + if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEF"), 60, cache.AnyMetaVersionToken()); err == nil { t.Fatal("expected an error from the truncated body") } if _, found, _ := c.GetMeta(context.Background(), wowBucket, wowKey); found { @@ -968,7 +968,7 @@ func TestPutBlocksFromStream_OversizedStreamLeavesMetaUnwritten(t *testing.T) { meta.BlockSize = 4 // ...but the body carries 6 bytes. - if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEF"), 60, cache.VersionAny); err == nil { + if _, err := svc.putBlocksFromStream(context.Background(), wowBucket, wowKey, meta, strings.NewReader("ABCDEF"), 60, cache.AnyMetaVersionToken()); err == nil { t.Fatal("expected an error from the oversized body") } if _, found, _ := c.GetMeta(context.Background(), wowBucket, wowKey); found { diff --git a/proxy/block_cache_prefetch_test.go b/proxy/block_cache_prefetch_test.go index 994b8f6e..59d0799d 100644 --- a/proxy/block_cache_prefetch_test.go +++ b/proxy/block_cache_prefetch_test.go @@ -547,7 +547,7 @@ func TestBlockCache_PipelineConcurrentFillDoesNotExhaustInlineFetchCap(t *testin StatusCode: http.StatusOK, BlockSize: blockSize, } - if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.VersionAny); err != nil || !wrote { + if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.AnyMetaVersionToken()); err != nil || !wrote { t.Fatalf("seed block metadata = (wrote=%t, err=%v)", wrote, err) } diff --git a/proxy/block_cache_remote_write_test.go b/proxy/block_cache_remote_write_test.go index aef11e92..acf4c387 100644 --- a/proxy/block_cache_remote_write_test.go +++ b/proxy/block_cache_remote_write_test.go @@ -161,7 +161,7 @@ func newGatedAssembledRangeService(t *testing.T, local bool) (*Service, *cache.C StatusCode: http.StatusOK, BlockSize: 4, } - if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.VersionAny); err != nil || !wrote { + if wrote, err := store.PutMetaIfVersion(context.Background(), bucket, key, meta, 60, cache.AnyMetaVersionToken()); err != nil || !wrote { t.Fatalf("seed block meta = (wrote=%t, err=%v)", wrote, err) } return svc, store, client, mock, meta, trace @@ -361,6 +361,10 @@ func TestServeAssembledRange_RemoteWriteRespectsTombstoneVisibility(t *testing.T if result.err != nil || !result.served { t.Fatalf("serveAssembledRange = (served=%t, err=%v)", result.served, result.err) } + _, preDeleteToken, found, err := store.GetMetaWithVersion(context.Background(), meta.Bucket, meta.Key) + if err != nil || !found { + t.Fatalf("pre-delete metadata snapshot=(found=%t, err=%v), want live snapshot", found, err) + } if err := store.Delete(context.Background(), meta.Bucket, meta.Key); err != nil { t.Fatalf("Delete: %v", err) } @@ -370,12 +374,10 @@ func TestServeAssembledRange_RemoteWriteRespectsTombstoneVisibility(t *testing.T if _, found, err := store.GetMeta(context.Background(), meta.Bucket, meta.Key); err != nil || found { t.Fatalf("metadata after fenced detached write = (found=%t, err=%v), want absent", found, err) } - // The invalidation left a fence: a writer holding pre-delete state cannot - // recreate the entry (the ordering the tombstone timestamp used to assert). - preDeleteToken := meta // any stale identity; commit with expected=0-era token must lose - _ = preDeleteToken + // The invalidation left a fence: a writer holding the pre-delete token + // cannot recreate the entry. staleMeta := *meta - if wrote, err := store.PutMetaIfVersion(context.Background(), meta.Bucket, meta.Key, &staleMeta, 60, 1); err != nil { + if wrote, err := store.PutMetaIfVersion(context.Background(), meta.Bucket, meta.Key, &staleMeta, 60, preDeleteToken); err != nil { t.Fatalf("PutMetaIfVersion: %v", err) } else if wrote { t.Fatal("stale pre-delete write recreated the entry over the fence") diff --git a/proxy/cache_background_benchmark_test.go b/proxy/cache_background_benchmark_test.go index a8b91c48..9c38798c 100644 --- a/proxy/cache_background_benchmark_test.go +++ b/proxy/cache_background_benchmark_test.go @@ -133,7 +133,7 @@ func benchmarkBackgroundCachePopulate(b *testing.B, blockMode bool) { started := make(chan struct{}, concurrency) forwarder.setBatch(gate, started) for _, key := range keys { - svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityReadMiss, cache.VersionAny) + svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityReadMiss, cache.AnyMetaVersionToken()) } for range keys { @@ -159,7 +159,7 @@ func benchmarkBackgroundCachePopulate(b *testing.B, blockMode bool) { for { allGone := true for _, key := range keys { - if _, loaded := svc.activeBackgroundFetches.Load(backgroundFetchKey(bucket, key, cache.VersionAny)); loaded { + if _, loaded := svc.activeBackgroundFetches.Load(backgroundFetchKey(bucket, key, cache.AnyMetaVersionToken())); loaded { allGone = false break } diff --git a/proxy/cache_background_test.go b/proxy/cache_background_test.go index 1c6eafa4..83ec2d1e 100644 --- a/proxy/cache_background_test.go +++ b/proxy/cache_background_test.go @@ -69,7 +69,7 @@ func TestFetchFullObjectToCache_DrainsUncacheableBody(t *testing.T) { return resp }) - if err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "no-etag", "access", "secret", false, priorityReadMiss, 0); err != nil { + if err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "no-etag", "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err != nil { t.Fatalf("fetchFullObjectToCache: %v", err) } if source == nil { @@ -100,7 +100,7 @@ func TestFetchFullObjectToCache_DrainsBodyAfterCacheWriteFailure(t *testing.T) { } svc := NewService(forwarder, cacheStore, cfg) - if err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "partial-failure", "access", "secret", false, priorityReadMiss, 0); err == nil { + if err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "partial-failure", "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err == nil { t.Fatal("partial cache write failure was swallowed") } if source == nil { @@ -130,7 +130,7 @@ func TestFetchFullObjectToCache_DrainsBodyWhenBlockScratchUnavailable(t *testing return resp }) - err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "block-scratch-unavailable", "access", "secret", false, priorityReadMiss, 0) + err := svc.fetchFullObjectToCache(context.Background(), "background-bucket", "block-scratch-unavailable", "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}) if !errors.Is(err, errCachePopulateDeclined) { t.Fatalf("fetchFullObjectToCache error = %v, want errCachePopulateDeclined", err) } @@ -159,7 +159,7 @@ func TestFetchFullObjectToCache_WritesWholeBodyDirectly(t *testing.T) { return cacheableGetResponse(body, etag) }) - if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, 0); err != nil { + if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err != nil { t.Fatalf("fetchFullObjectToCache: %v", err) } @@ -201,7 +201,7 @@ func TestFetchFullObjectToCache_SmallWholeObjectFitsWithoutBlockScratch(t *testi return cacheableGetResponse(body, `"small-whole-etag"`) }) - if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, 0); err != nil { + if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err != nil { t.Fatalf("fetchFullObjectToCache: %v", err) } meta, found, err := cacheStore.GetMeta(context.Background(), bucket, key) @@ -247,7 +247,7 @@ func TestFetchFullObjectToCache_WarmWaitsForBlockScratchWithoutHoldingCountSlot( warmDone := make(chan error, 1) go func() { - warmDone <- svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityWarmWrite, 0) + warmDone <- svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityWarmWrite, cache.MetaVersionToken{}) }() select { case <-started: @@ -309,7 +309,7 @@ func TestFetchFullObjectToCache_WritesBlockBodyDirectly(t *testing.T) { return cacheableGetResponse(body, etag) }) - if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, 0); err != nil { + if err := svc.fetchFullObjectToCache(context.Background(), bucket, key, "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err != nil { t.Fatalf("fetchFullObjectToCache: %v", err) } @@ -354,7 +354,7 @@ func TestFetchFullObjectToCache_DetachedWriteSurvivesFetchCancellation(t *testin ctx, cancelContext := context.WithCancel(context.Background()) cancel = cancelContext - if err := svc.fetchFullObjectToCache(ctx, bucket, key, "access", "secret", false, priorityReadMiss, 0); err != nil { + if err := svc.fetchFullObjectToCache(ctx, bucket, key, "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}); err != nil { t.Fatalf("fetchFullObjectToCache: %v", err) } if _, found, err := cacheStore.GetMeta(context.Background(), bucket, key); err != nil || !found { diff --git a/proxy/cache_populate_completion_test.go b/proxy/cache_populate_completion_test.go index 33752da9..8dde958b 100644 --- a/proxy/cache_populate_completion_test.go +++ b/proxy/cache_populate_completion_test.go @@ -153,7 +153,7 @@ func TestSetupCacheListener_PublishesWhenHeadersReadyAtRequestCancellation(t *te broadcaster, false, svc.populateWeight(int64(len(body))), - cache.VersionAny, + cache.AnyMetaVersionToken(), ) if cacheErrCh == nil { t.Fatal("setupCacheListener did not create a cache listener") diff --git a/proxy/caching.go b/proxy/caching.go index cd82233f..a800f2c7 100644 --- a/proxy/caching.go +++ b/proxy/caching.go @@ -165,7 +165,7 @@ func (s *Service) setupCacheListener( broadcaster *broadcast.Broadcaster, slotHeld bool, weight int64, - expected uint64, // decision-time token; the meta commit's CAS precondition + expected cache.MetaVersionToken, // complete decision-time metadata precondition ) (*io.PipeWriter, chan error) { // Bound concurrent cache-populate operations. When the limit is saturated, // skip caching entirely: the object is still served/forwarded from upstream, @@ -250,12 +250,10 @@ func (s *Service) setupCacheListener( // read-back. The whole-vs-block boundary is size, not access pattern (RFC 0001): both // full and range paths converge on one representation per size class. Sub-block objects // keep the single whole-body write. - // The decision-time token from before the upstream fetch: when the - // miss path runs while metadata still exists (a forced-revalidation - // fall-through, an anonymous read of a non-public entry) this - // populate refreshes exactly the version it observed — and loses - // to anything newer, including a fenced delete, instead of - // last-write-winning stale bytes over it. + // The decision-time token from before the upstream fetch: a forced- + // revalidation fall-through or anonymous miss may still have metadata. + // CAS refuses a replaced row; legacy stores this generation in the + // metadata so current readers ignore a late plain Put after invalidation. if s.isBlockEligibleSize(meta.ContentLength) { meta.BlockSize = s.config.Cache.BlockSize _, cacheErr = s.putBlocksFromStream(cacheCtx, bucket, key, meta, sigReader, ttl, expected) @@ -389,7 +387,7 @@ func (s *Service) fetchFullObjectToCache( bucket, key, accessKey, secretKey string, anonymous bool, prio populatePriority, - expected uint64, // meta-write precondition (see putMetaVersioned) + expected cache.MetaVersionToken, // metadata precondition (see putMetaVersioned) ) error { // This is a background fetch whose only purpose is to populate the cache, so // reserve a cache-populate slot up front. If the concurrent-write limit is @@ -424,9 +422,11 @@ func (s *Service) fetchFullObjectToCache( } }() - // No stamp needed: the trigger's decision-time token (expected) predates - // this fetch, so an invalidation landing anywhere in the round-trip bumps - // the fence past it and the meta commit loses atomically. + // The trigger's decision-time token (expected) predates this fetch. CAS + // mode rejects a replaced metadata row atomically. Legacy mode carries the + // sidecar generation into its plain metadata Put; current TAG readers reject + // that row if invalidation advanced the sidecar while the body was fetched. + // Older readers do not consult the sidecar. // Execute full object request (no Range header). An anonymous warm uses an // unsigned request so upstream applies anonymous authorization — 200 only if the @@ -597,20 +597,26 @@ func (s *Service) fetchFullObjectToCache( // key, and the commit precondition. One definition, shared with the tests that // wait on in-flight markers, so a format change cannot silently break their // waits into instant misses. -func backgroundFetchKey(bucket, key string, expected uint64) string { - return fmt.Sprintf("bg:%s/%s|%d", bucket, key, expected) +type backgroundFetchIdentity struct { + bucket, key string + expected cache.MetaVersionToken +} + +func backgroundFetchKey(bucket, key string, expected cache.MetaVersionToken) backgroundFetchIdentity { + return backgroundFetchIdentity{bucket: bucket, key: key, expected: expected} } // cached as public-read (see fetchFullObjectToCache); accessKey/secretKey are then // ignored. Pass anonymous=true exactly when the triggering request was anonymous, so // public-read is only ever inferred from a confirmed anonymous read. -func (s *Service) triggerBackgroundCacheFetch(bucket, key, accessKey, secretKey string, anonymous bool, prio populatePriority, expected uint64) { +func (s *Service) triggerBackgroundCacheFetch(bucket, key, accessKey, secretKey string, anonymous bool, prio populatePriority, expected cache.MetaVersionToken) { // Coalesce only triggers with IDENTICAL commit semantics: the precondition // is part of the dedup key. Keyed by bucket/key alone, a write repair // arriving while an absent-gated warm is in flight would be dropped // WITH its precondition — the in-flight warm then loses to the write's - // newer fence (its token predates it) and the repair that would have - // fixed the surviving state never runs. Distinct-precondition fetches for + // newer CAS fence, or its legacy generation is no longer served by current + // TAG readers; the repair that would have fixed the surviving state never runs. + // Distinct-precondition fetches for // one key are bounded by the distinct races that spawned them, and each is // budget-gated like any populate; identical triggers (a read-miss stampede) // still coalesce to one fetch. @@ -618,7 +624,7 @@ func (s *Service) triggerBackgroundCacheFetch(bucket, key, accessKey, secretKey // Atomic check-and-set: if key exists, an equivalent fetch is already in progress if _, loaded := s.activeBackgroundFetches.LoadOrStore(bcastKey, struct{}{}); loaded { - log.Debug().Str("bucket", bucket).Str("key", key).Uint64("expected", expected).Msg("Equivalent background fetch already in progress, coalescing") + log.Debug().Str("bucket", bucket).Str("key", key).Msg("Equivalent background fetch already in progress, coalescing") return } diff --git a/proxy/delete_objects.go b/proxy/delete_objects.go index 02403bc4..3d7a822e 100644 --- a/proxy/delete_objects.go +++ b/proxy/delete_objects.go @@ -10,6 +10,7 @@ import ( "time" "github.com/rs/zerolog/log" + "github.com/tigrisdata/tag/cache" "github.com/tigrisdata/tag/metrics" ) @@ -99,7 +100,7 @@ func (s *Service) HandleDeleteObjects(w http.ResponseWriter, r *http.Request) er // Tiered: pre-forward tokens per key, so the post-success converge is // ordered — see convergeTieredDelete. type tieredPrior struct { - version uint64 + version cache.MetaVersionToken known bool } var tieredPriors map[string]tieredPrior @@ -124,8 +125,8 @@ func (s *Service) HandleDeleteObjects(w http.ResponseWriter, r *http.Request) er // converge (see HandleDeleteObject). s.claimRetierWrite(bucket, obj.Key) defer s.releaseRetierWrite(bucket, obj.Key) - _, v, known := s.captureMarkerPrior(context.Background(), bucket, obj.Key) - tieredPriors[obj.Key] = tieredPrior{version: v, known: known} + _, token, known := s.captureMarkerPrior(context.Background(), bucket, obj.Key) + tieredPriors[obj.Key] = tieredPrior{version: token, known: known} } } requestedCounts[obj.Key]++ @@ -139,18 +140,29 @@ func (s *Service) HandleDeleteObjects(w http.ResponseWriter, r *http.Request) er // Forward request to upstream, capturing the response so we can tell which // per-object deletes actually succeeded — S3 returns 200 OK with per-key - // elements for partial failures. - capture, err := s.forwarder.ForwardWithCapture(r.Context(), w, r) + // elements for partial failures. The concrete forwarders also report + // whether the HTTP client was called so a local preparation error is not + // mistaken for an ambiguous upstream outcome. + var ( + capture *ResponseCapture + attempted bool + ) + if f, ok := s.forwarder.(captureAttemptForwarder); ok { + capture, attempted, err = f.forwardWithCaptureAttempted(r.Context(), w, r) + } else { + capture, err = s.forwarder.ForwardWithCapture(r.Context(), w, r) + } // Re-invalidate AFTER upstream confirms the deletes, for the same // read-after-write reason as HandleDeleteObject: a GET racing the in-flight - // bulk delete may have re-cached a not-yet-deleted object; this second fence bump - // blocks that stale repopulation. A key is re-invalidated when at least one of - // its requested entries was deleted — i.e. more entries were requested for the - // key than upstream reported as errored. Counting by key (never matching version - // IDs) is robust to VersionId representation differences and to Quiet mode, and - // can never leave a truly-deleted object cached (a success is never an ). - // A key whose entries ALL errored keeps its refill (it's still upstream). + // bulk delete may have re-cached a not-yet-deleted object; this second + // invalidation advances its CAS fence or legacy reader generation. A key is + // re-invalidated when at least one requested entry for it was deleted — + // i.e. more entries were requested for the key than upstream reported as errored. + // Counting by key (never matching version IDs) is robust to VersionId + // representation differences and Quiet mode, and can never leave a truly-deleted + // object cached (a success is never an ). A key whose entries ALL errored + // keeps its refill (it's still upstream). // Routed through invalidateObject so a failed re-invalidation is recorded/logged. if err == nil && capture != nil && capture.StatusCode >= 200 && capture.StatusCode < 300 && s.cache.IsEnabled() { erroredCounts, parsed := erroredDeleteKeyCounts(capture.Body) @@ -168,6 +180,22 @@ func (s *Service) HandleDeleteObjects(w http.ResponseWriter, r *http.Request) er } } + // A Do error has no authoritative per-key result: the origin may have applied + // the batch before its reply was lost. In proxy modes, advance the cache fence + // or legacy reader generation for every distinct requested key using a bounded + // context independent of the client. Keep tiered's success-only converge: its + // local entry may be the only copy. + if err != nil && attempted && requestedCounts != nil && tieredPriors == nil { + // One deadline bounds the whole batch, not each key independently. + // The response error still belongs to the upstream exchange; an expired + // cleanup context is reported by the shared invalidation helper. + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + for key := range requestedCounts { + s.convergeInvalidation(ctx, bucket, key) + } + cancel() + } + // Record metrics status := "success" if err != nil { diff --git a/proxy/delete_objects_lost_response_test.go b/proxy/delete_objects_lost_response_test.go new file mode 100644 index 00000000..fd1353f7 --- /dev/null +++ b/proxy/delete_objects_lost_response_test.go @@ -0,0 +1,305 @@ +package proxy + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/auth" + "github.com/tigrisdata/tag/cache" + "github.com/tigrisdata/tag/config" +) + +type deleteObjectsAttemptForwarder struct { + *mockForwarder + attempted bool +} + +func (f *deleteObjectsAttemptForwarder) forwardWithCaptureAttempted( + ctx context.Context, + w http.ResponseWriter, + r *http.Request, +) (*ResponseCapture, bool, error) { + capture, err := f.ForwardWithCapture(ctx, w, r) + return capture, f.attempted, err +} + +func newDeleteObjectsCoordinationService(forwarder RequestForwarder, legacy bool) (*Service, *cache.Cache) { + cfg := config.NewDefault() + cfg.Cache.SetLegacyCoordination(legacy) + store := cache.NewCacheWithClient(cacheclient.NewMemoryCache(), &cfg.Cache) + return NewService(forwarder, store, cfg), store +} + +func seedDeleteObjectsCache(t *testing.T, c *cache.Cache, bucket string, keys ...string) { + t.Helper() + for _, key := range keys { + meta := &cache.CachedObjectMeta{ + Bucket: bucket, + Key: key, + ETag: `"before-delete-` + key + `"`, + ContentLength: int64(len("old body")), + StatusCode: http.StatusOK, + } + if err := c.PutWithMeta(context.Background(), bucket, key, meta, []byte("old body"), 60); err != nil { + t.Fatalf("seed cache %s/%s: %v", bucket, key, err) + } + } +} + +func deleteObjectsRefillTokens(t *testing.T, ctx context.Context, c *cache.Cache, bucket string, keys ...string) map[string]func() bool { + t.Helper() + refills := make(map[string]func() bool, len(keys)) + for _, key := range keys { + key := key + _, token, found, err := c.GetMetaWithVersion(ctx, bucket, key) + if err != nil { + t.Fatalf("read decision token for %s/%s: %v", bucket, key, err) + } + if found { + t.Fatalf("pre-forward invalidation left %s/%s readable", bucket, key) + } + refills[key] = func() bool { + meta := &cache.CachedObjectMeta{ + Bucket: bucket, + Key: key, + ETag: `"old-refill-` + key + `"`, + ContentLength: int64(len("old body")), + StatusCode: http.StatusOK, + } + wrote, err := c.PutWithMetaStreamIfVersion(context.Background(), bucket, key, meta, strings.NewReader("old body"), 60, token) + if err != nil { + t.Fatalf("publish version-guarded refill for %s/%s: %v", bucket, key, err) + } + return wrote + } + } + return refills +} + +func checkDeleteObjectsRefill(t *testing.T, c *cache.Cache, bucket, key string, refill func() bool, wantVisible bool) { + t.Helper() + wrote := refill() + if wrote != wantVisible { + t.Errorf("version-guarded refill for %s/%s wrote=%t, want %t", bucket, key, wrote, wantVisible) + } + meta, found, err := c.GetMeta(context.Background(), bucket, key) + if err != nil { + t.Fatalf("read post-refill metadata for %s/%s: %v", bucket, key, err) + } + if found != wantVisible { + t.Errorf("post-refill metadata for %s/%s found=%t, want %t", bucket, key, found, wantVisible) + } else if found && meta.ETag != `"old-refill-`+key+`"` { + t.Errorf("post-refill ETag for %s/%s = %q, want old refill", bucket, key, meta.ETag) + } +} + +func TestDeleteObjectsLostResponseRefill(t *testing.T) { + modes := []struct { + name string + legacy bool + }{ + {name: "legacy-tombstones", legacy: true}, + {name: "cas-fences", legacy: false}, + } + + for _, mode := range modes { + mode := mode + t.Run("lost-after-apply/"+mode.name, func(t *testing.T) { + const bucket = "bulk-bucket" + keys := []string{"deleted-key", "second-key"} + requestBody := `deleted-keyv1second-keydeleted-keyv2` + accepted := make(chan []byte, 1) + applyAndLoseReply := make(chan struct{}) + applied := make(chan struct{}) + originErr := make(chan error, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.RawQuery != "delete" { + originErr <- errors.New("origin did not receive DeleteObjects") + return + } + body, err := io.ReadAll(r.Body) + if err != nil { + originErr <- err + return + } + accepted <- body + <-applyAndLoseReply + close(applied) + hijacker, ok := w.(http.Hijacker) + if !ok { + originErr <- errors.New("origin response writer cannot hijack connection") + return + } + conn, _, err := hijacker.Hijack() + if err != nil { + originErr <- err + return + } + _ = conn.Close() + })) + defer server.Close() + defer func() { + select { + case <-applyAndLoseReply: + default: + close(applyAndLoseReply) + } + }() + + base := newBaseForwarder(server.URL, "us-east-1", 10) + base.httpClient = server.Client() + forwarder := &transparentForwarder{ + baseForwarder: base, + proxySigner: auth.NewProxySigner("test-access-key", "test-secret-key"), + upstreamEndpoint: server.URL, + } + forwarder.initInterceptor() + svc, c := newDeleteObjectsCoordinationService(forwarder, mode.legacy) + seedDeleteObjectsCache(t, c, bucket, keys...) + + r := httptest.NewRequest(http.MethodPost, "/"+bucket+"?delete", strings.NewReader(requestBody)) + w := httptest.NewRecorder() + forwardDone := make(chan error, 1) + go func() { forwardDone <- svc.HandleDeleteObjects(w, r) }() + + select { + case got := <-accepted: + if string(got) != requestBody { + t.Fatalf("origin request body = %q, want %q", got, requestBody) + } + case err := <-originErr: + t.Fatalf("origin setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not receive DeleteObjects") + } + + // The GET begins after the service's first invalidation and obtains the + // same decision token as streamFromUpstream. Its old-object response is + // held until after the origin applies the delete and loses its reply. + tokens := deleteObjectsRefillTokens(t, context.Background(), c, bucket, keys...) + close(applyAndLoseReply) + select { + case <-applied: + case err := <-originErr: + t.Fatalf("origin apply/response-loss setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not apply DeleteObjects before losing its reply") + } + + select { + case err := <-forwardDone: + if err == nil || !errors.Is(err, io.EOF) { + t.Errorf("HandleDeleteObjects error = %v, want the original lost-response EOF", err) + } + case <-time.After(5 * time.Second): + t.Fatal("HandleDeleteObjects did not return after the origin lost its reply") + } + for _, key := range keys { + checkDeleteObjectsRefill(t, c, bucket, key, tokens[key], false) + } + }) + + t.Run("canceled-client-context/"+mode.name, func(t *testing.T) { + const bucket = "bulk-bucket" + const key = "cancelled-key" + seededErr := errors.New("upstream response unavailable") + var svc *Service + var c *cache.Cache + var refill func() bool + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + mock := &mockForwarder{captureFunc: func(ctx context.Context, _ http.ResponseWriter, _ *http.Request) (*ResponseCapture, error) { + refill = deleteObjectsRefillTokens(t, ctx, c, bucket, key)[key] + cancel() + return nil, seededErr + }} + forwarder := &deleteObjectsAttemptForwarder{mockForwarder: mock, attempted: true} + svc, c = newDeleteObjectsCoordinationService(forwarder, mode.legacy) + seedDeleteObjectsCache(t, c, bucket, key) + + r := httptest.NewRequest(http.MethodPost, "/"+bucket+"?delete", strings.NewReader(`cancelled-key`)).WithContext(ctx) + err := svc.HandleDeleteObjects(httptest.NewRecorder(), r) + if refill == nil { + t.Fatal("capture racing GET token did not prepare a refill") + } + if ctx.Err() != context.Canceled { + t.Fatalf("test request context error = %v, want canceled", ctx.Err()) + } + if err != seededErr { + t.Errorf("handler error = %v, want original error object %v", err, seededErr) + } + checkDeleteObjectsRefill(t, c, bucket, key, refill, false) + }) + + t.Run("pre-dispatch-error/"+mode.name, func(t *testing.T) { + const bucket = "bulk-bucket" + const key = "pre-dispatch-key" + forwardErr := errors.New("request construction failed") + var c *cache.Cache + var refill func() bool + mock := &mockForwarder{captureFunc: func(ctx context.Context, _ http.ResponseWriter, _ *http.Request) (*ResponseCapture, error) { + refill = deleteObjectsRefillTokens(t, ctx, c, bucket, key)[key] + return nil, forwardErr + }} + forwarder := &deleteObjectsAttemptForwarder{mockForwarder: mock, attempted: false} + svc, c := newDeleteObjectsCoordinationService(forwarder, mode.legacy) + seedDeleteObjectsCache(t, c, bucket, key) + + r := httptest.NewRequest(http.MethodPost, "/"+bucket+"?delete", strings.NewReader(`pre-dispatch-key`)) + err := svc.HandleDeleteObjects(httptest.NewRecorder(), r) + if refill == nil { + t.Fatal("capture racing GET token did not prepare a refill") + } + if err != forwardErr { + t.Errorf("handler error = %v, want original error object %v", err, forwardErr) + } + checkDeleteObjectsRefill(t, c, bucket, key, refill, true) + }) + + t.Run("partial-2xx/"+mode.name, func(t *testing.T) { + const bucket = "bulk-bucket" + keys := []string{"fully-failed", "deleted", "mixed-version"} + requestBody := `fully-faileddeletedmixed-versionv1mixed-versionv2` + responseBody := []byte(`deletedfully-failedAccessDeniedmixed-versionv2AccessDenied`) + var c *cache.Cache + mock := &mockForwarder{captureFunc: func(_ context.Context, w http.ResponseWriter, _ *http.Request) (*ResponseCapture, error) { + tokens := deleteObjectsRefillTokens(t, context.Background(), c, bucket, keys...) + for _, key := range keys { + if !tokens[key]() { + t.Fatalf("could not establish the racing refill for %s", key) + } + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(responseBody) + return &ResponseCapture{StatusCode: http.StatusOK, Body: responseBody, Complete: true}, nil + }} + svc, store := newDeleteObjectsCoordinationService(mock, mode.legacy) + c = store + seedDeleteObjectsCache(t, c, bucket, keys...) + + r := httptest.NewRequest(http.MethodPost, "/"+bucket+"?delete", strings.NewReader(requestBody)) + if err := svc.HandleDeleteObjects(httptest.NewRecorder(), r); err != nil { + t.Errorf("HandleDeleteObjects error for a 2xx response = %v", err) + } + if _, found, err := c.GetMeta(context.Background(), bucket, "fully-failed"); err != nil { + t.Fatalf("read fully failed key: %v", err) + } else if !found { + t.Errorf("refill for a key whose only entry failed was discarded") + } + for _, key := range []string{"deleted", "mixed-version"} { + if _, found, err := c.GetMeta(context.Background(), bucket, key); err != nil { + t.Fatalf("read deleted key %s: %v", key, err) + } else if found { + t.Errorf("refill for key %s with a successful delete entry was retained", key) + } + } + }) + } +} diff --git a/proxy/delete_objects_real_get_test.go b/proxy/delete_objects_real_get_test.go new file mode 100644 index 00000000..1535b5b2 --- /dev/null +++ b/proxy/delete_objects_real_get_test.go @@ -0,0 +1,363 @@ +package proxy + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/auth" + "github.com/tigrisdata/tag/cache" + "github.com/tigrisdata/tag/config" +) + +type deleteObjectsRealGETGate struct { + cacheclient.CacheClient + metaKey string + generationKey string + tombstoneKey string + armed atomic.Bool + tokenOnce sync.Once + tokenCaptured chan struct{} + pauseOnce sync.Once + publishPaused chan struct{} + publishDone chan struct{} + publishOnce sync.Once + releaseOnce sync.Once + release chan struct{} +} + +func newDeleteObjectsRealGETGate(client cacheclient.CacheClient, bucket, key string) *deleteObjectsRealGETGate { + return &deleteObjectsRealGETGate{ + CacheClient: client, + metaKey: cache.MakeMetaKey(bucket, key), + generationKey: "meta-gen|" + bucket + "|" + key, + tombstoneKey: cache.MakeTombstoneKey(bucket, key), + tokenCaptured: make(chan struct{}), + publishPaused: make(chan struct{}), + publishDone: make(chan struct{}), + release: make(chan struct{}), + } +} + +func (g *deleteObjectsRealGETGate) noteToken(key string) { + if g.armed.Load() && (key == g.generationKey || key == g.metaKey || key == g.tombstoneKey) { + g.tokenOnce.Do(func() { close(g.tokenCaptured) }) + } +} + +func (g *deleteObjectsRealGETGate) Get(ctx context.Context, key string) ([]byte, error) { + data, err := g.CacheClient.Get(ctx, key) + g.noteToken(key) + return data, err +} + +func (g *deleteObjectsRealGETGate) GetWithVersion(ctx context.Context, key string) ([]byte, uint64, bool, error) { + data, version, found, err := g.CacheClient.GetWithVersion(ctx, key) + g.noteToken(key) + return data, version, found, err +} + +func (g *deleteObjectsRealGETGate) waitBeforePublish(key string) { + if key != g.metaKey || !g.armed.Load() { + return + } + g.pauseOnce.Do(func() { + close(g.publishPaused) + <-g.release + }) +} + +func (g *deleteObjectsRealGETGate) notePublished(key string) { + if key == g.metaKey && g.armed.Load() { + g.publishOnce.Do(func() { close(g.publishDone) }) + } +} + +func (g *deleteObjectsRealGETGate) Put(ctx context.Context, key string, value []byte, ttl int64) error { + g.waitBeforePublish(key) + err := g.CacheClient.Put(ctx, key, value, ttl) + g.notePublished(key) + return err +} + +func (g *deleteObjectsRealGETGate) PutIfVersion(ctx context.Context, key string, value []byte, ttl int64, expected uint64) (uint64, error) { + g.waitBeforePublish(key) + version, err := g.CacheClient.PutIfVersion(ctx, key, value, ttl, expected) + g.notePublished(key) + return version, err +} + +func (g *deleteObjectsRealGETGate) releasePublish() { + g.releaseOnce.Do(func() { close(g.release) }) +} + +func TestDeleteObjectsLostResponseRealGET(t *testing.T) { + const bucket, key = "bulk-real-get", "old-key" + const oldBody = "old body" + requestBody := `` + key + `` + + for _, mode := range []struct { + name string + legacy bool + }{ + {name: "legacy-generation-sidecar", legacy: true}, + {name: "cas-fences", legacy: false}, + } { + mode := mode + t.Run(mode.name, func(t *testing.T) { + postReceived := make(chan []byte, 1) + closePost := make(chan struct{}) + postClosed := make(chan struct{}) + getReceived := make(chan struct{}, 4) + originErrors := make(chan error, 2) + var deleteApplied atomic.Bool + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.RawQuery == "delete": + body, err := io.ReadAll(r.Body) + if err != nil { + originErrors <- err + return + } + postReceived <- body + <-closePost + deleteApplied.Store(true) + hijacker, ok := w.(http.Hijacker) + if !ok { + originErrors <- errors.New("origin response writer cannot hijack connection") + return + } + conn, _, err := hijacker.Hijack() + if err != nil { + originErrors <- err + return + } + _ = conn.Close() + close(postClosed) + case r.Method == http.MethodGet && r.URL.Path == "/"+bucket+"/"+key: + getReceived <- struct{}{} + if deleteApplied.Load() { + body := "NoSuchKey" + w.Header().Set("Content-Length", "9") + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, body) + return + } + w.Header().Set("Content-Length", "8") + w.Header().Set("Content-Type", "text/plain") + w.Header().Set("ETag", `"old"`) + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, oldBody) + default: + originErrors <- errors.New("unexpected origin request: " + r.Method + " " + r.URL.String()) + w.WriteHeader(http.StatusBadRequest) + } + })) + defer origin.Close() + defer func() { + select { + case <-closePost: + default: + close(closePost) + } + }() + + cfg := config.NewDefault() + cfg.Cache.SetLegacyCoordination(mode.legacy) + gate := newDeleteObjectsRealGETGate(cacheclient.NewMemoryCache(), bucket, key) + store := cache.NewCacheWithClient(gate, &cfg.Cache) + seed := &cache.CachedObjectMeta{ + Bucket: bucket, + Key: key, + ETag: `"old"`, + ContentLength: int64(len(oldBody)), + ContentType: "text/plain", + StatusCode: http.StatusOK, + } + if err := store.PutWithMeta(context.Background(), bucket, key, seed, []byte(oldBody), 60); err != nil { + t.Fatalf("seed old cache entry: %v", err) + } + + base := newBaseForwarder(origin.URL, "us-east-1", 10) + base.httpClient = origin.Client() + forwarder := &transparentForwarder{ + baseForwarder: base, + proxySigner: auth.NewProxySigner("test-access-key", "test-secret-key"), + upstreamEndpoint: origin.URL, + } + forwarder.initInterceptor() + svc := NewService(forwarder, store, cfg) + + deleteReq := httptest.NewRequest(http.MethodPost, "http://gateway/"+bucket+"?delete", strings.NewReader(requestBody)) + deleteReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + deleteDone := make(chan error, 1) + go func() { deleteDone <- svc.HandleDeleteObjects(httptest.NewRecorder(), deleteReq) }() + select { + case body := <-postReceived: + if string(body) != requestBody { + t.Fatalf("origin DeleteObjects body=%q, want %q", body, requestBody) + } + case err := <-originErrors: + t.Fatalf("origin setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not receive DeleteObjects after the first invalidation") + } + + gate.armed.Store(true) + getReq := httptest.NewRequest(http.MethodGet, "http://gateway/"+bucket+"/"+key, nil) + getReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + getRecorder := httptest.NewRecorder() + getDone := make(chan error, 1) + go func() { getDone <- svc.HandleGetObject(getRecorder, getReq) }() + select { + case <-gate.tokenCaptured: + case err := <-originErrors: + t.Fatalf("origin setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("GET did not capture its decision token after the first invalidation") + } + select { + case <-getReceived: + case err := <-originErrors: + t.Fatalf("origin GET: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("racing GET did not reach the origin") + } + select { + case <-gate.publishPaused: + case err := <-originErrors: + t.Fatalf("origin setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("GET did not reach its metadata publish boundary") + } + + close(closePost) + select { + case <-postClosed: + case err := <-originErrors: + t.Fatalf("origin response-loss setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not close the DeleteObjects response") + } + select { + case err := <-deleteDone: + if err == nil || !errors.Is(err, io.EOF) { + t.Fatalf("HandleDeleteObjects error=%v; want original transport EOF", err) + } + case <-time.After(5 * time.Second): + t.Fatal("DeleteObjects handler did not return after its error-path invalidation") + } + + gate.releasePublish() + select { + case <-gate.publishDone: + case <-time.After(5 * time.Second): + t.Fatal("cache writer did not finish its metadata commit") + } + select { + case err := <-getDone: + if err != nil { + t.Fatalf("racing GET error=%v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("racing GET did not finish") + } + if getRecorder.Body.String() != oldBody { + t.Fatalf("racing GET body=%q, want old response snapshot %q", getRecorder.Body.String(), oldBody) + } + + if _, found, err := store.GetMeta(context.Background(), bucket, key); err != nil { + t.Fatalf("read metadata after late publish: %v", err) + } else if found { + t.Fatal("late old metadata remained readable after the error-path generation fence") + } + laterRecorder := httptest.NewRecorder() + laterReq := httptest.NewRequest(http.MethodGet, "http://gateway/"+bucket+"/"+key, nil) + laterReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + if err := svc.HandleGetObject(laterRecorder, laterReq); err != nil { + t.Fatalf("later GET error=%v", err) + } + if laterRecorder.Header().Get(XCacheHeader) == XCacheHit || laterRecorder.Body.String() == oldBody { + t.Fatalf("later GET served stale data: X-Cache=%q body=%q", laterRecorder.Header().Get(XCacheHeader), laterRecorder.Body.String()) + } + }) + } +} + +type deleteObjectsInvalidationCounter struct { + cacheclient.CacheClient + metaKey string + tombstoneKey string + legacy bool + armed atomic.Bool + invalidations atomic.Int32 +} + +func (c *deleteObjectsInvalidationCounter) Put(ctx context.Context, key string, data []byte, ttl int64) error { + err := c.CacheClient.Put(ctx, key, data, ttl) + if c.armed.Load() && c.legacy && key == c.tombstoneKey && err == nil { + c.invalidations.Add(1) + } + return err +} + +func (c *deleteObjectsInvalidationCounter) DeleteIfVersion(ctx context.Context, key string, expected uint64) error { + err := c.CacheClient.DeleteIfVersion(ctx, key, expected) + if c.armed.Load() && !c.legacy && key == c.metaKey && err == nil { + c.invalidations.Add(1) + } + return err +} + +func TestDeleteObjectsPreDispatchControl(t *testing.T) { + const bucket, key = "pre-dispatch-real", "key" + for _, mode := range []struct { + name string + legacy bool + }{ + {name: "legacy-tombstones", legacy: true}, + {name: "cas-fences", legacy: false}, + } { + mode := mode + t.Run(mode.name, func(t *testing.T) { + cfg := config.NewDefault() + cfg.Cache.SetLegacyCoordination(mode.legacy) + counter := &deleteObjectsInvalidationCounter{ + CacheClient: cacheclient.NewMemoryCache(), + metaKey: cache.MakeMetaKey(bucket, key), + tombstoneKey: cache.MakeTombstoneKey(bucket, key), + legacy: mode.legacy, + } + store := cache.NewCacheWithClient(counter, &cfg.Cache) + seedDeleteObjectsCache(t, store, bucket, key) + + base := newBaseForwarder("://invalid", "us-east-1", 10) + forwarder := &transparentForwarder{ + baseForwarder: base, + proxySigner: auth.NewProxySigner("test-access-key", "test-secret-key"), + upstreamEndpoint: "://invalid", + } + forwarder.initInterceptor() + svc := NewService(forwarder, store, cfg) + counter.armed.Store(true) + request := httptest.NewRequest(http.MethodPost, "http://gateway/"+bucket+"?delete", + strings.NewReader(``+key+``)) + err := svc.HandleDeleteObjects(httptest.NewRecorder(), request) + if err == nil || !strings.Contains(err.Error(), "failed to parse endpoint") { + t.Fatalf("HandleDeleteObjects error=%v; want request-build failure before Do", err) + } + if got := counter.invalidations.Load(); got != 1 { + t.Fatalf("successful per-key invalidations=%d; want only the pre-forward pass", got) + } + }) + } +} diff --git a/proxy/delete_objects_revalidation_test.go b/proxy/delete_objects_revalidation_test.go new file mode 100644 index 00000000..9f5e1483 --- /dev/null +++ b/proxy/delete_objects_revalidation_test.go @@ -0,0 +1,218 @@ +package proxy + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + cacheclient "github.com/tigrisdata/ocache/client" + "github.com/tigrisdata/tag/auth" + "github.com/tigrisdata/tag/cache" + "github.com/tigrisdata/tag/config" +) + +func TestDeleteObjectsLostResponseRevalidation(t *testing.T) { + const bucket, key = "revalidation-delete", "old-key" + const oldBody = "old body" + requestBody := `` + key + `` + + for _, mode := range []struct { + name string + legacy bool + }{ + {name: "legacy-tombstones", legacy: true}, + {name: "cas-fences", legacy: false}, + } { + mode := mode + t.Run(mode.name, func(t *testing.T) { + revalidationStarted := make(chan struct{}, 1) + releaseRevalidation := make(chan struct{}) + deleteAccepted := make(chan []byte, 1) + releaseDelete := make(chan struct{}) + deleteResponseLost := make(chan struct{}) + originErrors := make(chan error, 4) + var deleteApplied atomic.Bool + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.RawQuery == "delete": + body, err := io.ReadAll(r.Body) + if err != nil { + originErrors <- err + return + } + deleteAccepted <- body + <-releaseDelete + deleteApplied.Store(true) + hijacker, ok := w.(http.Hijacker) + if !ok { + originErrors <- errors.New("origin response writer cannot hijack connection") + return + } + conn, _, err := hijacker.Hijack() + if err != nil { + originErrors <- err + return + } + _ = conn.Close() + close(deleteResponseLost) + case r.Method == http.MethodGet && r.URL.Path == "/"+bucket+"/"+key && r.Header.Get("If-None-Match") == `"old"`: + // This conditional GET captured the old representation before the + // delete. Its response is held until the delete's error-path fence + // has completed, then arrives too late to be authoritative. + revalidationStarted <- struct{}{} + <-releaseRevalidation + w.Header().Set("Content-Length", "8") + w.Header().Set("Content-Type", "text/plain") + w.Header().Set("ETag", `"old"`) + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, oldBody) + case r.Method == http.MethodHead && r.URL.Path == "/"+bucket+"/"+key: + if deleteApplied.Load() { + w.Header().Set("Content-Length", "9") + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Length", "8") + w.Header().Set("Content-Type", "text/plain") + w.Header().Set("ETag", `"old"`) + w.WriteHeader(http.StatusOK) + case r.Method == http.MethodGet && r.URL.Path == "/"+bucket+"/"+key: + if deleteApplied.Load() { + w.Header().Set("Content-Length", "9") + w.WriteHeader(http.StatusNotFound) + _, _ = io.WriteString(w, "NoSuchKey") + return + } + w.Header().Set("Content-Length", "8") + w.Header().Set("Content-Type", "text/plain") + w.Header().Set("ETag", `"old"`) + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, oldBody) + default: + originErrors <- errors.New("unexpected origin request: " + r.Method + " " + r.URL.String()) + w.WriteHeader(http.StatusBadRequest) + } + })) + defer origin.Close() + defer func() { + select { + case <-releaseDelete: + default: + close(releaseDelete) + } + select { + case <-releaseRevalidation: + default: + close(releaseRevalidation) + } + }() + + cfg := config.NewDefault() + cfg.Cache.SetLegacyCoordination(mode.legacy) + store := cache.NewCacheWithClient(cacheclient.NewMemoryCache(), &cfg.Cache) + seed := &cache.CachedObjectMeta{ + Bucket: bucket, + Key: key, + ETag: `"old"`, + ContentLength: int64(len(oldBody)), + ContentType: "text/plain", + StatusCode: http.StatusOK, + } + if err := store.PutWithMeta(context.Background(), bucket, key, seed, []byte(oldBody), 60); err != nil { + t.Fatalf("seed old cache entry: %v", err) + } + + base := newBaseForwarder(origin.URL, "us-east-1", 10) + base.httpClient = origin.Client() + forwarder := &transparentForwarder{ + baseForwarder: base, + proxySigner: auth.NewProxySigner("test-access-key", "test-secret-key"), + upstreamEndpoint: origin.URL, + } + forwarder.initInterceptor() + svc := NewService(forwarder, store, cfg) + + getReq := httptest.NewRequest(http.MethodGet, "http://gateway/"+bucket+"/"+key, nil) + getReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + getReq.Header.Set("Cache-Control", "no-cache") + getRecorder := httptest.NewRecorder() + getDone := make(chan error, 1) + go func() { getDone <- svc.HandleGetObject(getRecorder, getReq) }() + select { + case <-revalidationStarted: + case err := <-originErrors: + t.Fatalf("origin revalidation setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("conditional GET did not reach the origin") + } + + deleteReq := httptest.NewRequest(http.MethodPost, "http://gateway/"+bucket+"?delete", strings.NewReader(requestBody)) + deleteReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + deleteDone := make(chan error, 1) + go func() { deleteDone <- svc.HandleDeleteObjects(httptest.NewRecorder(), deleteReq) }() + select { + case body := <-deleteAccepted: + if string(body) != requestBody { + t.Fatalf("origin DeleteObjects body=%q, want %q", body, requestBody) + } + case err := <-originErrors: + t.Fatalf("origin DeleteObjects setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not receive DeleteObjects after the first invalidation") + } + + close(releaseDelete) + select { + case <-deleteResponseLost: + case err := <-originErrors: + t.Fatalf("origin response-loss setup: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("origin did not apply DeleteObjects before losing its reply") + } + select { + case err := <-deleteDone: + if err == nil || !errors.Is(err, io.EOF) { + t.Fatalf("HandleDeleteObjects error=%v; want original transport EOF", err) + } + case <-time.After(5 * time.Second): + t.Fatal("DeleteObjects handler did not return after its error-path invalidation") + } + + close(releaseRevalidation) + select { + case err := <-getDone: + if err != nil { + t.Fatalf("revalidation GET error=%v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("revalidation GET did not finish") + } + if getRecorder.Code != http.StatusOK || getRecorder.Body.String() != oldBody { + t.Fatalf("in-flight revalidation response=(%d,%q), want the already-received old body", getRecorder.Code, getRecorder.Body.String()) + } + + if _, found, err := store.GetMeta(context.Background(), bucket, key); err != nil { + t.Fatalf("read metadata after delayed revalidation: %v", err) + } else if found { + t.Fatal("late revalidation metadata remained readable after the lost DeleteObjects reply") + } + + laterRecorder := httptest.NewRecorder() + laterReq := httptest.NewRequest(http.MethodGet, "http://gateway/"+bucket+"/"+key, nil) + laterReq.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=test") + if err := svc.HandleGetObject(laterRecorder, laterReq); err != nil { + t.Fatalf("later GET error=%v", err) + } + if laterRecorder.Header().Get(XCacheHeader) == XCacheHit || laterRecorder.Body.String() == oldBody { + t.Fatalf("later GET served stale data: X-Cache=%q body=%q", laterRecorder.Header().Get(XCacheHeader), laterRecorder.Body.String()) + } + }) + } +} diff --git a/proxy/forwarder.go b/proxy/forwarder.go index 076f79fa..aba6c171 100644 --- a/proxy/forwarder.go +++ b/proxy/forwarder.go @@ -73,10 +73,10 @@ type RequestForwarder interface { // Caller is responsible for closing the response body. DoConditionalGetRequest(ctx context.Context, bucket, key, accessKey, secretKey, etag string, lastModified int64, rangeHeader string) (*http.Response, error) - // DoConditionalHeadRequest executes a conditional HEAD for cache revalidation of HEAD requests. - // Sends If-None-Match and/or If-Modified-Since headers. - // Returns 304 if unchanged, 200 with headers only if changed (no body). - // Caller is responsible for closing the response body. + // DoConditionalHeadRequest executes a HEAD for cache revalidation. Non-empty + // validators add If-None-Match and/or If-Modified-Since; empty validators + // send an unconditional HEAD to confirm current object metadata. The response + // has headers only. Caller is responsible for closing its body. DoConditionalHeadRequest(ctx context.Context, bucket, key, accessKey, secretKey, etag string, lastModified int64) (*http.Response, error) } @@ -160,6 +160,13 @@ var ( _ RequestForwarder = (*transparentForwarder)(nil) ) +// captureAttemptForwarder exposes whether capture forwarding reached http.Client.Do. +// An attempted call has an unknown upstream outcome when Do returns an error; it does +// not assert that the origin received the request. +type captureAttemptForwarder interface { + forwardWithCaptureAttempted(ctx context.Context, w http.ResponseWriter, r *http.Request) (*ResponseCapture, bool, error) +} + // ResponseInterceptor is called after receiving the upstream response but before // headers are sent to the client. Used by transparentForwarder to extract signing // keys and strip internal headers from the response. @@ -254,7 +261,7 @@ func (b *baseForwarder) executeAndStreamWithMeta(w http.ResponseWriter, fwdReq * // executeAndCapture executes the request, streams to client, and captures the response. // originalReq is the original client request, passed to the response interceptor // for parsing auth info. It can be nil if no interceptor is set. -func (b *baseForwarder) executeAndCapture(w http.ResponseWriter, fwdReq *http.Request, inContentLength int64, originalReq *http.Request) (*ResponseCapture, error) { +func (b *baseForwarder) executeAndCapture(w http.ResponseWriter, fwdReq *http.Request, inContentLength int64, originalReq *http.Request) (*ResponseCapture, bool, error) { if inContentLength > 0 { metrics.BytesTransferred.WithLabelValues("in").Add(float64(inContentLength)) } @@ -264,7 +271,7 @@ func (b *baseForwarder) executeAndCapture(w http.ResponseWriter, fwdReq *http.Re metrics.RecordUpstreamRequest(fwdReq.Method, time.Since(upstreamStart).Seconds(), err) if err != nil { log.Error().Err(err).Str("method", fwdReq.Method).Str("path", fwdReq.URL.Path).Msg("Failed to forward request") - return nil, err + return nil, true, err } defer resp.Body.Close() @@ -297,7 +304,7 @@ func (b *baseForwarder) executeAndCapture(w http.ResponseWriter, fwdReq *http.Re metrics.BytesTransferred.WithLabelValues("out").Add(float64(len(capture.Body))) logUpstreamResponse(fwdReq, originalReq, resp.StatusCode, upstreamStart) - return capture, nil + return capture, true, nil } // executeRequest executes the request and returns the raw response. @@ -421,10 +428,9 @@ func (b *baseForwarder) DoConditionalGetRequest(ctx context.Context, bucket, key return b.doConditionalRequest(ctx, "GET", bucket, key, accessKey, secretKey, etag, lastModified, rangeHeader) } -// DoConditionalHeadRequest executes a conditional HEAD request to upstream. -// Used for cache revalidation of HEAD requests: sends If-None-Match and/or -// If-Modified-Since headers to check if a cached object is still fresh. -// Returns 304 Not Modified if unchanged, 200 OK with headers only if changed. +// DoConditionalHeadRequest executes a HEAD request to upstream. +// Non-empty validators add If-None-Match and/or If-Modified-Since; empty +// validators send an unconditional HEAD to confirm current object metadata. // Caller is responsible for closing the response body. func (b *baseForwarder) DoConditionalHeadRequest(ctx context.Context, bucket, key, accessKey, secretKey, etag string, lastModified int64) (*http.Response, error) { return b.doConditionalRequest(ctx, "HEAD", bucket, key, accessKey, secretKey, etag, lastModified, "") diff --git a/proxy/forwarder_attempt_test.go b/proxy/forwarder_attempt_test.go new file mode 100644 index 00000000..58773949 --- /dev/null +++ b/proxy/forwarder_attempt_test.go @@ -0,0 +1,102 @@ +package proxy + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/tigrisdata/tag/auth" +) + +func TestForwardWithCaptureAttemptedReportsDoBoundary(t *testing.T) { + t.Run("transparent-build-error-is-not-attempted", func(t *testing.T) { + var calls int + base := newBaseForwarder("http://%", "us-east-1", 1) + base.httpClient = &http.Client{Transport: forwarderTestTransport(func(*http.Request) (*http.Response, error) { + calls++ + return nil, errors.New("unexpected HTTP client call") + })} + forwarder := &transparentForwarder{ + baseForwarder: base, + proxySigner: auth.NewProxySigner("test-access-key", "test-secret-key"), + upstreamEndpoint: "http://%", + } + capture, attempted, err := forwarder.forwardWithCaptureAttempted(context.Background(), httptest.NewRecorder(), newTransparentRequestForTest(t)) + if err == nil || attempted || capture != nil || calls != 0 { + t.Fatalf("capture=%v attempted=%t err=%v HTTP calls=%d; want local error before Do", capture, attempted, err, calls) + } + }) + + t.Run("transparent-transport-error-is-attempted", func(t *testing.T) { + transportErr := errors.New("round trip failed") + forwarder := newTransparentForwarderForTest(forwarderTestTransport(func(*http.Request) (*http.Response, error) { + return nil, transportErr + })) + capture, attempted, err := forwarder.forwardWithCaptureAttempted(context.Background(), httptest.NewRecorder(), newTransparentRequestForTest(t)) + if capture != nil || !attempted || !errors.Is(err, transportErr) { + t.Fatalf("capture=%v attempted=%t err=%v; want attempted transport error %v", capture, attempted, err, transportErr) + } + }) + + t.Run("signing-validation-error-is-not-attempted", func(t *testing.T) { + var calls int + store := auth.NewCredentialStore() + base := newBaseForwarder("https://upstream.example.com", "us-east-1", 1) + base.httpClient = &http.Client{Transport: forwarderTestTransport(func(*http.Request) (*http.Response, error) { + calls++ + return nil, errors.New("unexpected HTTP client call") + })} + forwarder := &signingForwarder{ + baseForwarder: base, + credStore: store, + validator: auth.NewRequestValidator(store), + } + r := httptest.NewRequest(http.MethodPost, "https://client.example.com/bulk-bucket?delete", strings.NewReader(``)) + capture, attempted, err := forwarder.forwardWithCaptureAttempted(context.Background(), httptest.NewRecorder(), r) + if err == nil || attempted || capture != nil || calls != 0 { + t.Fatalf("capture=%v attempted=%t err=%v HTTP calls=%d; want validation error before Do", capture, attempted, err, calls) + } + }) + + t.Run("signing-transport-error-is-attempted", func(t *testing.T) { + const accessKey = "AKIAIOSFODNN7EXAMPLE" + const secretKey = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" + body := `key` + hash := sha256.Sum256([]byte(body)) + request, err := auth.NewRequestSigner("https://client.example.com", "us-east-1").SignRequest( + context.Background(), + http.MethodPost, + "/bulk-bucket?delete", + strings.NewReader(body), + hex.EncodeToString(hash[:]), + accessKey, + secretKey, + http.Header{}, + ) + if err != nil { + t.Fatalf("sign test request: %v", err) + } + + transportErr := errors.New("round trip failed") + store := auth.NewCredentialStore() + store.AddCredential(accessKey, secretKey) + base := newBaseForwarder("https://upstream.example.com", "us-east-1", 1) + base.httpClient = &http.Client{Transport: forwarderTestTransport(func(*http.Request) (*http.Response, error) { + return nil, transportErr + })} + forwarder := &signingForwarder{ + baseForwarder: base, + credStore: store, + validator: auth.NewRequestValidator(store), + } + capture, attempted, err := forwarder.forwardWithCaptureAttempted(context.Background(), httptest.NewRecorder(), request) + if capture != nil || !attempted || !errors.Is(err, transportErr) { + t.Fatalf("capture=%v attempted=%t err=%v; want attempted transport error %v", capture, attempted, err, transportErr) + } + }) +} diff --git a/proxy/forwarder_signing.go b/proxy/forwarder_signing.go index 9f802d90..2b4c5f38 100644 --- a/proxy/forwarder_signing.go +++ b/proxy/forwarder_signing.go @@ -104,6 +104,11 @@ func (f *signingForwarder) ForwardTeeingBody(ctx context.Context, w http.Respons // Validates and re-signs like Forward, but also captures the response body // for caching while streaming to the client. func (f *signingForwarder) ForwardWithCapture(ctx context.Context, w http.ResponseWriter, r *http.Request) (*ResponseCapture, error) { + capture, _, err := f.forwardWithCaptureAttempted(ctx, w, r) + return capture, err +} + +func (f *signingForwarder) forwardWithCaptureAttempted(ctx context.Context, w http.ResponseWriter, r *http.Request) (*ResponseCapture, bool, error) { // Decode AWS chunked encoding if present, otherwise pass through unchanged body, bodyHash, contentLength, chunked := decodeChunkedIfNeeded(r) @@ -111,13 +116,13 @@ func (f *signingForwarder) ForwardWithCapture(ctx context.Context, w http.Respon accessKey, err := f.validator.ValidateRequest(r) if err != nil { log.Warn().Err(err).Str("path", r.URL.Path).Msg("Request signature validation failed") - return nil, mapAuthError(err) + return nil, false, mapAuthError(err) } // Look up secret key secretKey, err := f.credStore.GetSecretKey(accessKey) if err != nil { - return nil, mapAuthError(err) + return nil, false, mapAuthError(err) } // Build the path with query string @@ -129,7 +134,7 @@ func (f *signingForwarder) ForwardWithCapture(ctx context.Context, w http.Respon // Create signed request (passes body hash, streams body directly) fwdReq, err := f.signer.SignRequest(ctx, r.Method, path, body, bodyHash, accessKey, secretKey, r.Header) if err != nil { - return nil, err + return nil, false, err } prepareForwardedRequest(fwdReq, contentLength, chunked) diff --git a/proxy/forwarder_transparent.go b/proxy/forwarder_transparent.go index 9a9ab6f4..bcd789cc 100644 --- a/proxy/forwarder_transparent.go +++ b/proxy/forwarder_transparent.go @@ -176,9 +176,14 @@ func (f *transparentForwarder) Forward(ctx context.Context, w http.ResponseWrite // Response interception (signing key learning, header stripping, authZ revocation) // is handled by the base forwarder's response interceptor. func (f *transparentForwarder) ForwardWithCapture(ctx context.Context, w http.ResponseWriter, r *http.Request) (*ResponseCapture, error) { + capture, _, err := f.forwardWithCaptureAttempted(ctx, w, r) + return capture, err +} + +func (f *transparentForwarder) forwardWithCaptureAttempted(ctx context.Context, w http.ResponseWriter, r *http.Request) (*ResponseCapture, bool, error) { fwdReq, err := f.buildTransparentRequest(ctx, r) if err != nil { - return nil, err + return nil, false, err } return f.executeAndCapture(w, fwdReq, r.ContentLength, r) diff --git a/proxy/get_object.go b/proxy/get_object.go index ed3c1731..b270b38f 100644 --- a/proxy/get_object.go +++ b/proxy/get_object.go @@ -342,9 +342,9 @@ func (s *Service) fetchAndBroadcast( if upErr := broadcaster.Error(); upErr == nil || errors.Is(upErr, context.Canceled) || errors.Is(upErr, context.DeadlineExceeded) { if _, tok, found, gerr := s.cache.GetMetaWithVersion(context.Background(), bucket, key); gerr == nil && !found { - // Absent-gated, carrying the absence TOKEN (ocache v1.13.0): - // the warm is ordered against a fenced delete landing after - // this look, where a bare put-if-absent would recreate over it. + // Carry the absent-state token (metadata version or legacy + // generation sidecar) into the warm so a later invalidation + // rejects its commit or makes it invisible to current readers. s.triggerBackgroundCacheFetch(bucket, key, accessKey, secretKey, hasNoAuthCredentials(r), priorityReadMiss, tok) } } @@ -366,18 +366,15 @@ func (s *Service) streamFromUpstream( bucket, key, accessKey, secretKey string, broadcaster *broadcast.Broadcaster, ) error { - // Stamp the cache-write start BEFORE issuing the upstream request, not after - // The populate's DECISION-TIME token, read before the upstream request: the - // commit applies only if the entry is unchanged from this instant — a - // fenced delete (or any write) landing while the body streams makes the - // commit lose atomically, never retried with the same bytes. This is the - // refill pattern the fence contract requires (ocache v1.13.0): a token - // read after the fetch could postdate a delete and resurrect pre-delete - // bytes. Covers presence (live version) and absence (absence token) alike. + // Capture the cache-write decision token BEFORE issuing the upstream request. + // CAS mode carries the metadata-key version; legacy mode carries both the + // decision-time tombstone stamp and generation-sidecar version. A later + // invalidation makes CAS reject the commit or makes current TAG readers reject + // a legacy row with an older generation. A + // token read after fetch could postdate the delete and label old bytes fresh. _, expected, _, tokErr := s.cache.GetMetaWithVersion(ctx, bucket, key) - // Without a token the commit cannot be ordered — expected=0 is the LEGACY - // unordered put-if-absent, which would publish over a fence. The safe - // failure is not populating at all: tokenOK gates the cache listener below. + // Without a token the commit cannot be ordered. A read error must not fall + // back to VersionAny or expected=0; tokenOK gates the cache listener below. tokenOK := tokErr == nil // Execute upstream request @@ -838,9 +835,9 @@ func (s *Service) handleRangeWithBackgroundCache( } else if cacheable { defer func() { if _, tok, found, gerr := s.cache.GetMetaWithVersion(context.Background(), bucket, key); gerr == nil && !found { - // Absent-gated, carrying the absence TOKEN (ocache v1.13.0): - // the warm is ordered against a fenced delete landing after - // this look, where a bare put-if-absent would recreate over it. + // Carry the absent-state token (metadata version or legacy + // generation sidecar) into the warm so a later invalidation + // rejects its commit or makes it invisible to current readers. s.triggerBackgroundCacheFetch(bucket, key, accessKey, secretKey, hasNoAuthCredentials(r), priorityReadMiss, tok) } }() diff --git a/proxy/localstore.go b/proxy/localstore.go index 762c8369..13e629a5 100644 --- a/proxy/localstore.go +++ b/proxy/localstore.go @@ -98,7 +98,7 @@ func (s *Service) HandleOriginlessObject(w http.ResponseWriter, r *http.Request) // existed before, during, and after the overwrite. One read, one decision — // a read racing an overwrite serves the version its snapshot saw, the legal // atomic-replace answer (and the hottest path saves a doubled meta read). -func (s *Service) serveOriginlessObject(w http.ResponseWriter, r *http.Request, operation string, start time.Time, meta *cache.CachedObjectMeta, metaVersion uint64) error { +func (s *Service) serveOriginlessObject(w http.ResponseWriter, r *http.Request, operation string, start time.Time, meta *cache.CachedObjectMeta, metaVersion cache.MetaVersionToken) error { ctx := r.Context() bucket, key := ParseBucketKey(r) @@ -171,7 +171,7 @@ func (s *Service) serveOriginlessObject(w http.ResponseWriter, r *http.Request, // NOT absence. Minting an authoritative NoSuchKey here would tell the // caller a live object does not exist — so it propagates as a retryable // 5xx, matching this handler's meta-read and probe legs. -func (s *Service) finishServeBodyError(ctx context.Context, w http.ResponseWriter, r *http.Request, bucket, key string, metaVersion uint64, serveErr error, operation string, start time.Time) error { +func (s *Service) finishServeBodyError(ctx context.Context, w http.ResponseWriter, r *http.Request, bucket, key string, metaVersion cache.MetaVersionToken, serveErr error, operation string, start time.Time) error { if !bodyGone(serveErr) { metrics.RecordRequest(operation, "error", metrics.SourceLocal, time.Since(start).Seconds()) return serveErr @@ -301,7 +301,7 @@ func (s *Service) HandleOriginlessPut(w http.ResponseWriter, r *http.Request) er // marker committing between two reads) and the doubled hot-path meta read. type putPrior struct { meta *cache.CachedObjectMeta - version uint64 + version cache.MetaVersionToken found bool } @@ -319,26 +319,23 @@ func (s *Service) handleOriginlessPut(w http.ResponseWriter, r *http.Request, pr return nil } - // EVERY store commits under a decision-time token read here, before the - // body is consumed: a DELETE (or competing PUT) that lands after this - // instant refuses the commit — the ordering the pre-coordinator engine got - // from stamping writeStartTime at handler start, now expressed as the - // opaque token the selected coordinator orders by (fenced version under - // CAS, wall-clock stamp under legacy). For an absent key the token is the - // nonzero absence token, never 0 (ocache v1.13 contract). + // EVERY store captures a decision-time token here, before the body is + // consumed. CAS mode uses the metadata-key version; legacy mode carries the + // reader-generation sidecar and tombstone timestamp. A later legacy + // invalidation makes that row invisible to current TAG readers, even if the + // plain metadata Put completes late. For an absent key, the token preserves + // both the sidecar observation and tombstone decision, not an unordered write. // - // Conditional writes additionally evaluate the precondition here and then - // ENFORCE it at the store: a concurrent write between check and store - // surfaces as a refused store (answered 412) instead of a silent lost - // update. That closure is CAS-coordinator strength; under legacy - // coordination the token orders against DELETEs only, and write-vs-write - // remains the check-then-store race the pre-CAS engine documented as - // accepted. Semantics follow the ceph suite: If-Match against a MISSING + // Conditional writes additionally evaluate the precondition here. CAS + // enforces it atomically at the store. Legacy retains the check-and-plain- + // Put protocol for older peers; a competing write can still race that + // check, and older readers do not consult the generation sidecar. Semantics + // follow the ceph suite: If-Match against a MISSING // object answers NoSuchKey (there is nothing to match), a // present-but-different ETag is the 412; If-None-Match refuses when the // object exists. var existing *cache.CachedObjectMeta - var expected uint64 + var expected cache.MetaVersionToken var found bool if prior != nil { // The caller's snapshot IS the decision state: reuse it so the tier @@ -577,7 +574,7 @@ func (s *Service) handleOriginlessPut(w http.ResponseWriter, r *http.Request, pr // conditional writes answer the 412 their precondition earned (and // reclaim the staged body), unconditional writes retry under a fresh // token so the client's 200 always means the entry is visible. - store := func(token uint64) (bool, error) { + store := func(token cache.MetaVersionToken) (bool, error) { return s.cache.PutMetaIfVersion(ctx, bucket, key, meta, ttl, token) } var wrote bool diff --git a/proxy/meta_on_write_measure_test.go b/proxy/meta_on_write_measure_test.go index 3e47c1eb..037e0639 100644 --- a/proxy/meta_on_write_measure_test.go +++ b/proxy/meta_on_write_measure_test.go @@ -123,7 +123,7 @@ func TestMeasure_FirstOpenUpstreamRoundTrips(t *testing.T) { // of this measurement produced "no improvement". func mustPrimeMeta(t *testing.T, c *cache.Cache, bucket, key string, meta *cache.CachedObjectMeta) { t.Helper() - wrote, err := c.PutMetaIfVersion(context.Background(), bucket, key, meta, 3600, cache.VersionAny) + wrote, err := c.PutMetaIfVersion(context.Background(), bucket, key, meta, 3600, cache.AnyMetaVersionToken()) if err != nil || !wrote { t.Fatalf("prime meta: wrote=%v err=%v", wrote, err) } diff --git a/proxy/revalidation.go b/proxy/revalidation.go index 245baed6..975348e5 100644 --- a/proxy/revalidation.go +++ b/proxy/revalidation.go @@ -75,7 +75,7 @@ func (s *Service) revalidateAndServe( return s.forwardAfterCacheMiss(ctx, w, r, bucket, key, meta.ETag, revalErr, start) case http.StatusOK: // Full-object response (no range or upstream ignored range) - return s.handleRevalidation200(ctx, w, bucket, key, meta.ETag, resp, start) + return s.handleRevalidation200(ctx, w, bucket, key, accessKey, secretKey, meta.ETag, resp, start) case http.StatusPartialContent: // Range response — object changed, upstream returned only the requested range return s.handleRevalidation206Range(ctx, w, r, bucket, key, accessKey, secretKey, meta.ETag, resp, start) @@ -159,44 +159,36 @@ func (s *Service) handleRevalidation304( return true, nil } -// revalidationExpectedVersion picks the meta-write precondition for a -// revalidation repopulate. The guarded delete that precedes the repopulate is -// best-effort, so "expect absent" alone is wrong: a transiently failed delete -// leaves the KNOWN-STALE row in place, and put-if-absent would then refuse the -// replacement and keep serving stale data. Instead: -// - entry absent → 0 (put-if-absent); -// - the observed stale row still present → its version (the replacement -// overwrites exactly that row; if it moves first, the newer write wins); -// - anything else present → a racer already re-established a fresh entry → -// 0, which is guaranteed to mismatch, skipping the write in its favor. +// revalidationExpectedVersion picks the decision token for a revalidation +// repopulate. The guarded delete that precedes it is best-effort, so a +// transiently failed delete can leave the known-stale row in place. Instead: +// - an absent entry carries its CAS absence token or legacy generation token; +// - the observed stale row carries its decision token so the replacement can +// update that state; +// - any other present row belongs to a fresh racer, so the repopulate is skipped. // -// A read failure returns 0 as well: refusing to overwrite is the safe -// direction when the store cannot be consulted, and the entry converges via -// the next revalidation or TTL. -func (s *Service) revalidationExpectedVersion(bucket, key, staleETag string) (uint64, bool) { +// A read failure also skips the write; never substitute an unordered token. +func (s *Service) revalidationExpectedVersion(bucket, key, staleETag string) (cache.MetaVersionToken, bool) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() cur, version, found, err := s.cache.GetMetaWithVersion(ctx, bucket, key) if err != nil { - // No token, no ordered commit — expected=0 is the legacy unordered - // put-if-absent and could publish over a fence. Skip the repopulate; - // the client is still served and the entry heals on a later read. - return 0, false + // Without a decision token, skip the repopulate. This includes a + // legacy sidecar that an older cache owner cannot version; never fall + // back to an unordered write. + return cache.MetaVersionToken{}, false } if !found || cur == nil { - // Absent: use the absence TOKEN (ocache v1.13.0), not 0 — the repopulate - // is then ordered against a fenced delete landing after this look. + // Preserve the absent-state token (CAS version or legacy generation + // sidecar) rather than replacing it with the unordered zero sentinel. return version, true } if cur.ETag == staleETag { return version, true } - // A fresh racer holds the key: skip the write in its favor. There is no - // precondition that fails in every future — 0 mismatches the live row, but - // if the racer is fenced-deleted before our (asynchronous) commit, 0 - // becomes legacy put-if-absent over absence and would publish our - // pre-fetch bytes over that fence. - return 0, false + // A fresh racer holds the key: skip the write in its favor rather than + // keeping stale bytes from this revalidation response. + return cache.MetaVersionToken{}, false } // handleRevalidation200 handles a 200 OK revalidation response (object changed). @@ -204,7 +196,7 @@ func (s *Service) revalidationExpectedVersion(bucket, key, staleETag string) (ui func (s *Service) handleRevalidation200( ctx context.Context, w http.ResponseWriter, - bucket, key, staleETag string, + bucket, key, accessKey, secretKey, staleETag string, resp *http.Response, start time.Time, ) error { @@ -226,14 +218,24 @@ func (s *Service) handleRevalidation200( // (with the fresh version), that newer entry is left in place. s.invalidateStaleMeta(bucket, key, staleETag) - // The repopulate's precondition is read AFTER the guarded delete, so our - // own invalidation doesn't block the write, while a concurrent DELETE - // arriving later bumps the fence past it and correctly does. It must be - // read BEFORE the not-cacheable early-return below: a failed token read - // downgrades this response to stream-only — never a commit with the - // legacy unordered expected=0. + // The repopulate's decision token is read AFTER the guarded delete, so our + // own invalidation doesn't block the write. A later CAS delete advances + // the metadata fence; a later legacy delete advances the reader-generation + // sidecar. Read before the not-cacheable return: a failed token read makes + // this response stream-only, never an unordered metadata commit. expected, tokenOK := s.revalidationExpectedVersion(bucket, key, staleETag) shouldCache = shouldCache && tokenOK + if shouldCache { + // The conditional GET may have completed after a concurrent DELETE. Its + // 200 body is a snapshot from before that mutation, while expected is a + // fresh post-invalidation token. Confirm the response still names the + // current strong representation before granting it that token. + if current, ok := s.confirmRevalidation200(ctx, bucket, key, accessKey, secretKey, newMeta); ok { + newMeta = current + } else { + shouldCache = false + } + } // Write response headers to client copyHeaders(w.Header(), resp.Header) @@ -311,6 +313,36 @@ func (s *Service) handleRevalidation200( return copyErr } +// confirmRevalidation200 checks whether the representation from a 200 +// revalidation is still current. A strong ETag plus its exact length ties the +// already-received body to the current object; weak or unknown validators are +// streamed to the client but not cached. The HEAD metadata is used for the new +// entry so changed headers are not copied from the older response. +func (s *Service) confirmRevalidation200(ctx context.Context, bucket, key, accessKey, secretKey string, responseMeta *cache.CachedObjectMeta) (*cache.CachedObjectMeta, bool) { + if responseMeta == nil || responseMeta.ETag == "" || strings.HasPrefix(strings.TrimSpace(responseMeta.ETag), "W/") || responseMeta.ContentLength < 0 { + return nil, false + } + headCtx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + resp, err := s.forwarder.DoConditionalHeadRequest(headCtx, bucket, key, accessKey, secretKey, "", 0) + if err != nil { + log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Revalidation 200 HEAD confirmation failed; response not cached") + return nil, false + } + if resp.Body != nil { + defer resp.Body.Close() + } + if resp.StatusCode != http.StatusOK { + return nil, false + } + current := cache.MetaFromHTTPHeaders(bucket, key, resp.StatusCode, resp.Header) + if current.ETag != responseMeta.ETag || current.ContentLength != responseMeta.ContentLength || + !current.IsCacheable(s.config.Cache.SizeThreshold) || s.hasNoCacheHeaders(resp.Header) { + return nil, false + } + return current, true +} + // serveFromCache serves an object from the cache body. // Used as fallback during revalidation (304 or error paths). func (s *Service) serveFromCache( diff --git a/proxy/revalidation_cas_test.go b/proxy/revalidation_cas_test.go index f5449bfa..c1e4ea56 100644 --- a/proxy/revalidation_cas_test.go +++ b/proxy/revalidation_cas_test.go @@ -186,12 +186,12 @@ func TestBackgroundFetchRepairNotCoalescedBehindWarm(t *testing.T) { bucket, key := "b", "k" // 1. Absent-gated warm starts and blocks in its upstream fetch. - svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityReadMiss, 0) + svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityReadMiss, cache.MetaVersionToken{}) waitFor(t, func() bool { return calls.Load() == 1 }) // 2. A write repair triggers while the warm is in flight. It must run, // not coalesce: with the old bucket/key-only dedup it was dropped here. - svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityWarmWrite, cache.VersionAny) + svc.triggerBackgroundCacheFetch(bucket, key, "access", "secret", false, priorityWarmWrite, cache.AnyMetaVersionToken()) waitFor(t, func() bool { meta, found, _ := c.GetMeta(ctx, bucket, key) return found && meta != nil && meta.ETag == `"new"` @@ -201,7 +201,7 @@ func TestBackgroundFetchRepairNotCoalescedBehindWarm(t *testing.T) { // and must lose to it. close(gate) waitFor(t, func() bool { - if _, busy := svc.activeBackgroundFetches.Load(backgroundFetchKey(bucket, key, 0)); busy { + if _, busy := svc.activeBackgroundFetches.Load(backgroundFetchKey(bucket, key, cache.MetaVersionToken{})); busy { return false } return true @@ -295,9 +295,8 @@ func (f *failingGetVersionClient) GetWithVersion(ctx context.Context, key string return f.CacheClient.GetWithVersion(ctx, key) } -// A revalidation-200 whose precondition token cannot be read must stream to -// the client WITHOUT caching: a commit would carry the legacy unordered -// expected=0 and could publish over a fence. +// A revalidation-200 whose decision token cannot be read must stream to the +// client WITHOUT caching: no mode may fall back to an unordered metadata write. func TestRevalidation200_TokenFailureSkipsCaching(t *testing.T) { newBody := "fresh content" mock := &mockForwarder{ @@ -349,6 +348,46 @@ func TestRevalidation200_TokenFailureSkipsCaching(t *testing.T) { } } +// If the confirmation HEAD fails, the successful revalidation response still +// reaches the client, but its body is not installed under an unverified token. +func TestRevalidation200_HeadConfirmationFailureSkipsCaching(t *testing.T) { + freshBody := "fresh content" + mock := &mockForwarder{ + conditionalResp: &http.Response{ + StatusCode: http.StatusOK, + Body: io.NopCloser(strings.NewReader(freshBody)), + Header: http.Header{ + "Content-Type": []string{"text/plain"}, + "Content-Length": []string{"13"}, + "Etag": []string{`"new"`}, + }, + }, + conditionalHeadErr: errors.New("origin HEAD unavailable"), + } + svc, c := newTestService(mock, true) + ctx := context.Background() + const bucket, key = "b", "k" + stale := &cache.CachedObjectMeta{ + Bucket: bucket, Key: key, ETag: `"old"`, + ContentType: "text/plain", ContentLength: 5, StatusCode: http.StatusOK, + } + if err := c.PutWithMeta(ctx, bucket, key, stale, []byte("stale"), 0); err != nil { + t.Fatalf("seed stale entry: %v", err) + } + + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/b/k", nil) + if err := svc.revalidateAndServe(ctx, w, r, bucket, key, "access", "secret", stale, time.Now()); err != nil { + t.Fatalf("revalidateAndServe: %v", err) + } + if w.Code != http.StatusOK || w.Body.String() != freshBody { + t.Fatalf("client response=(%d,%q), want the fresh upstream body", w.Code, w.Body.String()) + } + if meta, found, err := c.GetMeta(ctx, bucket, key); err != nil || found { + t.Fatalf("unconfirmed revalidation cached metadata=(%+v, found=%t, err=%v)", meta, found, err) + } +} + // A fresh racer holding the key makes the revalidation SKIP its cache write — // not attempt a "guaranteed mismatch" with expected=0, which stops being // guaranteed the moment the racer is fenced-deleted before the async commit diff --git a/proxy/revalidation_test.go b/proxy/revalidation_test.go index be50dfeb..e6988987 100644 --- a/proxy/revalidation_test.go +++ b/proxy/revalidation_test.go @@ -17,11 +17,14 @@ import ( // mockForwarder implements RequestForwarder for revalidation unit tests. type mockForwarder struct { - conditionalResp *http.Response - conditionalErr error + conditionalResp *http.Response + conditionalErr error + conditionalHeadResp *http.Response + conditionalHeadErr error // Track calls for verification conditionalCalled bool conditionalETag string + headCalled bool // Optional Forward implementation for fallback tests forwardFunc func(ctx context.Context, w http.ResponseWriter, r *http.Request) error // Optional ForwardWithCapture implementation for capture-gated tests @@ -97,12 +100,24 @@ func (m *mockForwarder) DoConditionalGetRequest(ctx context.Context, bucket, key } func (m *mockForwarder) DoConditionalHeadRequest(ctx context.Context, bucket, key, accessKey, secretKey, etag string, lastModified int64) (*http.Response, error) { - m.conditionalCalled = true - m.conditionalETag = etag + m.headCalled = true + if m.conditionalHeadErr != nil { + return nil, m.conditionalHeadErr + } if m.conditionalErr != nil { return nil, m.conditionalErr } - return m.conditionalResp, nil + if m.conditionalHeadResp != nil { + return m.conditionalHeadResp, nil + } + if m.conditionalResp == nil { + return nil, errors.New("mock: conditional HEAD not implemented") + } + return &http.Response{ + StatusCode: m.conditionalResp.StatusCode, + Header: m.conditionalResp.Header.Clone(), + Body: http.NoBody, + }, nil } // newTestService creates a Service with an in-memory cache for unit tests. @@ -191,13 +206,22 @@ func TestRevalidation200_StreamsNewBody(t *testing.T) { Body: io.NopCloser(strings.NewReader(newBody)), Header: http.Header{ "Content-Type": []string{"text/plain"}, - "Content-Length": []string{"24"}, + "Content-Length": []string{"25"}, + "Etag": []string{`"newetag"`}, + }, + }, + conditionalHeadResp: &http.Response{ + StatusCode: http.StatusOK, + Header: http.Header{ + "Content-Type": []string{"application/json"}, + "Content-Length": []string{"25"}, "Etag": []string{`"newetag"`}, }, + Body: http.NoBody, }, } - svc, _ := newTestService(mock, true) + svc, c := newTestService(mock, true) ctx := context.Background() bucket, key := "test-bucket", "test-key" @@ -227,6 +251,12 @@ func TestRevalidation200_StreamsNewBody(t *testing.T) { if w.Body.String() != newBody { t.Errorf("body = %q, want %q", w.Body.String(), newBody) } + if !mock.headCalled { + t.Fatal("expected a current-representation HEAD before caching the revalidation response") + } + if cached, found, err := c.GetMeta(ctx, bucket, key); err != nil || !found || cached.ETag != `"newetag"` || cached.ContentType != "application/json" { + t.Fatalf("confirmed 200 revalidation metadata=(%+v, found=%t, err=%v), want current HEAD metadata cached", cached, found, err) + } } func TestRevalidation200_UncacheableDeletesStale(t *testing.T) { diff --git a/proxy/service.go b/proxy/service.go index eb9e9958..97247635 100644 --- a/proxy/service.go +++ b/proxy/service.go @@ -662,8 +662,8 @@ func (s *Service) releaseCacheSlot(weight int64) { // written while forwarding an upstream response. Forward() returns nil even when // upstream responds 4xx/5xx (the response streamed successfully), so mutating // handlers use this to gate post-forward cache re-invalidation on an actual 2xx — -// otherwise a rejected PUT/DELETE/COPY would still fence the destination and -// discard a valid racing refill, causing later reads to miss unnecessarily. +// otherwise a rejected PUT/DELETE/COPY would still advance the destination's +// fence/generation and discard a valid racing refill, causing later reads to miss. type statusRecorder struct { http.ResponseWriter status int @@ -723,9 +723,8 @@ func (s *Service) HandlePutObject(w http.ResponseWriter, r *http.Request) error // Re-invalidate AFTER upstream confirms the write. A GET that raced the // in-flight PUT may have fetched the pre-PUT object and begun re-caching it; - // this second invalidation bumps the fence past that write's decision-time - // token, so its version-preconditioned commit loses — - // restoring read-after-write semantics. + // this second invalidation advances the CAS fence or legacy sidecar generation, + // so current TAG readers reject that old refill. // Gated on a 2xx: a rejected PUT leaves the object unchanged, so re-invalidating // would only discard a valid racing refill and cause an unnecessary later miss. // Routed through invalidateObject (like the pre-forward call) so a failure of this @@ -802,11 +801,13 @@ func (s *Service) HandleDeleteObject(w http.ResponseWriter, r *http.Request) err // this DELETE displaced, and anything newer keeps the key. var ( delPrior *cache.CachedObjectMeta - delPriorVer uint64 + delPriorVer cache.MetaVersionToken delPriorKnown bool ) if s.config.IsTiered() { - delPrior, delPriorVer, delPriorKnown = s.captureMarkerPrior(r.Context(), bucket, key) + var token cache.MetaVersionToken + delPrior, token, delPriorKnown = s.captureMarkerPrior(r.Context(), bucket, key) + delPriorVer = token _ = delPrior } @@ -817,7 +818,7 @@ func (s *Service) HandleDeleteObject(w http.ResponseWriter, r *http.Request) err // Re-invalidate AFTER upstream confirms the delete, for the same // read-after-write reason as HandlePutObject: a GET racing the in-flight // DELETE may have re-cached the not-yet-deleted object; this second - // fence bump blocks that stale repopulation. + // invalidation moves the legacy reader generation or the CAS metadata fence. // Gated on a 2xx: a rejected DELETE leaves the object present, so re-invalidating // would only discard a valid racing refill and cause an unnecessary later miss. if err == nil && rec.wroteSuccess() && s.cache.IsEnabled() { @@ -939,7 +940,7 @@ func (s *Service) HandleCopyObject(w http.ResponseWriter, r *http.Request) error // (cancels/excludes re-tiers) and capture the pre-forward token; the // converge removes only the state this copy displaced. var ( - cpPriorVer uint64 + cpPriorVer cache.MetaVersionToken cpPriorKnown bool ) if s.config.IsTiered() { @@ -956,8 +957,8 @@ func (s *Service) HandleCopyObject(w http.ResponseWriter, r *http.Request) error // Re-invalidate the destination AFTER upstream confirms the copy, for the same // read-after-write reason as HandlePutObject: a GET racing the in-flight copy - // may have re-cached the pre-copy destination object; this second fence bump - // blocks that stale repopulation. + // may have re-cached the pre-copy destination object; the second invalidation + // advances its CAS fence or legacy reader generation. // Gated on a confirmed-successful copy: a rejected copy leaves the destination // unchanged, so re-invalidating would only discard a valid racing refill. if err == nil && s3WriteSucceeded(capture) && s.cache.IsEnabled() { @@ -996,11 +997,11 @@ func s3WriteSucceeded(capture *ResponseCapture) bool { return !isS3ErrorBody(capture.Body) } -// invalidateObject removes an object's cached metadata (a fenced CAS delete) and -// records the true outcome of the attempt. A failed backend invalidation is recorded -// as an error rather than success: a false-green delete metric would hide the very -// read-after-write hazard the invalidation exists to prevent, since the stale entry -// is still in place. It is a no-op when the cache is disabled. +// invalidateObject removes an object's cached metadata through the selected +// coordinator and records the true outcome of the attempt. A failed backend +// invalidation is recorded as an error rather than success: a false-green metric +// would hide the read-after-write hazard the invalidation exists to prevent, since +// the stale entry is still in place. It is a no-op when the cache is disabled. // The error return matters only to origin-less callers, where the cache is the // only store and an acked-but-failed delete keeps serving until TTL. Proxy-mode // callers ignore it: there the origin is authoritative and the upstream DELETE @@ -1040,7 +1041,7 @@ func (s *Service) convergeInvalidation(ctx context.Context, bucket, key string) // (its read failed) nothing is removed: the possibly-stale marker serves // until TTL — an availability inconsistency, chosen over unguarded deletion // in the mode where the cache is the store. -func (s *Service) convergeTieredDelete(bucket, key string, priorVersion uint64, priorKnown bool) { +func (s *Service) convergeTieredDelete(bucket, key string, priorVersion cache.MetaVersionToken, priorKnown bool) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() if !priorKnown { @@ -1115,11 +1116,9 @@ func (s *Service) invalidateObject(ctx context.Context, bucket, key string) erro // this key is already in flight — a concurrent read-path warm, or the warm from a // rapid prior write to the same key — this warm coalesces into that one and is // dropped. When it coalesces into a fetch that predates this write, that fetch's own -// populate is fence-blocked (its decision-time token predates this write's -// invalidation), so it writes nothing either: the key is simply left absent, not -// left stale. The next read then misses and inline-populates the current object. -// This can never serve a stale object — the same fence that blocks the racing -// populate is the read-after-write guard. +// populate is rejected by CAS or tagged with a legacy generation that current +// readers no longer accept, so the next read misses and can populate the current +// object. Older TAG readers do not consult the legacy sidecar. func (s *Service) warmOnWrite(r *http.Request, bucket, key string) { if !s.config.Cache.WarmOnWrite || !s.cache.IsEnabled() { return @@ -1137,9 +1136,9 @@ func (s *Service) warmOnWrite(r *http.Request, bucket, key string) { // probe). See the doc comment: never infer public-read from a public write. if hasNoAuthCredentials(r) { // Token at trigger time: the absence token if this write's invalidation - // succeeded, the surviving stale row's live version if it failed — the - // warm then repairs exactly that state and loses to anything newer, - // including a fenced delete landing mid-fetch. + // succeeded, the surviving stale row's token if it failed. CAS rejects a + // newer row; current legacy readers reject a warm whose sidecar generation + // predates a later invalidation. warmTok, ok := s.warmToken(bucket, key) if !ok { return @@ -1162,13 +1161,13 @@ func (s *Service) warmOnWrite(r *http.Request, bucket, key string) { } // warmToken reads a warm trigger's decision-time token. ok=false means the -// token could not be read and the warm must be SKIPPED: expected=0 is the -// legacy unordered put-if-absent and would publish over a fence. -func (s *Service) warmToken(bucket, key string) (uint64, bool) { +// token could not be read and the warm must be skipped; it must not fall back +// to an unordered metadata write. +func (s *Service) warmToken(bucket, key string) (cache.MetaVersionToken, bool) { _, tok, _, err := s.cache.GetMetaWithVersion(context.Background(), bucket, key) if err != nil { log.Debug().Err(err).Str("bucket", bucket).Str("key", key).Msg("Warm skipped - decision-time token unavailable") - return 0, false + return cache.MetaVersionToken{}, false } return tok, true } @@ -1223,7 +1222,7 @@ func (s *Service) HandleCompleteMultipartUpload(w http.ResponseWriter, r *http.R // real Content-Length. var ( mpPrior *cache.CachedObjectMeta - mpPriorVer uint64 + mpPriorVer cache.MetaVersionToken mpPriorKnown bool mpAK, mpSK string ) @@ -1244,8 +1243,8 @@ func (s *Service) HandleCompleteMultipartUpload(w http.ResponseWriter, r *http.R // Re-invalidate AFTER upstream confirms the completion, for the same // read-after-write reason as HandlePutObject: a GET racing the in-flight - // completion may have re-cached the pre-overwrite object; this second fence bump - // blocks that stale repopulation. Gated on a confirmed-successful completion + // completion may have re-cached the pre-overwrite object; the second + // invalidation advances its CAS fence or legacy reader generation. Gated on a confirmed-successful completion // (2xx and not a 200-with- body) so a failed completion, which leaves the // object unchanged, doesn't discard a valid racing refill. completed := s3WriteSucceeded(capture) diff --git a/proxy/tiered.go b/proxy/tiered.go index 66d54c16..ddcba743 100644 --- a/proxy/tiered.go +++ b/proxy/tiered.go @@ -234,7 +234,7 @@ func (s *Service) handleTieredPut(w http.ResponseWriter, r *http.Request) error markerOwning := originlessPlainObject(r) var prior *cache.CachedObjectMeta - var priorVersion uint64 + var priorVersion cache.MetaVersionToken priorKnown := false if markerOwning { prior, priorVersion, priorKnown = s.captureMarkerPrior(ctx, bucket, key) @@ -279,9 +279,9 @@ func (s *Service) handleTieredPut(w http.ResponseWriter, r *http.Request) error // failed lookup leaves the prior unknown — the marker is then not written and // the sweep refuses to delete anything rather than guess (see // commitUpstreamMarker). Tolerated, never blocking. -func (s *Service) captureMarkerPrior(ctx context.Context, bucket, key string) (prior *cache.CachedObjectMeta, priorVersion uint64, priorKnown bool) { +func (s *Service) captureMarkerPrior(ctx context.Context, bucket, key string) (prior *cache.CachedObjectMeta, priorVersion cache.MetaVersionToken, priorKnown bool) { if !s.cache.IsEnabled() { - return nil, 0, false + return nil, cache.MetaVersionToken{}, false } if m, version, found, cacheErr := s.cache.GetMetaWithVersion(ctx, bucket, key); cacheErr == nil { priorKnown = true @@ -293,7 +293,7 @@ func (s *Service) captureMarkerPrior(ctx context.Context, bucket, key string) (p return prior, priorVersion, priorKnown } -func (s *Service) putUpstreamMarker(r *http.Request, etag, bucket, key string, prior *cache.CachedObjectMeta, priorVersion uint64, priorKnown bool) { +func (s *Service) putUpstreamMarker(r *http.Request, etag, bucket, key string, prior *cache.CachedObjectMeta, priorVersion cache.MetaVersionToken, priorKnown bool) { if etag == "" { s.invalidateDisplacedTieredMeta(bucket, key, prior, priorVersion, priorKnown) log.Warn().Str("bucket", bucket).Str("key", key).Msg("Upstream PUT response had no ETag - no tier marker; object reads as a miss until re-put") @@ -337,7 +337,7 @@ func (s *Service) putUpstreamMarker(r *http.Request, etag, bucket, key string, p // and an unordered marker could resurrect over a DELETE that ran during the // forward — so no marker is written: the object reads as a miss until re-put, // on a path that already requires the metadata store to be failing. -func (s *Service) commitUpstreamMarker(bucket, key string, meta *cache.CachedObjectMeta, prior *cache.CachedObjectMeta, priorVersion uint64, priorKnown bool) { +func (s *Service) commitUpstreamMarker(bucket, key string, meta *cache.CachedObjectMeta, prior *cache.CachedObjectMeta, priorVersion cache.MetaVersionToken, priorKnown bool) { ttl := int(s.config.Cache.TTL.Seconds()) ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() @@ -372,7 +372,7 @@ func (s *Service) commitUpstreamMarker(bucket, key string, meta *cache.CachedObj // the fallback — GETs forward regardless, HEAD just omits the length, and // the re-tier skips unknown-length markers. Commit and sweep are the // large-PUT path's, under the same pre-forward token. -func (s *Service) stampUpstreamMarkerAfterCompletion(bucket, key, etag, accessKey, secretKey string, prior *cache.CachedObjectMeta, priorVersion uint64, priorKnown bool) { +func (s *Service) stampUpstreamMarkerAfterCompletion(bucket, key, etag, accessKey, secretKey string, prior *cache.CachedObjectMeta, priorVersion cache.MetaVersionToken, priorKnown bool) { if etag == "" { s.invalidateDisplacedTieredMeta(bucket, key, prior, priorVersion, priorKnown) log.Warn().Str("bucket", bucket).Str("key", key).Msg("Multipart completion carried no ETag - no tier marker; object reads as a miss until re-put") @@ -460,7 +460,7 @@ func (s *Service) headObjectMeta(ctx context.Context, bucket, key, accessKey, se // that path requires the metadata store to be failing already, and an // unguarded delete there would trade a bounded staleness window for the // unbounded loss of a racing local write that has no upstream copy. -func (s *Service) invalidateDisplacedTieredMeta(bucket, key string, prior *cache.CachedObjectMeta, priorVersion uint64, priorKnown bool) { +func (s *Service) invalidateDisplacedTieredMeta(bucket, key string, prior *cache.CachedObjectMeta, priorVersion cache.MetaVersionToken, priorKnown bool) { if !priorKnown { log.Warn().Str("bucket", bucket).Str("key", key).Msg("Tier marker failed with unknown prior; possibly-stale metadata serves until TTL") return diff --git a/proxy/tiered_test.go b/proxy/tiered_test.go index c3d5e8ae..0e92cbc7 100644 --- a/proxy/tiered_test.go +++ b/proxy/tiered_test.go @@ -1601,7 +1601,7 @@ func TestTieredConvergeRetriesOnceUnderSameVersion(t *testing.T) { c := cache.NewCacheWithClient(flake, &cfg.Cache) return NewService(mock, c, cfg), c, flake } - seedMarker := func(c *cache.Cache) uint64 { + seedMarker := func(c *cache.Cache) cache.MetaVersionToken { marker := &cache.CachedObjectMeta{Bucket: "b", Key: "obj", ETag: `"up-1"`, BodyUpstream: true, ContentLength: 5000, StatusCode: 200} _, tok, _, _ := c.GetMetaWithVersion(context.Background(), "b", "obj") if wrote, err := c.PutMetaIfVersion(context.Background(), "b", "obj", marker, 60, tok); err != nil || !wrote { @@ -1618,8 +1618,8 @@ func TestTieredConvergeRetriesOnceUnderSameVersion(t *testing.T) { if len(flake.versions) != 2 { t.Fatalf("attempts = %d, want 2 (one retry)", len(flake.versions)) } - if flake.versions[0] != v || flake.versions[1] != v { - t.Fatalf("retry changed the version guard: %v, want both == %d", flake.versions, v) + if flake.versions[0] != flake.versions[1] { + t.Fatalf("retry changed the version guard: %v", flake.versions) } if _, found, _ := c.GetMeta(context.Background(), "b", "obj"); found { t.Fatal("converge did not land on the retry") diff --git a/proxy/write_through.go b/proxy/write_through.go index 740e22ec..76b028db 100644 --- a/proxy/write_through.go +++ b/proxy/write_through.go @@ -219,12 +219,12 @@ const ( // teeFallbackWarm when it couldn't confirm/write the version (so a read-back warm may help). func (s *Service) cacheTeedBodyFromHead(bucket, key, putETag string, body []byte, accessKey, secretKey string) teeOutcome { // Decision-time token BEFORE the HEAD (mirroring the populate paths), so - // the whole HEAD-to-write window is ordered: a competing overwrite whose - // invalidation lands after this read bumps the fence past this token and - // the write loses, while one that landed earlier is caught by the - // ETag-consistency check below. Read after this PUT's own post-forward - // invalidation (this goroutine runs after it), so our own fence doesn't - // block us. + // the whole HEAD-to-write window is ordered: CAS rejects a competing + // metadata version; legacy records this sidecar generation and current + // readers ignore a late plain write after invalidation. An overwrite that + // landed earlier is caught by the ETag-consistency check below. Read after + // this PUT's own post-forward invalidation (this goroutine runs after it), so + // our own generation does not block us. _, expected, _, tokErr := s.cache.GetMetaWithVersion(context.Background(), bucket, key) if tokErr != nil { // No token, no ordered commit: fall back to the warm, which reads its diff --git a/tests/integration/cache_test.go b/tests/integration/cache_test.go index a1c3bd37..fa70b01d 100644 --- a/tests/integration/cache_test.go +++ b/tests/integration/cache_test.go @@ -1141,7 +1141,7 @@ func TestBlockCache_ServeInteriorBlockRangeThroughHandler(t *testing.T) { ContentLength: total, StatusCode: 200, BlockSize: blockSize, LastModified: time.Now().Unix(), } - wrote, err := env.Cache.PutMetaIfVersion(ctx, bucket, key, meta, 300, cache.VersionAny) + wrote, err := env.Cache.PutMetaIfVersion(ctx, bucket, key, meta, 300, cache.AnyMetaVersionToken()) require.NoError(t, err) require.True(t, wrote, "block-mode meta write skipped")