Skip to content

PersistedDocumentsManager: replace get+insert miss handling with single-flight loading (moka try_get_with_by_ref) #867

@kamilkisiela

Description

@kamilkisiela

PersistedDocumentsManager::resolve_document currently uses a get + fetch + insert pattern.
This pattern is highly inefficient under concurrency.

With N concurrent requests for the same uncached document_id:

  • all N requests observe a miss,
  • all N perform outgoing CDN HTTP fetches,
  • all N attempt to insert the same value into cache.

It also writes to the cache in a function that is responsible for fetching data with http, it shouldn't.

What should change
Refactor cache-miss loading to Moka single-flight semantics:

  • Use self.cache.try_get_with_by_ref(document_id, async { ... }) or other similar APIs
  • Let Moka multiplex concurrent same-key misses so only one loader runs per key

Why this matters
Single-flight turns the miss behavior from:

  • current: N misses -> N fetches + N inserts
  • target: N misses -> 1 fetch + 1 insert, N waiters share result

Current: get + fetch + insert (N requests => N fetches)

sequenceDiagram
    autonumber
    participant A as Req A
    participant B as Req B
    participant C as Req C
    participant D as Req D
    participant Cache
    participant CDN
    A->>Cache: get(X)
    Cache-->>A: MISS
    B->>Cache: get(X)
    Cache-->>B: MISS
    C->>Cache: get(X)
    Cache-->>C: MISS
    D->>Cache: get(X)
    Cache-->>D: MISS
    A->>CDN: fetch(X)
    B->>CDN: fetch(X)
    C->>CDN: fetch(X)
    D->>CDN: fetch(X)
    CDN-->>A: doc
    CDN-->>B: doc
    CDN-->>C: doc
    CDN-->>D: doc
    A->>Cache: insert(X, doc)
    B->>Cache: insert(X, doc)
    C->>Cache: insert(X, doc)
    D->>Cache: insert(X, doc)
Loading

Target: single-flight try_get_with_by_ref (N requests => 1 fetch)

sequenceDiagram
    autonumber
    participant A as Req A
    participant B as Req B
    participant C as Req C
    participant D as Req D
    participant Cache
    participant Loader as In-flight loader
    participant CDN
    A->>Cache: try_get_with_by_ref(X)
    Cache->>Loader: start loader for X
    B->>Cache: try_get_with_by_ref(X)
    Cache-->>B: wait for loader(X)
    C->>Cache: try_get_with_by_ref(X)
    Cache-->>C: wait for loader(X)
    D->>Cache: try_get_with_by_ref(X)
    Cache-->>D: wait for loader(X)
    Loader->>CDN: fetch(X)
    CDN-->>Loader: doc
    Loader->>Cache: insert(X, doc)
    Cache-->>A: doc
    Cache-->>B: doc
    Cache-->>C: doc
    Cache-->>D: doc
Loading

Error handling note
The try_get_with_by_ref returns Result<V, Arc<E>>, so we should define an explicit error strategy instead of lossy fallback behavior:

  • either return Arc<PersistedDocumentsError> from resolve_document, or
  • provide deterministic owned-error mapping that preserves root cause context

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions