Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature request] some kind of "return stale value, but keep updating" signal #268

Closed
isaacs opened this issue Feb 16, 2023 · 3 comments
Closed

Comments

@isaacs
Copy link
Owner

isaacs commented Feb 16, 2023

(inspired by thinking through #263)

It'd be nice to be able to use fetch() to say something like: "Try to get the new value from the source, but if it takes longer than ${time}, just return the stale value, and continue updating in the background".

One way to maybe do that is to use the abort signal as a method for signaling an early return? Then the fetchMethod could do something like:

const fetchMethod = async (key, staleValue, { signal }) => {
  // return the stale value (or undefined) after 1000ms, but still keep updating
  // kind of a weird use/abuse of the API?
  const t = setTimeout(() => signal.dispatchEvent(new Event('returnStale')), 1000)
  // still return the actual value if this comes first
  const result = await potentiallyLongAPIRequest(key, signal)
  clearTimeout(t)
  return result
}

That could be sugared up into a maxFetchTime option, so like:

// guaranteed to return _something_ after 1000ms or less, but will
// keep updating the background cache for as long as it takes
const value = await cache.fetch(key, { maxFetchTime: 1000 })

What do you think? Would you use this if it existed?

@isaacs
Copy link
Owner Author

isaacs commented Feb 17, 2023

Hmm, maxFetchTime makes me think it's going to abort the fetch() if it goes over time. That's probably also useful, I guess?

Maybe to clarify, there could be two options, both of which could be mutable by the fetchMethod:

  • fetchTimeout: some number of ms after which the fetchMethod will get an abort signal and be removed from the cache if it hasn't returned yet
  • allowStaleOnTimeout: boolean to say "if it times out, return the stale value". This would also allow the fetchMethod to keep going past the timeout so it'll eventually fill in the updated value, though of course it'll still get an abort signal if it's eventually evicted or deleted.

So, to signal back up from the fetchMethod to the cache, that you expect this fetch to take some specific amount of time, you could do:

const cache = new LRUCache<string, APIResponse>({
  fetchMethod: async (key, oldVal, { options, signal }) => {
    // calculate the expected time to wait somehow
    options.fetchTimeout = calculatedExpectedWaitTime()
    return slowBackendAPIRequest(key, signal)
  }
}

This also doesn't require that the consumer on either end of the cache has to deal with custom events or other weird abuses of the AbortController/AbortSignal interface.

@isaacs
Copy link
Owner Author

isaacs commented Feb 18, 2023

Oh, there is a native interface for AbortSignal.timeout(), which returns a signal that automatically aborts after a certain amount of time, but there's no way to use both an abort signal and a timeout signal, so I'd still have to do something like AbortSignal.timeout().addEventListener('abort', e => ac.abort()). And probably that'll mean ditching the AbortController polyfill and dropping node < 16 (and at that point, may as well also rewrite in TS and have proper types for all the combinations of when size is required, when fetch is available, etc.)

@isaacs
Copy link
Owner Author

isaacs commented Feb 21, 2023

With the ability to pass in an AbortSignal, you'd be able to do this, if there was a "ignoreFetchAbort" option:

const c = new LRUCache({
  fetchMethod: async (key, oldValue, { signal }) => {
    return await longSlowThing(key)
  },
  allowStaleOnFetchRejection: true,
  ignoreFetchAbort: true,
})

// will definitely return *something* in 100ms,
// either the cached value or `undefined`,
// but if a value eventually resolves, it'll be cached
const value = c.fetch('key', { signal: AbortSignal.timeout(100) })

@isaacs isaacs closed this as completed in cca36d5 Feb 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant