Only one pending refresh on staleRefreshTimeout

[assensamuelsson]

It seems that cachified always queues up refresh requests based on staleRefreshTimeout even if we have pending requests. This seems a bit unnecessary. Is it possible to detect this and don't queue up more refresh requests if there is 1 pending?

Example: Lets say I'm using cachified like:

return cachified({
  key,
  ttl: 1000,
  staleWhileRevalidate: 60_000,
  staleRefreshTimeout: 10_000,
  getFreshValue: () => {
    console.log("Getting fresh value");
    return 123;
  }
});

If we get a request during our stale time cachified correctly queues up a refresh in 10 seconds. However if we get another request for the same key in lets say 5 seconds - cachified queues up 1 more and we totally executes 2 getFreshValues.

[kentcdodds]

Thanks for opening this up! This is tricky because it depends on your intent. I understand the scenario you've described, but let's expand on the example scenario you provided. What if the first fresh request takes a total of 11 seconds? That would mean the entry is immediately past the refresh timeout time. I think the intent of staleRefreshTimeout is to ensure the value in the cache is as close to up-to-date as indicated by the staleRefreshTimeout time after each call and if we skip a request just because we scheduled an update in the past then we could be in trouble.

That said, let's consider an alternative scenario where the cache for a key is stale, and suddenly there are tons of requests for that key. We could end up making tons of fresh requests for that value before it's updated. This is definitely not desirable either.

In light of that, what if we have a compromise where if there's an ongoing refresh happening throttle refreshes so there's only one made at a time.

[assensamuelsson]

Ok I understand. I can give some more context to our actual use case.

We're developing API A that depends on API B. Traffic spikes to API A is common and we know that API B does not scale that well and might temporarily become unavailable for a few minutes . Were doing what we can to protect API B - using cachified with as long ttls and swrs that we think are reasonable. Most of our cache keys to API B while be stale and updated in the background during our traffic spikes.

I had an idea we could use staleRefreshTimeout to flatten the traffic spike curve to API B. I.e. serve stale data and randomly queue up the background refresh between 0 and 60 seconds. However if a request comes in for the same key again during that time - cachified will queue up another request somewhat defeating the idea I had.

In our use case I don't think throttling will work that well because the request time for API B is quite fast and the probability of two requests running concurrently are very small.

Is it wrong of us to consider using staleRefreshTimeout for this? What are its other use cases then?

[kentcdodds]

Thanks for the detailed explanation of your use case! This is a really interesting problem, and I think I have a suggestion that might help.

While we could add this functionality to cachified, I think this might be better handled at the application level. The reason being that cachified's primary responsibility is managing cache entries and their lifecycle, while protecting downstream services is more of an application-specific concern.

Here's an idea to try:

import { cachified } from '@epic-web/cachified'
import pThrottle from 'p-throttle'

// Create a throttled wrapper for your API calls
const throttledApiCall = pThrottle(
  (key: string) => apiB.getData(key),
  1, // only 1 request at a time
  60000 // cooldown period
)

function fetchFromApiB(key: string) {
  return cachified({
    key,
    ttl: 1000,
    staleWhileRevalidate: 60_000,
    getFreshValue: () => throttledApiCall(key),
  })
}

This approach gives you full control over the throttling behavior while still leveraging cachified's caching capabilities. You could even implement a random delay distribution if you prefer:

function fetchFromApiB(key: string) {
  return cachified({
    key,
    ttl: 1000,
    staleWhileRevalidate: 60_000,
    getFreshValue: async () => {
      // Randomly delay between 0-60s to flatten traffic spikes
      await new Promise(resolve => 
        setTimeout(resolve, Math.random() * 60000)
      )
      return apiB.getData(key)
    },
  })
}

This keeps cachified focused on doing one thing well (caching) while giving you the flexibility to implement whatever traffic management strategy works best for your specific use case.

What do you think about this approach? 🤔

[Xiphe]

This is indeed an interesting topic!

The initial intention of this option was only to move background refreshes later into the event loop to not bind recources that are needed for the current request.

The reason I've added the staleRefreshTimeout option is that the initial implementation had this on 200ms and I've moved it to 0 by default. Then made it an option so the original behavior could be configured.

I'd argue we should deprecate it as the option and the name is not intuitive.

~~That said. The problem is very much als present without the option. With a longer running getFreshValue, the "parallel fetch protection" doesn't work as we forceFresh the calls after that timeout without de-duplicating it.~~

~~I'd say this is not intended behavior and should be considered a bug.~~

Edit: The above statement is only true for synchronously executed calls into a stale revaidation state which should™ not present an issue for real-world cases.

I think this would be the best™ way to handle cases as presented by @assensamuelsson:

const value = await cachified({
  key,
  ttl: 1000,
  staleWhileRevalidate: 60_000,
  getFreshValue: async ({ background }) => {
    if (background) sleep(10_000); /* effective staleRefreshTimeout */
    return apiB.getData(key);
  },
});

[assensamuelsson]

Thank you both for your thoughtful and detailed responses 🙏. This library is fantastic, and having active and friendly maintainers like you makes it even better.

Special thanks to @Xiphe for providing an example and a clear solution to my problem. I really appreciate this approach, as it feels more explicit and intuitive compared to the staleRefreshTimeout option, which, as you pointed out, has a somewhat misleading name.

I'm happy to consider this issue resolved on my end, but feel free to keep it open if you'd like to use it to discuss a potential deprecation. :)

[Xiphe]

Thank you for the feedback! Really appreciate it 🤗

I'm gonna address a few of the misleading/unintuitive things here and then close

[github-actions[bot]]

:tada: This issue has been resolved in version 5.5.2 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: