Skip to content

[Bug] ObjectStorageDurableStateStore.etagCache is an unbounded Map keyed by persistenceId evicted only on delete or close, so a sharded deployment leaks one entry per entity ever loaded #962

Description

@pathosDev

Problem

ObjectStorageDurableStateStore.etagCache is a plain Map<string, CachedEntry> keyed by persistenceId, with no cap, no TTL and no eviction policy. It gains an entry on every successful load and every successful upsert, and it loses one in exactly four places: delete(persistenceId), close(), the forgetEtagForTest hook, and the ObjectStorageConcurrencyError branch that drops a stale etag before rethrowing. None of those is reached by the normal lifecycle of an entity that is simply finished with.

The store is process-scoped and is shared by every actor the plugin serves, so its cache accumulates one entry per distinct persistenceId the process has ever touched — which is the wrong axis. Under sharding the entity population is unbounded by design and the residency is not: entities passivate, the shard rebalances away, the DurableStateActor stops. The actor goes; the entry stays. Restarting the actor does not reset it either, because the cache lives on the store, not on the actor. In a deployment where the persistenceId is derived from a request-supplied identifier — a cart id, a session id, a tenant-scoped user id, which is the ordinary sharding pattern — the growth axis is "distinct ids the process has seen", and each entry retains the id string plus its etag string for the life of the process.

The close() clear is the tell: the class knows the map needs releasing, and the only moment it releases is when the whole store is torn down.

Evidence

The declaration — no bound, no eviction hook:

src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:61-72
export class ObjectStorageDurableStateStore implements DurableStateStore {
  private readonly backend: ObjectStorageBackend;
  private readonly ownsBackend: boolean;
  private readonly prefix: string;
  private readonly compression: CompressionConfig | CompressionResolver | undefined;
  private readonly encryption: EncryptionConfig | EncryptionResolver | undefined;
  private readonly integrity: IntegrityConfig | IntegrityResolver | undefined;
  private readonly requireIntegrity: boolean;
  private readonly maxDecompressedBytes: number;
  private readonly etagCache = new Map<string, CachedEntry>();

  private readonly serializer?: Serializer;

Every load inserts:

src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:130-133
    // Cache AFTER decode succeeds (integrity check inside decodeBody).
    // Before #116 we cached before parsing; an attacker could tamper
    // with the revision in the body and the cache would trust it.
    this.etagCache.set(persistenceId, { etag: fetched.value.etag, revision: parsed.revision });

Every successful upsert inserts:

src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:241-242
    this.etagCache.set(persistenceId, { etag, revision: newRevision });
    return { persistenceId: persistenceId, revision: newRevision, state, timestamp: now };

And the only lifecycle-driven removals are an explicit record deletion and store shutdown:

src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:245-260
  async delete(persistenceId: string): Promise<void> {
    await this.backend.delete(this.keyFor(persistenceId));
    this.etagCache.delete(persistenceId);
  }

  async close(): Promise<void> {
    this.etagCache.clear();
    // Only close a backend we own.  When it's shared (e.g. registerObjectStoragePlugins
    // hands the same backend to the snapshot + durable-state stores) the owner closes it.
    if (this.ownsBackend) await this.backend.close?.();
  }

  /** Test hook — drop the cached ETag for a persistenceId (simulates actor restart). */
  forgetEtagForTest(persistenceId: string): void {
    this.etagCache.delete(persistenceId);
  }

Note what forgetEtagForTest's own doc comment concedes: dropping the entry is what "simulates actor restart". The real restart path does not do it — nothing in DurableStateActor.postStop, in passivation, or in the shard-region teardown touches the store's cache.

The cache is not load-bearing for correctness, which is what makes the fix cheap: upsert already has a full recovery path for a missing entry — expectedRevision > 0 && cached === undefined re-loads to refresh the etag and only then decides.

src/persistence/durable-state-stores/ObjectStorageDurableStateStore.ts:192-208
    if (expectedRevision > 0 && cached === undefined) {
      // We were asked to expect revision N>0 but have no etag in cache.  Two
      // legitimate paths: caller never `load`ed (operator error) or cache
      // was wiped (e.g. on actor restart).  Do an extra load to refresh;
      // if the bucket's revision matches expected, retry with the fresh
      // etag.  If not, surface the concurrency error so the caller can
      // recover.
      // Pass `options` so the cache-refresh load can decrypt with the
      // caller's encryption preferences.
      const option = await this.load<S>(persistenceId, options);
      if (option.isNone()) {
        throw new DurableStateConcurrencyError(persistenceId, expectedRevision, 0);
      }
      if (option.value.revision !== expectedRevision) {
        throw new DurableStateConcurrencyError(persistenceId, expectedRevision, option.value.revision);
      }
    }

Evicting an entry therefore costs one extra GET on the next write for that id, and nothing else.

Proposal

Bound it. The entries are pure optimisation, so any of these is correct and the choice is about which fits the deployment:

  • An LRU with a configured maxCachedEtags (default in the low thousands), surfaced on ObjectStorageDurableStateStoreOptions as withMaxCachedEtags(n) and reachable from HOCON via ConfigKeys like every other tunable.
  • Or a TTL — an etag is only useful while the entity is resident, and a resident entity refreshes it on every write.
  • Independently: give the store an eviction entry point (forgetEtag(persistenceId) — the test hook, promoted and made public) and call it from DurableStateActor.postStop, so passivation releases what it allocated instead of relying on a cap to clean up after it.

Acceptance sketch

  • etagCache has a configurable upper bound with a documented default; exceeding it evicts rather than grows.
  • The bound is settable through ObjectStorageDurableStateStoreOptions and through HOCON, with a validator rule (positive integer).
  • A test loads more distinct persistence ids than the bound and asserts the map size stays at the bound and that a subsequent upsert on an evicted id still succeeds.
  • DurableStateActor.postStop releases the store's cache entry for its own persistence id.

Reference issues: #728 (ConsumerController's per-producerId dedup map), #593 (DevTools federation peer reports), #137/#138/#139 (receptionist, member map, pubsub subscriber lists) are the same defect class elsewhere in the tree — an unbounded Map keyed by an identifier the process does not control the cardinality of. #786 concerns the strength of the etag this cache stores, not its lifetime.

Verification status

Found in the ten-lens production-readiness review of 2026-08-05 (v0.13.0) and re-verified before filing: confirmed by reading. The declaration, both insertion sites and every removal site are quoted above from the current tree; there is no other reference to etagCache in the file. Not reproduced by execution: a memory-growth demonstration would need a long-running sharded deployment to be meaningful, and the absence of an eviction path is not a matter of degree.

Part of the production-readiness review batch — tracked in #913.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumUseful, not urgentproduction-goalBlocks or defines the path to production readiness

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions