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

Jump to bottom

[WIP] feat: add cache provider implementation #4169

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
+622 −307
Draft

[WIP] feat: add cache provider implementation #4169

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d38096a
feat: add cache provider implementation
milosgajdos Nov 23, 2023
30f8cf7
Update registry/storage/cache/provider/cacheprovider.go
milosgajdos Dec 7, 2023
90665be
update: make cacheprovider.Register panic on error
milosgajdos Dec 7, 2023
15bdb51
refactor: complete refactoring of cache providers and cache config
milosgajdos Dec 8, 2023
6e079b8
Update cache.go
milosgajdos Dec 8, 2023
7bf9aae
Update redis.go
milosgajdos Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/content/about/configuration.md
View file
Open in desktop
Expand Up @@ -507,6 +507,7 @@ Use the `cache` structure to enable caching of data accessed in the storage
backend. Currently, the only available cache provides fast access to layer
metadata, which uses the `blobdescriptor` field if configured.

Currently the only two available cache implementations are `redis` and `inmemory`.
You can set `blobdescriptor` field to `redis` or `inmemory`. If set to `redis`,a
Redis pool caches layer metadata. If set to `inmemory`, an in-memory map caches
layer metadata.
Expand Down
16 changes: 15 additions & 1 deletion registry/handlers/app.go
View file
Open in desktop
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/distribution/distribution/v3/registry/proxy"
"github.com/distribution/distribution/v3/registry/storage"
memorycache "github.com/distribution/distribution/v3/registry/storage/cache/memory"
cacheprovider "github.com/distribution/distribution/v3/registry/storage/cache/provider"
rediscache "github.com/distribution/distribution/v3/registry/storage/cache/redis"
storagedriver "github.com/distribution/distribution/v3/registry/storage/driver"
"github.com/distribution/distribution/v3/registry/storage/driver/factory"
Expand Down Expand Up @@ -279,7 +280,20 @@ func NewApp(ctx context.Context, config *configuration.Configuration) *App {
dcontext.GetLogger(app).Infof("using inmemory blob descriptor cache")
default:
if v != "" {
dcontext.GetLogger(app).Warnf("unknown cache type %q, caching disabled", config.Storage["cache"])
name, ok := v.(string)
if !ok {
panic(fmt.Sprintf("unexpected type of value %T (string expected)", v))
}
cacheProvider, err := cacheprovider.Get(app, name, cc)
if err != nil {
panic("unable to initialize cache provider: " + err.Error())
}
localOptions := append(options, storage.BlobDescriptorCacheProvider(cacheProvider))
app.registry, err = storage.NewRegistry(app, app.driver, localOptions...)
if err != nil {
panic("could not create registry: " + err.Error())
}
dcontext.GetLogger(app).Infof("using %s blob descriptor cache", name)
}
}
}
Expand Down
37 changes: 37 additions & 0 deletions registry/storage/cache/provider/cacheprovider.go
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder how many near-duplicates of this pattern are scattered throughout the codebase. I betcha they could all be replaced with instantiations of a single generic implementation.

type FactoryRegistry[T any, F func(context.Context, map[string]any] struct{ ... }
func NewFactoryRegistry[T any, F func(context.Context, map[string]any) (T, error)]() FactoryRegistry[T]

func (*FactoryRegistry[T, F]) Register(name string, factory F)
func (*FactoryRegistry[T, F]) Get(ctx context.Context, name string, options map[string]any) (T, error)
var Registry = factoryutil.NewFactoryRegistry[cache.BlobDescriptorCacheProvider]()

Copy link
Member Author

Choose a reason for hiding this comment

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

I think with generics, this codebase could use proper massaging.

@@ -0,0 +1,37 @@
package cacheprovider

import (
"context"
"fmt"

"github.com/distribution/distribution/v3/registry/storage/cache"
)

// InitFunc is the type of a CacheProvider factory function and is
// used to register the constructor for different CacheProvider backends.
type InitFunc func(ctx context.Context, options map[string]interface{}) (cache.BlobDescriptorCacheProvider, error)
corhere marked this conversation as resolved.
Show resolved Hide resolved

var cacheProviders map[string]InitFunc

// Register is used to register an InitFunc for
// a CacheProvider backend with the given name.
// It's meant to be called from init() function
// of the cache provider.
func Register(name string, initFunc InitFunc) {
if cacheProviders == nil {
cacheProviders = make(map[string]InitFunc)
}
if _, exists := cacheProviders[name]; exists {
panic(fmt.Sprintf("name already registered: %s", name))
}

cacheProviders[name] = initFunc
}

// Get constructs a CacheProvider with the given options using the named backend.
func Get(ctx context.Context, name string, options map[string]interface{}) (cache.BlobDescriptorCacheProvider, error) {
if initFunc, exists := cacheProviders[name]; exists {
return initFunc(ctx, options)
}
return nil, fmt.Errorf("no cache Provider registered with name: %s", name)
}