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.

Sign up for GitHub

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: Pass redis connection pool settings to config #1140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: Pass redis connection pool settings to config #1140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6560775
Pass redis connection pool settings to config
Tetrergeru Feb 5, 2025
2a7e4a6
Fix parameters passing
Tetrergeru Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions cmd/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ type RedisConfig struct {
// Allows routing read-only commands to the **random** master or slave node.
// It automatically enables ReadOnly.
RouteRandomly bool `yaml:"route_randomly"`
// Time to await for a client from client pool. Default value is 4s.
PoolTimeout string `yaml:"pool_timeout"`
// Size of client pool. Default value is 5 * GOMAXPROCS.
PoolSize int `yaml:"pool_size"`
Copy link
Member

Choose a reason for hiding this comment

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

А я правильно понимаю, что такие дефолтные значения задаёт либа? Мб тогда явно их пропишем, а то вдруг что-то в либе поменяется?

Copy link
Member

Choose a reason for hiding this comment

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

Либо из godoc уберём)

Copy link
Member Author

Choose a reason for hiding this comment

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

Возможно это хорошая идея, но надо подумать, как лучше сделать. Альтернативный вариант — обновлять документацию вместе с одновлением либы, потому что если в либе поменяют дефолтные значения — значит, скорее всего, в этом есть какой-то смысл

Copy link
Member Author

Choose a reason for hiding this comment

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

Ещё возможно стоит прокинуть в конфиг дополнительный параметр, чтобы можно было через конфиг задавать не просто фиксированное значение, а что-то типа PoolPerProc * GOMAXPROCS + PollSizeConst

Copy link
Member

Choose a reason for hiding this comment

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

Ещё возможно стоит прокинуть в конфиг дополнительный параметр, чтобы можно было через конфиг задавать не просто фиксированное значение, а что-то типа PoolPerProc * GOMAXPROCS + PollSizeConst

Вот это звучит прикольно

Альтернативный вариант — обновлять документацию вместе с одновлением либы, потому что если в либе поменяют дефолтные значения — значит, скорее всего, в этом есть какой-то смысл

Можно и так, но вариант с проставленными нами дефолтными значениями мне немного больше импонирует

Copy link
Member Author

Choose a reason for hiding this comment

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

Тогда, кстати, надо бы всем параметрам дефолтные значения выставить, вроде у базы все неявно ожидаются от библиотеки

Copy link
Member

Choose a reason for hiding this comment

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

блинб, и их же много да?

}

// GetSettings returns redis config parsed from moira config files.
Expand All @@ -84,6 +88,8 @@ func (config *RedisConfig) GetSettings() redis.DatabaseConfig {
ReadOnly: config.ReadOnly,
RouteByLatency: config.RouteByLatency,
RouteRandomly: config.RouteRandomly,
PoolTimeout: to.Duration(config.PoolTimeout),
PoolSize: config.PoolSize,
}
}

Expand Down
3 changes: 3 additions & 0 deletions database/redis/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ type DatabaseConfig struct {
MinRetryBackoff time.Duration
MaxRetryBackoff time.Duration

PoolTimeout time.Duration
PoolSize int

ReadOnly bool
RouteByLatency bool
RouteRandomly bool
Expand Down
3 changes: 3 additions & 0 deletions database/redis/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ func NewDatabase(logger moira.Logger, config DatabaseConfig, nh NotificationHist
MinRetryBackoff: config.MinRetryBackoff,
MaxRetryBackoff: config.MaxRetryBackoff,

PoolTimeout: config.PoolTimeout,
PoolSize: config.PoolSize,

ReadOnly: config.ReadOnly,
RouteByLatency: config.RouteByLatency,
RouteRandomly: config.RouteRandomly,
Expand Down