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

Remove the use of redis keys command, which should rarely (if ever) be used in production #263

Closed
whatl3y opened this issue Sep 6, 2018 · 2 comments

Comments

@whatl3y
Copy link
Contributor

whatl3y commented Sep 6, 2018

In high throughput Redis environments or in DBs with a lot of keys the use of the keys command can cause significant performance issues (see Warning here: https://redis.io/commands/keys). It's a blocking command that can cause high latency in large redis environments.

I documented all the places I could find that uses keys today below, but the approach that redis experts have given me is to recursively call scan with a pattern provided in the query until the cursor gets back to 0. This approach is certainly slower than using keys alone, especially in DBs with a lot of keys, but the load on the redis server is basically eliminated.

https://github.com/taskrabbit/node-resque/blob/master/lib/scheduler.js#L196
https://github.com/taskrabbit/node-resque/blob/master/lib/queue.js#L134
https://github.com/taskrabbit/node-resque/blob/master/lib/queue.js#L175
https://github.com/taskrabbit/node-resque/blob/master/lib/queue.js#L178
https://github.com/taskrabbit/node-resque/blob/master/lib/queue.js#L328

Example recursive method to using the scan pattern (can obviously be updated as needed):

async scanMatch(match, iterationCallback, cursor=0, numMatches=0) {
  const [ newCursor, matches ] = await this.connection.redis.scan(cursor, 'match', match)
  if (matches && matches.length > 0) {
    numMatches += matches.length
    await Promise.all(matches.map(async match => await iterationCallback(match)))
  }

  if (newCursor == '0')
    return numMatches

  return await this.scanMatch(match, iterationCallback, newCursor, numMatches)
}

Then use this function to get all keys returned in an array, just like keys as follows:

async getKeysLike(pattern) {
  let allKeys = []
  await this.scanMatch(pattern, async match => allKeys.push(match))
  return allKeys
}
@evantahler
Copy link
Member

Thank you for the examples! Are you comfortable submitting a pull request with these changes?

@whatl3y
Copy link
Contributor Author

whatl3y commented Sep 6, 2018

@evantahler yeah I can likely work on this within the next week or so (hopefully sooner). I actually might have to do this sooner than later since we're using node-resque in prod today and this issue is causing redis latency issues for us.

On a side, and lighter, note I love node-resque, it's solved a lot of problems for us with queueing & processing background jobs. Well done!

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

2 participants