-
Notifications
You must be signed in to change notification settings - Fork 59
Add inmemory cache for env #636
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
Conversation
pkg/environments/environments.go
Outdated
// Environment keeps all TLS Environments | ||
type Environment struct { | ||
// EnvironmentManager keeps all TLS Environments | ||
type EnvironmentManager struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe the name can be EnvManager
to reduce its length
pkg/environments/env-cache.go
Outdated
func NewEnvCache(envs EnvManager) *EnvCache { | ||
// Create a new cache with a 10-minute cleanup interval | ||
envCache := cache.NewMemoryCache( | ||
cache.WithCleanupInterval[TLSEnvironment](2 * time.Hour), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value is currently hardcoded, and I think we need a more robust approach to configuration management. Otherwise, the number of command-line flags will quickly become unmanageable and difficult to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done!
This PR introduces an in-memory cache to store parameters that rarely change, reducing the number of requests to the backend database. However, it should only be used for bounded values—such as the environment and node key—because the cache is currently unbounded and may grow indefinitely, potentially leading to OOMKilled errors.
At this point, the cache has been added for the
environment
value only.