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

feat(redis): Allow configuring Redis pools individually #3859

Merged
merged 26 commits into from
Aug 1, 2024

Conversation

loewenheim
Copy link
Contributor

Second attempt at #3843. Implements #3829.

This time around, if there is only one Redis pool configuration, we only create one pool and clone it 4 ways (instead of creating 4 identical pools like last time). This means that the case of only one configuration preserves the current behavior, modulo the bug fix related to DEFAULT_MIN_MAX_CONNECTIONS.

@loewenheim loewenheim requested a review from a team as a code owner July 24, 2024 15:34
@loewenheim loewenheim self-assigned this Jul 24, 2024
Comment on lines +385 to +390
let pool = create_redis_pool(pool)?;
Ok(RedisPools {
project_configs: pool.clone(),
cardinality: pool.clone(),
quotas: pool.clone(),
misc: pool,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part that's different from the first PR: we only create one pool, then clone it.

@loewenheim loewenheim changed the title Loewenheim/redis pools feat(redis): Allow configuring Redis pools individually Jul 24, 2024
Copy link
Member

@Dav1dde Dav1dde left a comment

Choose a reason for hiding this comment

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

👍 I might have introduced some merge conflicts with min_idle, should be straight forward to update.

@loewenheim loewenheim enabled auto-merge (squash) August 1, 2024 09:44
@loewenheim loewenheim merged commit 824cdb2 into master Aug 1, 2024
24 checks passed
@loewenheim loewenheim deleted the loewenheim/redis-pools branch August 1, 2024 09:57
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.

2 participants