Skip to content

Commit

Permalink
Remove on-disk caches
Browse files Browse the repository at this point in the history
When doing any sort of restart (either in-process or across the process
boundary) with sqlite caches, we end up firing up multiple Caches over
sqlite backends over the same database. This is very much broken, as
there's no inherent synchronization between caches and the old and new
caches will happily end up stepping on each other's toes (the new cache
will begin its job by fetching all relevant resources, *clearing the
backend* and then reinserting the resources while the old cache is still
using the same backend).

As restarts while auth is unreachable are already broken, there seems to
be no real point to maintaining any sort of persistent on-disk cache;
memory usage could be an issue but the cache for node and node-like
roles should be pretty small to begin with, and for big deployments we
are already recommending that proxies use in-memory caches (as per
docs/pages/setup/operations/scaling.mdx ).
  • Loading branch information
espadolini committed Mar 2, 2022
1 parent 0644267 commit 7e74595
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 54 deletions.
3 changes: 3 additions & 0 deletions lib/config/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,9 @@ func ApplyFileConfig(fc *FileConfig, cfg *service.Config) error {
if fc.CachePolicy.TTL != "" {
log.Warnf("cache.ttl config option is deprecated and will be ignored, caches no longer attempt to anticipate resource expiration.")
}
if fc.CachePolicy.Type != "" {
log.Warnf("cache.type config option is deprecated and will be ignored, caches are always in-memory.")
}

// apply cache policy for node and proxy
cachePolicy, err := fc.CachePolicy.Parse()
Expand Down
12 changes: 6 additions & 6 deletions lib/config/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -920,12 +920,12 @@ func TestParseCachePolicy(t *testing.T) {
out *service.CachePolicy
err error
}{
{in: &CachePolicy{EnabledFlag: "yes", TTL: "never"}, out: &service.CachePolicy{Enabled: true, Type: lite.GetName()}},
{in: &CachePolicy{EnabledFlag: "yes", TTL: "10h"}, out: &service.CachePolicy{Enabled: true, Type: lite.GetName()}},
{in: &CachePolicy{Type: memory.GetName(), EnabledFlag: "false", TTL: "10h"}, out: &service.CachePolicy{Enabled: false, Type: memory.GetName()}},
{in: &CachePolicy{Type: memory.GetName(), EnabledFlag: "yes", TTL: "never"}, out: &service.CachePolicy{Enabled: true, Type: memory.GetName()}},
{in: &CachePolicy{EnabledFlag: "no"}, out: &service.CachePolicy{Type: lite.GetName(), Enabled: false}},
{in: &CachePolicy{Type: "memsql"}, err: trace.BadParameter("unsupported backend")},
{in: &CachePolicy{EnabledFlag: "yes", TTL: "never"}, out: &service.CachePolicy{Enabled: true}},
{in: &CachePolicy{EnabledFlag: "yes", TTL: "10h"}, out: &service.CachePolicy{Enabled: true}},
{in: &CachePolicy{Type: memory.GetName(), EnabledFlag: "false", TTL: "10h"}, out: &service.CachePolicy{Enabled: false}},
{in: &CachePolicy{Type: memory.GetName(), EnabledFlag: "yes", TTL: "never"}, out: &service.CachePolicy{Enabled: true}},
{in: &CachePolicy{EnabledFlag: "no"}, out: &service.CachePolicy{Enabled: false}},
{in: &CachePolicy{Type: "memsql"}, out: &service.CachePolicy{Enabled: true}},
}
for i, tc := range tcs {
comment := fmt.Sprintf("test case #%v", i)
Expand Down
1 change: 0 additions & 1 deletion lib/config/fileconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,6 @@ func (c *CachePolicy) Enabled() bool {
// Parse parses cache policy from Teleport config
func (c *CachePolicy) Parse() (*service.CachePolicy, error) {
out := service.CachePolicy{
Type: c.Type,
Enabled: c.Enabled(),
}
if err := out.CheckAndSetDefaults(); err != nil {
Expand Down
13 changes: 1 addition & 12 deletions lib/service/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import (
"github.com/gravitational/teleport/lib/auth/keystore"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/backend/lite"
"github.com/gravitational/teleport/lib/backend/memory"
"github.com/gravitational/teleport/lib/bpf"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
Expand Down Expand Up @@ -304,22 +303,12 @@ func (cfg *Config) DebugDumpToYAML() string {

// CachePolicy sets caching policy for proxies and nodes
type CachePolicy struct {
// Type sets the cache type
Type string
// Enabled enables or disables caching
Enabled bool
}

// CheckAndSetDefaults checks and sets default values
func (c *CachePolicy) CheckAndSetDefaults() error {
switch c.Type {
case "", lite.GetName():
c.Type = lite.GetName()
case memory.GetName():
default:
return trace.BadParameter("unsupported cache type %q, supported values are %q and %q",
c.Type, lite.GetName(), memory.GetName())
}
return nil
}

Expand All @@ -328,7 +317,7 @@ func (c CachePolicy) String() string {
if !c.Enabled {
return "no cache policy"
}
return fmt.Sprintf("%v cache will store frequently accessed items", c.Type)
return "in-memory cache will store frequently accessed items"
}

// ProxyConfig specifies configuration for proxy service
Expand Down
43 changes: 8 additions & 35 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -1307,7 +1307,6 @@ func (process *TeleportProcess) initAuthService() error {
services: authServer.Services,
setup: cache.ForAuth,
cacheName: []string{teleport.ComponentAuth},
inMemory: true,
events: true,
})
if err != nil {
Expand Down Expand Up @@ -1527,9 +1526,6 @@ type accessCacheConfig struct {
setup cache.SetupConfigFn
// cacheName is a cache name
cacheName []string
// inMemory is true if cache
// should use memory
inMemory bool
// events is true if cache should turn on events
events bool
// pollPeriod contains period for polling
Expand Down Expand Up @@ -1557,36 +1553,14 @@ func (process *TeleportProcess) newAccessCache(cfg accessCacheConfig) (*cache.Ca
if err := cfg.CheckAndSetDefaults(); err != nil {
return nil, trace.Wrap(err)
}
var cacheBackend backend.Backend
if cfg.inMemory {
process.log.Debugf("Creating in-memory backend for %v.", cfg.cacheName)
mem, err := memory.New(memory.Config{
Context: process.ExitContext(),
EventsOff: !cfg.events,
Mirror: true,
})
if err != nil {
return nil, trace.Wrap(err)
}
cacheBackend = mem
} else {
process.log.Debugf("Creating sqlite backend for %v.", cfg.cacheName)
path := filepath.Join(append([]string{process.Config.DataDir, "cache"}, cfg.cacheName...)...)
if err := os.MkdirAll(path, teleport.SharedDirMode); err != nil {
return nil, trace.ConvertSystemError(err)
}
liteBackend, err := lite.NewWithConfig(process.ExitContext(),
lite.Config{
Path: path,
EventsOff: !cfg.events,
Memory: false,
Mirror: true,
PollStreamPeriod: 100 * time.Millisecond,
})
if err != nil {
return nil, trace.Wrap(err)
}
cacheBackend = liteBackend
process.log.Debugf("Creating in-memory backend for %v.", cfg.cacheName)
cacheBackend, err := memory.New(memory.Config{
Context: process.ExitContext(),
EventsOff: !cfg.events,
Mirror: true,
})
if err != nil {
return nil, trace.Wrap(err)
}
reporter, err := backend.NewReporter(backend.ReporterConfig{
Component: teleport.ComponentCache,
Expand Down Expand Up @@ -1745,7 +1719,6 @@ func (process *TeleportProcess) newLocalCacheForWindowsDesktop(clt auth.ClientI,
// newLocalCache returns new instance of access point
func (process *TeleportProcess) newLocalCache(clt auth.ClientI, setupConfig cache.SetupConfigFn, cacheName []string) (*cache.Cache, error) {
return process.newAccessCache(accessCacheConfig{
inMemory: process.Config.CachePolicy.Type == memory.GetName(),
services: clt,
setup: setupConfig,
cacheName: cacheName,
Expand Down

0 comments on commit 7e74595

Please sign in to comment.