goburrow / cache

Mango Cache 🥭 - Partial implementation of Guava Cache in Go (golang).
BSD 3-Clause "New" or "Revised" License
580 stars 48 forks source link

Add context.Context propagation to LoadingCache #8

Closed MatthewDolan closed 3 years ago

MatthewDolan commented 6 years ago

The current definition of LoadingCache is

type LoadingCache interface {
    Cache

    // Get returns value associated with Key or call underlying LoaderFunc
    // to load value if it is not present.
    Get(Key) (Value, error)
}

I have been running into problems debugging errors on the Get method because I can't associate a given Get call with a specific call of LoaderFunc. The context chain is broken.

I would propose adding a new parameter to Get and LoaderFunc the would propagate the context and make debugging easier.

type LoadingCache interface {
    Cache

    // Get returns value associated with Key or call underlying LoaderFunc
    // to load value if it is not present.
    Get(Key) (Value, error)

    // GetContext returns value associated with Key or call underlying LoaderFunc
    // to load value if it is not present.
    GetContext(context.Context, Key) (Value, error)
}

type LoaderFuncContext func(context.Context, Key) (Value, error)

What do you think?

esiqveland commented 4 years ago

I think a context version is needed. It is hard to debug errors in the underlying loader without the context from the operation that caused the load.

nqv commented 4 years ago

Reloading value can be done asynchronously, so I'm not sure how context is used/created in that case.

codyaray commented 3 years ago

Any update on this? I'd love to pass the context through (where possible) since my underlying datastore is context-aware (e.g., handles context cancellation etc well)