diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2021-03-01 17:53:32 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2021-03-04 10:27:17 +0300 |
commit | 38ef5f65d7d0cd82d97ebf746eb514b08afdf411 (patch) | |
tree | a43b3280c5f20da53f101391047d6ad2cf375140 /internal | |
parent | ac76e9e62f60d2419dcbf9e6b2d809c1dfe2f441 (diff) |
remove DirectStorageProvider
This commit removes DirectStorageProvider as it is just a proxy to
call GetConsistentStorages on the RepositoryStore.
Diffstat (limited to 'internal')
-rw-r--r-- | internal/praefect/coordinator_test.go | 4 | ||||
-rw-r--r-- | internal/praefect/datastore/storage_provider.go | 31 | ||||
-rw-r--r-- | internal/praefect/datastore/storage_provider_test.go | 54 | ||||
-rw-r--r-- | internal/praefect/helper_test.go | 3 | ||||
-rw-r--r-- | internal/praefect/nodes/manager_test.go | 3 | ||||
-rw-r--r-- | internal/praefect/server_factory_test.go | 3 | ||||
-rw-r--r-- | internal/praefect/server_test.go | 3 |
7 files changed, 13 insertions, 88 deletions
diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go index 5cd7d218b..dc4dd77bc 100644 --- a/internal/praefect/coordinator_test.go +++ b/internal/praefect/coordinator_test.go @@ -466,9 +466,7 @@ func TestCoordinatorStreamDirector_distributesReads(t *testing.T) { }, } - sp := datastore.NewDirectStorageProvider(repoStore) - - nodeMgr, err := nodes.NewManager(entry, conf, nil, sp, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) + nodeMgr, err := nodes.NewManager(entry, conf, nil, repoStore, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) require.NoError(t, err) nodeMgr.Start(0, time.Minute) diff --git a/internal/praefect/datastore/storage_provider.go b/internal/praefect/datastore/storage_provider.go index 2541db036..50362cf63 100644 --- a/internal/praefect/datastore/storage_provider.go +++ b/internal/praefect/datastore/storage_provider.go @@ -20,28 +20,13 @@ type ConsistentStoragesGetter interface { GetConsistentStorages(ctx context.Context, virtualStorage, relativePath string) (map[string]struct{}, error) } -// DirectConsistentStoragesGetter provides the latest state of the synced nodes. -type DirectConsistentStoragesGetter struct { - sp ConsistentStoragesGetter -} - -// NewDirectConsistentStoragesGetter returns a new storage provider. -func NewDirectConsistentStoragesGetter(sp ConsistentStoragesGetter) *DirectConsistentStoragesGetter { - return &DirectConsistentStoragesGetter{sp: sp} -} - -// GetConsistentStorages returns list of gitaly storages that are in up to date state based on the generation tracking. -func (c *DirectConsistentStoragesGetter) GetConsistentStorages(ctx context.Context, virtualStorage, relativePath string) (map[string]struct{}, error) { - return c.sp.GetConsistentStorages(ctx, virtualStorage, relativePath) -} - // errNotExistingVirtualStorage indicates that the requested virtual storage can't be found or not configured. var errNotExistingVirtualStorage = errors.New("virtual storage does not exist") // CachingConsistentStoragesGetter is a ConsistentStoragesGetter that caches up to date storages by repository. // Each virtual storage has it's own cache that invalidates entries based on notifications. type CachingConsistentStoragesGetter struct { - dsp *DirectConsistentStoragesGetter + csg ConsistentStoragesGetter // caches is per virtual storage cache. It is initialized once on construction. caches map[string]*lru.Cache // access is access method to use: 0 - without caching; 1 - with caching. @@ -54,9 +39,9 @@ type CachingConsistentStoragesGetter struct { } // NewCachingConsistentStoragesGetter returns a ConsistentStoragesGetter that uses caching. -func NewCachingConsistentStoragesGetter(logger logrus.FieldLogger, sp ConsistentStoragesGetter, virtualStorages []string) (*CachingConsistentStoragesGetter, error) { - csp := &CachingConsistentStoragesGetter{ - dsp: NewDirectConsistentStoragesGetter(sp), +func NewCachingConsistentStoragesGetter(logger logrus.FieldLogger, csg ConsistentStoragesGetter, virtualStorages []string) (*CachingConsistentStoragesGetter, error) { + cached := &CachingConsistentStoragesGetter{ + csg: csg, caches: make(map[string]*lru.Cache, len(virtualStorages)), syncer: syncer{inflight: map[string]chan struct{}{}}, callbackLogger: logger.WithField("component", "caching_storage_provider"), @@ -72,15 +57,15 @@ func NewCachingConsistentStoragesGetter(logger logrus.FieldLogger, sp Consistent for _, virtualStorage := range virtualStorages { virtualStorage := virtualStorage cache, err := lru.NewWithEvict(2<<20, func(key interface{}, value interface{}) { - csp.cacheAccessTotal.WithLabelValues(virtualStorage, "evict").Inc() + cached.cacheAccessTotal.WithLabelValues(virtualStorage, "evict").Inc() }) if err != nil { return nil, err } - csp.caches[virtualStorage] = cache + cached.caches[virtualStorage] = cache } - return csp, nil + return cached, nil } type notificationEntry struct { @@ -150,7 +135,7 @@ func (c *CachingConsistentStoragesGetter) getCache(virtualStorage string) (*lru. func (c *CachingConsistentStoragesGetter) cacheMiss(ctx context.Context, virtualStorage, relativePath string) (map[string]struct{}, error) { c.cacheAccessTotal.WithLabelValues(virtualStorage, "miss").Inc() - return c.dsp.GetConsistentStorages(ctx, virtualStorage, relativePath) + return c.csg.GetConsistentStorages(ctx, virtualStorage, relativePath) } func (c *CachingConsistentStoragesGetter) tryCache(virtualStorage, relativePath string) (func(), *lru.Cache, map[string]struct{}, bool) { diff --git a/internal/praefect/datastore/storage_provider_test.go b/internal/praefect/datastore/storage_provider_test.go index f4e0d4652..f68ebfa1e 100644 --- a/internal/praefect/datastore/storage_provider_test.go +++ b/internal/praefect/datastore/storage_provider_test.go @@ -19,60 +19,6 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/testhelper" ) -func TestDirectStorageProvider_GetSyncedNodes(t *testing.T) { - t.Run("ok", func(t *testing.T) { - ctx, cancel := testhelper.Context() - defer cancel() - - for _, tc := range []struct { - desc string - ret map[string]struct{} - exp map[string]struct{} - }{ - { - desc: "primary included", - ret: map[string]struct{}{"g2": {}, "g3": {}}, - exp: map[string]struct{}{"g2": {}, "g3": {}}, - }, - { - desc: "distinct values", - ret: map[string]struct{}{"g1": {}, "g2": {}, "g3": {}}, - exp: map[string]struct{}{"g1": {}, "g2": {}, "g3": {}}, - }, - { - desc: "none", - ret: nil, - exp: nil, - }, - } { - t.Run(tc.desc, func(t *testing.T) { - rs := &mockConsistentSecondariesProvider{} - rs.On("GetConsistentStorages", ctx, "vs", "/repo/path").Return(tc.ret, nil) - - sp := NewDirectConsistentStoragesGetter(rs) - storages, err := sp.GetConsistentStorages(ctx, "vs", "/repo/path") - require.NoError(t, err) - require.Equal(t, tc.exp, storages) - }) - } - }) - - t.Run("repository store returns an error", func(t *testing.T) { - ctx, cancel := testhelper.Context(testhelper.ContextWithLogger(testhelper.DiscardTestEntry(t))) - defer cancel() - - rs := &mockConsistentSecondariesProvider{} - rs.On("GetConsistentStorages", ctx, "vs", "/repo/path"). - Return(nil, assert.AnError). - Once() - - sp := NewDirectConsistentStoragesGetter(rs) - - _, err := sp.GetConsistentStorages(ctx, "vs", "/repo/path") - require.Equal(t, assert.AnError, err) - }) -} - type mockConsistentSecondariesProvider struct { mock.Mock } diff --git a/internal/praefect/helper_test.go b/internal/praefect/helper_test.go index 0c584f3bb..030ba8d89 100644 --- a/internal/praefect/helper_test.go +++ b/internal/praefect/helper_test.go @@ -122,8 +122,7 @@ func defaultTxMgr(conf config.Config) *transactions.Manager { } func defaultNodeMgr(t testing.TB, conf config.Config, rs datastore.RepositoryStore) nodes.Manager { - sp := datastore.NewDirectStorageProvider(rs) - nodeMgr, err := nodes.NewManager(testhelper.DiscardTestEntry(t), conf, nil, sp, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) + nodeMgr, err := nodes.NewManager(testhelper.DiscardTestEntry(t), conf, nil, rs, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) require.NoError(t, err) nodeMgr.Start(0, time.Hour) return nodeMgr diff --git a/internal/praefect/nodes/manager_test.go b/internal/praefect/nodes/manager_test.go index be61d7a21..8661adfe6 100644 --- a/internal/praefect/nodes/manager_test.go +++ b/internal/praefect/nodes/manager_test.go @@ -334,9 +334,8 @@ func TestMgr_GetSyncedNode(t *testing.T) { return consistentStorages, consistentSecondariesErr }, } - sp := datastore.NewDirectConsistentStoragesGetter(rs) - nm, err := NewManager(testhelper.DiscardTestEntry(t), conf, nil, sp, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) + nm, err := NewManager(testhelper.DiscardTestEntry(t), conf, nil, rs, promtest.NewMockHistogramVec(), protoregistry.GitalyProtoPreregistered, nil) require.NoError(t, err) for i := range healthSrvs { diff --git a/internal/praefect/server_factory_test.go b/internal/praefect/server_factory_test.go index 9d5783503..e21a6d753 100644 --- a/internal/praefect/server_factory_test.go +++ b/internal/praefect/server_factory_test.go @@ -94,8 +94,7 @@ func TestServerFactory(t *testing.T) { queue := datastore.NewMemoryReplicationEventQueue(conf) rs := datastore.MockRepositoryStore{} - sp := datastore.NewDirectStorageProvider(rs) - nodeMgr, err := nodes.NewManager(logger, conf, nil, sp, &promtest.MockHistogramVec{}, protoregistry.GitalyProtoPreregistered, nil) + nodeMgr, err := nodes.NewManager(logger, conf, nil, rs, &promtest.MockHistogramVec{}, protoregistry.GitalyProtoPreregistered, nil) require.NoError(t, err) nodeMgr.Start(0, time.Second) txMgr := transactions.NewManager(conf) diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go index 1aa298b35..26371d99b 100644 --- a/internal/praefect/server_test.go +++ b/internal/praefect/server_test.go @@ -866,8 +866,7 @@ func TestErrorThreshold(t *testing.T) { require.NoError(t, err) rs := datastore.MockRepositoryStore{} - sp := datastore.NewDirectStorageProvider(rs) - nodeMgr, err := nodes.NewManager(entry, conf, nil, sp, promtest.NewMockHistogramVec(), registry, errorTracker) + nodeMgr, err := nodes.NewManager(entry, conf, nil, rs, promtest.NewMockHistogramVec(), registry, errorTracker) require.NoError(t, err) coordinator := NewCoordinator( |