Skip to content

Commit

Permalink
fix(cache): make the validity timeout configurable
Browse files Browse the repository at this point in the history
  • Loading branch information
@karol-kokoszka
karol-kokoszka committed Feb 22, 2024
1 parent 5522381 commit c991f13
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 46 deletions.
4 changes: 4 additions & 0 deletions dist/etc/scylla-manager.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ http: 127.0.0.1:5080
# Debug server that allows to run pporf profiling on demand on a live system.
#debug: 127.0.0.1:5112

# Set the validity timeout for Scylla Manager Agent API clients cached by Scylla Manager.
# Use 0 to disable the cache.
#client_cache_timeout: 15m

# Logging configuration.
#logger:
# Where to print logs, stderr or stdout.
Expand Down
3 changes: 2 additions & 1 deletion pkg/cmd/scylla-manager/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ func (s *server) makeServices() error {
drawerStore := store.NewTableStore(s.session, table.Drawer)
secretsStore := store.NewTableStore(s.session, table.Secrets)

s.clusterSvc, err = cluster.NewService(s.session, metrics.NewClusterMetrics().MustRegister(), secretsStore, s.config.TimeoutConfig, s.logger.Named("cluster"))
s.clusterSvc, err = cluster.NewService(s.session, metrics.NewClusterMetrics().MustRegister(), secretsStore, s.config.TimeoutConfig,
s.config.ClientCacheTimeout, s.logger.Named("cluster"))
if err != nil {
return errors.Wrapf(err, "cluster service")
}
Expand Down
44 changes: 23 additions & 21 deletions pkg/config/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,23 @@ type SSLConfig struct {

// Config contains configuration structure for scylla manager.
type Config struct {
HTTP string `yaml:"http"`
HTTPS string `yaml:"https"`
TLSVersion config.TLSVersion `yaml:"tls_version"`
TLSCertFile string `yaml:"tls_cert_file"`
TLSKeyFile string `yaml:"tls_key_file"`
TLSCAFile string `yaml:"tls_ca_file"`
Prometheus string `yaml:"prometheus"`
Debug string `yaml:"debug"`
Logger config.LogConfig `yaml:"logger"`
Database DBConfig `yaml:"database"`
SSL SSLConfig `yaml:"ssl"`
Healthcheck healthcheck.Config `yaml:"healthcheck"`
Backup backup.Config `yaml:"backup"`
Restore restore.Config `yaml:"restore"`
Repair repair.Config `yaml:"repair"`
TimeoutConfig scyllaclient.TimeoutConfig `yaml:"agent_client"`
HTTP string `yaml:"http"`
HTTPS string `yaml:"https"`
TLSVersion config.TLSVersion `yaml:"tls_version"`
TLSCertFile string `yaml:"tls_cert_file"`
TLSKeyFile string `yaml:"tls_key_file"`
TLSCAFile string `yaml:"tls_ca_file"`
Prometheus string `yaml:"prometheus"`
Debug string `yaml:"debug"`
ClientCacheTimeout time.Duration `yaml:"client_cache_timeout"`
Logger config.LogConfig `yaml:"logger"`
Database DBConfig `yaml:"database"`
SSL SSLConfig `yaml:"ssl"`
Healthcheck healthcheck.Config `yaml:"healthcheck"`
Backup backup.Config `yaml:"backup"`
Restore restore.Config `yaml:"restore"`
Repair repair.Config `yaml:"repair"`
TimeoutConfig scyllaclient.TimeoutConfig `yaml:"agent_client"`
}

func DefaultConfig() Config {
Expand All @@ -83,11 +84,12 @@ func DefaultConfig() Config {
SSL: SSLConfig{
Validate: true,
},
Healthcheck: healthcheck.DefaultConfig(),
Backup: backup.DefaultConfig(),
Restore: restore.DefaultConfig(),
Repair: repair.DefaultConfig(),
TimeoutConfig: scyllaclient.DefaultTimeoutConfig(),
Healthcheck: healthcheck.DefaultConfig(),
ClientCacheTimeout: 15 * time.Minute,
Backup: backup.DefaultConfig(),
Restore: restore.DefaultConfig(),
Repair: repair.DefaultConfig(),
TimeoutConfig: scyllaclient.DefaultTimeoutConfig(),
}
}

Expand Down
17 changes: 9 additions & 8 deletions pkg/config/server/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,15 @@ func TestConfigModification(t *testing.T) {
}

golden := server.Config{
HTTP: "127.0.0.1:80",
HTTPS: "127.0.0.1:443",
TLSVersion: "TLSv1.3",
TLSCertFile: "tls.cert",
TLSKeyFile: "tls.key",
TLSCAFile: "ca.cert",
Prometheus: "127.0.0.1:9090",
Debug: "127.0.0.1:112",
HTTP: "127.0.0.1:80",
HTTPS: "127.0.0.1:443",
TLSVersion: "TLSv1.3",
TLSCertFile: "tls.cert",
TLSKeyFile: "tls.key",
TLSCAFile: "ca.cert",
Prometheus: "127.0.0.1:9090",
Debug: "127.0.0.1:112",
ClientCacheTimeout: 15 * time.Minute,
Logger: config.LogConfig{
Config: log.Config{
Mode: log.StderrMode,
Expand Down
13 changes: 3 additions & 10 deletions pkg/scyllaclient/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import (
// ProviderFunc is a function that returns a Client for a given cluster.
type ProviderFunc func(ctx context.Context, clusterID uuid.UUID) (*Client, error)

// hostCheckValidity specifies how often to check if hosts changed.
const hostCheckValidity = 15 * time.Minute

type clientTTL struct {
client *Client
ttl time.Time
Expand All @@ -32,10 +29,10 @@ type CachedProvider struct {
logger log.Logger
}

func NewCachedProvider(f ProviderFunc, logger log.Logger) *CachedProvider {
func NewCachedProvider(f ProviderFunc, cacheInvalidationTimeout time.Duration, logger log.Logger) *CachedProvider {
return &CachedProvider{
inner: f,
validity: hostCheckValidity,
validity: cacheInvalidationTimeout,
clients: make(map[uuid.UUID]clientTTL),
logger: logger.Named("cache-provider"),
}
Expand All @@ -49,16 +46,12 @@ func (p *CachedProvider) Client(ctx context.Context, clusterID uuid.UUID) (*Clie

// Cache hit
if ok {
if c.ttl.After(timeutc.Now()) {
return c.client, nil
}

// Check if hosts did not change before returning
changed, err := c.client.CheckHostsChanged(ctx)
if err != nil {
p.logger.Error(ctx, "Cannot check if hosts changed", "error", err)
}
if !changed && err == nil {
if c.ttl.After(timeutc.Now()) && !changed && err == nil {
return c.client, nil
}
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/service/cluster/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"sort"
"strconv"
"time"

"github.com/gocql/gocql"
"github.com/pkg/errors"
Expand Down Expand Up @@ -56,7 +57,9 @@ type Service struct {
onChangeListener func(ctx context.Context, c Change) error
}

func NewService(session gocqlx.Session, metrics metrics.ClusterMetrics, secretsStore store.Store, timeoutConfig scyllaclient.TimeoutConfig, l log.Logger) (*Service, error) {
func NewService(session gocqlx.Session, metrics metrics.ClusterMetrics, secretsStore store.Store, timeoutConfig scyllaclient.TimeoutConfig,
cacheInvalidationTimeout time.Duration, l log.Logger,
) (*Service, error) {
if session.Session == nil || session.Closed() {
return nil, errors.New("invalid session")
}
Expand All @@ -68,7 +71,7 @@ func NewService(session gocqlx.Session, metrics metrics.ClusterMetrics, secretsS
logger: l,
timeoutConfig: timeoutConfig,
}
s.clientCache = scyllaclient.NewCachedProvider(s.createClient)
s.clientCache = scyllaclient.NewCachedProvider(s.createClient, cacheInvalidationTimeout, l)

return s, nil
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/service/cluster/service_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/pkg/errors"
"github.com/scylladb/go-log"
"github.com/scylladb/scylla-manager/v3/pkg/config/server"
. "github.com/scylladb/scylla-manager/v3/pkg/testutils/testconfig"

"github.com/scylladb/scylla-manager/v3/pkg/metrics"
Expand All @@ -35,7 +36,8 @@ func TestClientIntegration(t *testing.T) {

session := CreateScyllaManagerDBSession(t)
secretsStore := store.NewTableStore(session, table.Secrets)
s, err := cluster.NewService(session, metrics.NewClusterMetrics(), secretsStore, scyllaclient.DefaultTimeoutConfig(), log.NewDevelopment())
s, err := cluster.NewService(session, metrics.NewClusterMetrics(), secretsStore, scyllaclient.DefaultTimeoutConfig(),
server.DefaultConfig().ClientCacheTimeout, log.NewDevelopment())
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -103,7 +105,8 @@ func TestServiceStorageIntegration(t *testing.T) {

secretsStore := store.NewTableStore(session, table.Secrets)

s, err := cluster.NewService(session, metrics.NewClusterMetrics(), secretsStore, scyllaclient.DefaultTimeoutConfig(), log.NewDevelopment())
s, err := cluster.NewService(session, metrics.NewClusterMetrics(), secretsStore, scyllaclient.DefaultTimeoutConfig(),
server.DefaultConfig().ClientCacheTimeout, log.NewDevelopment())
if err != nil {
t.Fatal(err)
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/service/healthcheck/service_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ func TestStatusIntegration(t *testing.T) {
defer session.Close()

s := store.NewTableStore(session, table.Secrets)
clusterSvc, err := cluster.NewService(session, metrics.NewClusterMetrics(), s, scyllaclient.DefaultTimeoutConfig(), log.NewDevelopment())
clusterSvc, err := cluster.NewService(session, metrics.NewClusterMetrics(), s, scyllaclient.DefaultTimeoutConfig(),
15*time.Minute, log.NewDevelopment())
if err != nil {
t.Fatal(err)
}
Expand All @@ -69,7 +70,8 @@ func TestStatusWithCQLCredentialsIntegration(t *testing.T) {
defer session.Close()

s := store.NewTableStore(session, table.Secrets)
clusterSvc, err := cluster.NewService(session, metrics.NewClusterMetrics(), s, scyllaclient.DefaultTimeoutConfig(), log.NewDevelopment())
clusterSvc, err := cluster.NewService(session, metrics.NewClusterMetrics(), s, scyllaclient.DefaultTimeoutConfig(),
15*time.Minute, log.NewDevelopment())
if err != nil {
t.Fatal(err)
}
Expand Down

0 comments on commit c991f13

Please sign in to comment.