Skip to content
This repository has been archived by the owner on Nov 4, 2021. It is now read-only.

Redis connection pool #204

Merged
merged 19 commits into from
Feb 23, 2021
Merged

Redis connection pool #204

merged 19 commits into from
Feb 23, 2021

Conversation

mariusandra
Copy link
Collaborator

Changes

  • With this change each Redis query needs to finish before a new one is allowed (per thread ofc). This might slow down plugins by a bit, but should greatly increase stability of the pipeline.
  • If this works fine, we could add a few more workers to the pool to increase throughput. This will increase the used redis connections, getting us in trouble with Heroku again...

Checklist

  • Updated Settings section in README.md, if settings are affected
  • Jest tests

@mariusandra mariusandra added the bump patch Bump patch version when this PR gets merged label Feb 23, 2021
Twixes
Twixes reviewed Feb 23, 2021
View reviewed changes
src/db.ts Outdated Show resolved Hide resolved
src/db.ts Outdated Show resolved Hide resolved
@mariusandra mariusandra marked this pull request as draft February 23, 2021 15:12
@mariusandra
Copy link
Collaborator Author

Let's actually go for a real pool...

@mariusandra mariusandra changed the title Pool of 1 for Redis Redis connection pool Feb 23, 2021
@mariusandra
Copy link
Collaborator Author

Something's broken with celery now :/. Will investigate/fix later today.

@mariusandra mariusandra marked this pull request as ready for review February 23, 2021 19:34
@mariusandra
Copy link
Collaborator Author

Ready for a look! I had to change the tests around quite a bit, such as removing ioredis-mock, etc...

Twixes
Twixes approved these changes Feb 23, 2021
View reviewed changes
Copy link
Collaborator

@Twixes Twixes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@mariusandra mariusandra merged commit 0a04a14 into master Feb 23, 2021
@mariusandra mariusandra deleted the redis-pool branch February 23, 2021 20:39
@mariusandra mariusandra mentioned this pull request Feb 24, 2021
Closed
@mariusandra mariusandra mentioned this pull request May 26, 2021
Closed
fuziontech pushed a commit to PostHog/posthog that referenced this pull request Oct 12, 2021
@mariusandra @Twixes
[plugin-server] Redis connection pool (PostHog/plugin-server#204)
25126be 
* pool of 1 for redis to help with async

* space

* increase delay now that redis is sequential

* actually use a pool

* fix some tests

* add REDIS_POOL_MIN_SIZE and REDIS_POOL_MAX_SIZE, and fix a few tests

* redlock uses another redis connection outside the pool

* increase delay

* fix type

* more fixes

* more fixes

* queue test without mocked redis

* increase redis pool for worker test

* fix worker test

* shorter delay

* create pubsub via createRedis

* improve flakiness

* improve flakiness... again

Co-authored-by: Michael Matloka <dev@twixes.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Twixes Twixes Twixes approved these changes

Assignees
No one assigned
Labels
bump patch Bump patch version when this PR gets merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mariusandra @Twixes