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

Cache seems to return unrelated values after a while #192

Closed
bcomnes opened this issue Feb 9, 2022 · 8 comments
Closed

Cache seems to return unrelated values after a while #192

bcomnes opened this issue Feb 9, 2022 · 8 comments

Comments

@bcomnes
Copy link

bcomnes commented Feb 9, 2022

I apologize in advance for not having this fully pinned down, but after updating to version 7.0.0-7.3.0, it seems as though the cache starts returning values for unrelated cache keys, after a while under moderate load (~5 minutes).

This is my cache:

const cache = new LRU({
  max: 10000,
  maxAge: 1000 * 60 * 20, // 20 mins,
  updateAgeOnGet: false
})

And these keys:

function getCacheKey ({
  access_token,
  refresh_token,
  purchase_id,
  file_id
}) {
  return [access_token, refresh_token, purchase_id, file_id].join(';')
}

Seem to start returning value from these keys:

function getOtherCacheKey ({
  access_token,
  refresh_token,
  purchase_id,
  proxyFiles,
  incomingHost
}) {
  return ['rss', access_token, refresh_token, purchase_id, proxyFiles, incomingHost].join(';')
}

(where the arguments are url escaped strings)

I haven't been able to reproduce it locally yet (though it does reliable begin under moderate production load), but given the delay it seems to have a 'something filling up'ness to it.

The other idea is that maybe the keys are a little long. Are there new restrictions on string keys in version 7?

Usage is limited to simple get/set, and I updated the config to match the what the migration guide recommended.

@bcomnes bcomnes changed the title Cache seems to return unrelated keys after a while Cache seems to return unrelated values after a while Feb 9, 2022
@HitkoDev
Copy link

HitkoDev commented Feb 9, 2022

Can confirm, in the new version cache starts returning random values after a couple of gets / sets.

For now I upgraded to v6, hope this gets resolved soon.

@isaacs
Copy link
Owner

isaacs commented Feb 9, 2022

Can you share any more details about how you're using it? I have a lot of tests where I fill up the cache to the point of eviction, that sounds really strange.

@isaacs
Copy link
Owner

isaacs commented Feb 9, 2022

There is not a limit on string length. Anything that worked before should work now, they're just keys in a map.

@HitkoDev
Copy link

HitkoDev commented Feb 9, 2022

https://stackblitz.com/edit/node-wccxgj?file=index.js There you go, a quick repro

@isaacs
Copy link
Owner

isaacs commented Feb 9, 2022

I got a fix. Will ship and backport to 7.x versions within the hour. Sorry for the hassle.

@isaacs isaacs closed this as completed in 7e7856e Feb 9, 2022
isaacs added a commit that referenced this issue Feb 9, 2022
isaacs added a commit that referenced this issue Feb 9, 2022
isaacs added a commit that referenced this issue Feb 9, 2022
@isaacs
Copy link
Owner

isaacs commented Feb 9, 2022

Fixed with a patch on all 7.x minors, and deprecated all affected versions.

@bcomnes
Copy link
Author

bcomnes commented Feb 9, 2022

Thanks @HitkoDev for isolating the issue, and thanks @isaacs for the fix (and faster algorithm)!

@bcomnes
Copy link
Author

bcomnes commented Feb 10, 2022

Been running the working version of 7 in production and it does speed up cache hits. Not that 6 wasn't doing great, but its fun to see the difference. Basically, uncached results are slow... but when it is cached, it's spending less time returning cached results.
Screen Shot 2022-02-10 at 9 40 26 AM

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

3 participants