feat: Redis connection pooling #433
Conversation
|
@paveltiunov I could use some extra eyes on this in terms of testing, it is working locally but I haven't exercised it as extensively as I wanted - one thing I want to do is to cause redis connections to ramp up and then watch that they are detected as idle and ramp back down. I haven't gone out of my way to write extra tests for this - but I did convert to jest while I was troubleshooting something, hopefully that is ok, it seems to be in use in other areas of this project. I also haven't updated any docs. |
|
@jcw- Hey Jc! Looks really great! And thanks for migrating tests to jest! Really appreciate that! |
| class RedisCacheDriver { | ||
| constructor() { | ||
| this.redisClient = createRedisClient(process.env.REDIS_URL); | ||
| constructor(pool) { |
There was a problem hiding this comment.
Would it be possible for the constructor options to be an object, with a property pool instead? This would be helpful in the future, in case we need to add any more parameters to instantiate a Redis driver
There was a problem hiding this comment.
@hassankhan Yeah. Great idea! I think we should provide options object to RedisPool instead of arguments.
…b-analytics * 'web-analytics' of github.com:statsbotco/cube.js: (94 commits) Update dependencies Update dependencies Update web-analytics v0.18.0 fix: Redis query queue locking redesign Update Subquery.md tests: Cleanup redis pool properly in tests (#458) Debug CI Update client core dependency for vue tests Fix vue tests and client build Fix tests race conditions fix: Handle multiple occurrences in the first event of a funnel: conversion percent discrepancies. feat: Redis connection pooling (#433) Thanks to @jcw! build(deps): bump moment-range from 4.0.1 to 4.0.2 (#442) feat: Add role parameter to driver options (#448) Thanks to @smbkr! build(deps-dev): bump request from 2.88.0 to 2.88.2 (#441) build(deps): bump recharts from 1.7.1 to 1.8.5 (#446) build(deps): bump sqlite3 from 4.1.0 to 4.1.1 (#445) build(deps): bump tslib from 1.9.3 to 1.11.0 (#440) build(deps-dev): bump http-server from 0.11.1 to 0.12.1 (#437) ...
Check List
Issue Reference this PR resolves
Fixes #104
Description of Changes Made (if issue reference is not provided)
Adds a connection pool class based on
generic-pool. By default, uses (min=2, max=10). If max is set to zero (e.g. through the env var), then the pool is disabled and it falls back to prior behavior (non-pooled).A single instance of the connection pool is created in the
QueryOrchestratorand is wired up to two final destinations:RedisQueueDriverConnectionRedisCacheDriver