Skip to content

Adjustable Redis Connection Pooling#7283

Merged
henrydrichardson merged 1 commit intomainfrom
redis-connection-pooling
Nov 3, 2022
Merged

Adjustable Redis Connection Pooling#7283
henrydrichardson merged 1 commit intomainfrom
redis-connection-pooling

Conversation

@henrydrichardson
Copy link
Contributor

🎫 Ticket

https://github.com/18F/identity-devops/issues/5429

🛠 Summary of changes

This PR enables adjustable connection pool sizes to redis instances.

**Why**:
- Adding the configuration parameters `redis_pool_size` and
  `redis_throttle_pool_size` allows us to limit, or increase,
  the number of connections to our redis instances.

changelog: Internal, Redis, Manage total connections to redis.
Copy link
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

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

👍🏼 👍🏼

Copy link
Contributor

@jgrevich jgrevich left a comment

Choose a reason for hiding this comment

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

Nice, lgtm

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

end

REDIS_THROTTLE_POOL = ConnectionPool.new(size: 5) do
REDIS_THROTTLE_POOL = ConnectionPool.new(size: IdentityConfig.store.redis_throttle_pool_size) do
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we initialize the attempts API redis outside of this file:

cattr_accessor :redis_pool do
ConnectionPool.new(size: 10) do
Redis::Namespace.new(
'irs-attempt-api',
redis: Redis.new(url: IdentityConfig.store.redis_irs_attempt_api_url),
)
end

Maybe if not for this PR, but could we follow up and add a config for it too, maybe even move it inside this file as a constant instead of a class member there?

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

Successfully merging this pull request may close these issues.

4 participants