From f9cb37e23dded563c24bd363eed99f74302cfd32 Mon Sep 17 00:00:00 2001 From: Nate Broyles Date: Wed, 7 Oct 2020 17:27:30 -0400 Subject: [PATCH 1/3] Convert bootstrap.Cache to interface type --- src/dbnode/storage/bootstrap/cache.go | 21 ++++++++++++--------- src/dbnode/storage/bootstrap/types.go | 15 ++++++++++----- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/dbnode/storage/bootstrap/cache.go b/src/dbnode/storage/bootstrap/cache.go index 93640cd259..f24bbaf637 100644 --- a/src/dbnode/storage/bootstrap/cache.go +++ b/src/dbnode/storage/bootstrap/cache.go @@ -35,22 +35,28 @@ var ( errInstrumentOptsNotSet = errors.New("instrumentOptions not set") ) +type cache struct { + fsOpts fs.Options + namespaceDetails []NamespaceDetails + infoFilesByNamespace InfoFilesByNamespace + iOpts instrument.Options +} + // NewCache creates a cache specifically to be used during the bootstrap process. // Primarily a mechanism for passing info files along without needing to re-read them at each // stage of the bootstrap process. func NewCache(options CacheOptions) (Cache, error) { if err := options.Validate(); err != nil { - return Cache{}, err + return nil, err } - return Cache{ + return &cache{ fsOpts: options.FilesystemOptions(), namespaceDetails: options.NamespaceDetails(), iOpts: options.InstrumentOptions(), }, nil } -// InfoFilesForNamespace returns the info files grouped by shard for the provided namespace. -func (c *Cache) InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPerShard, error) { +func (c *cache) InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPerShard, error) { infoFilesByShard, ok := c.ReadInfoFiles()[ns] // This should never happen as Cache object is initialized with all namespaces to bootstrap. if !ok { @@ -60,8 +66,7 @@ func (c *Cache) InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPer return infoFilesByShard, nil } -// InfoFilesForShard returns the info files grouped by shard for the provided namespace. -func (c *Cache) InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.ReadInfoFileResult, error) { +func (c *cache) InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.ReadInfoFileResult, error) { infoFilesByShard, err := c.InfoFilesForNamespace(ns) if err != nil { return nil, err @@ -75,9 +80,7 @@ func (c *Cache) InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.Rea return infoFileResults, nil } -// ReadInfoFiles returns info file results for each shard grouped by namespace. A cached copy -// is returned if the info files have already been read. -func (c *Cache) ReadInfoFiles() InfoFilesByNamespace { +func (c *cache) ReadInfoFiles() InfoFilesByNamespace { if c.infoFilesByNamespace != nil { return c.infoFilesByNamespace } diff --git a/src/dbnode/storage/bootstrap/types.go b/src/dbnode/storage/bootstrap/types.go index 467dd1f436..62b1d8d015 100644 --- a/src/dbnode/storage/bootstrap/types.go +++ b/src/dbnode/storage/bootstrap/types.go @@ -432,11 +432,16 @@ type InfoFilesByNamespace map[namespace.Metadata]InfoFileResultsPerShard // Cache provides a snapshot of info files for use throughout all stages of the bootstrap. // This struct is not threadsafe and relies on the single-threaded nature of the bootstrap // process. -type Cache struct { - fsOpts fs.Options - namespaceDetails []NamespaceDetails - infoFilesByNamespace InfoFilesByNamespace - iOpts instrument.Options +type Cache interface { + // InfoFilesForNamespace returns the info files grouped by shard for the provided namespace. + InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPerShard, error) + + // InfoFilesForShard returns the info files grouped by shard for the provided namespace. + InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.ReadInfoFileResult, error) + + // ReadInfoFiles returns info file results for each shard grouped by namespace. A cached copy + // is returned if the info files have already been read. + ReadInfoFiles() InfoFilesByNamespace } // CacheOptions represents the options for Cache. From e6bf421b901b770dfc6b9fd67a568f815b496991 Mon Sep 17 00:00:00 2001 From: Nate Broyles Date: Wed, 7 Oct 2020 17:31:49 -0400 Subject: [PATCH 2/3] Add sync.Once around ReadInfoFiles --- src/dbnode/storage/bootstrap/cache.go | 29 ++++++++++++++------------- src/dbnode/storage/bootstrap/types.go | 4 +--- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/dbnode/storage/bootstrap/cache.go b/src/dbnode/storage/bootstrap/cache.go index f24bbaf637..f5468e773c 100644 --- a/src/dbnode/storage/bootstrap/cache.go +++ b/src/dbnode/storage/bootstrap/cache.go @@ -23,6 +23,7 @@ package bootstrap import ( "errors" "fmt" + "sync" "github.com/m3db/m3/src/dbnode/namespace" "github.com/m3db/m3/src/dbnode/persist" @@ -36,6 +37,8 @@ var ( ) type cache struct { + sync.Once + fsOpts fs.Options namespaceDetails []NamespaceDetails infoFilesByNamespace InfoFilesByNamespace @@ -81,21 +84,19 @@ func (c *cache) InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.Rea } func (c *cache) ReadInfoFiles() InfoFilesByNamespace { - if c.infoFilesByNamespace != nil { - return c.infoFilesByNamespace - } - - c.infoFilesByNamespace = make(InfoFilesByNamespace, len(c.namespaceDetails)) - for _, finder := range c.namespaceDetails { - result := make(InfoFileResultsPerShard, len(finder.Shards)) - for _, shard := range finder.Shards { - result[shard] = fs.ReadInfoFiles(c.fsOpts.FilePathPrefix(), - finder.Namespace.ID(), shard, c.fsOpts.InfoReaderBufferSize(), c.fsOpts.DecodingOptions(), - persist.FileSetFlushType) + c.Once.Do(func() { + c.infoFilesByNamespace = make(InfoFilesByNamespace, len(c.namespaceDetails)) + for _, finder := range c.namespaceDetails { + result := make(InfoFileResultsPerShard, len(finder.Shards)) + for _, shard := range finder.Shards { + result[shard] = fs.ReadInfoFiles(c.fsOpts.FilePathPrefix(), + finder.Namespace.ID(), shard, c.fsOpts.InfoReaderBufferSize(), c.fsOpts.DecodingOptions(), + persist.FileSetFlushType) + } + + c.infoFilesByNamespace[finder.Namespace] = result } - - c.infoFilesByNamespace[finder.Namespace] = result - } + }) return c.infoFilesByNamespace } diff --git a/src/dbnode/storage/bootstrap/types.go b/src/dbnode/storage/bootstrap/types.go index 62b1d8d015..186728af31 100644 --- a/src/dbnode/storage/bootstrap/types.go +++ b/src/dbnode/storage/bootstrap/types.go @@ -430,10 +430,8 @@ type InfoFileResultsPerShard map[uint32][]fs.ReadInfoFileResult type InfoFilesByNamespace map[namespace.Metadata]InfoFileResultsPerShard // Cache provides a snapshot of info files for use throughout all stages of the bootstrap. -// This struct is not threadsafe and relies on the single-threaded nature of the bootstrap -// process. type Cache interface { - // InfoFilesForNamespace returns the info files grouped by shard for the provided namespace. + // InfoFilesForNamespace returns the info files grouped by namespace. InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPerShard, error) // InfoFilesForShard returns the info files grouped by shard for the provided namespace. From e0bd10cc3a19b526047d6a50334b23b78f4ac6da Mon Sep 17 00:00:00 2001 From: Nate Broyles Date: Wed, 7 Oct 2020 18:04:52 -0400 Subject: [PATCH 3/3] Re-gen mocks --- .../storage/bootstrap/bootstrap_mock.go | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/dbnode/storage/bootstrap/bootstrap_mock.go b/src/dbnode/storage/bootstrap/bootstrap_mock.go index de89112105..c5a420d702 100644 --- a/src/dbnode/storage/bootstrap/bootstrap_mock.go +++ b/src/dbnode/storage/bootstrap/bootstrap_mock.go @@ -647,6 +647,73 @@ func (mr *MockSourceMockRecorder) Read(ctx, namespaces, cache interface{}) *gomo return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Read", reflect.TypeOf((*MockSource)(nil).Read), ctx, namespaces, cache) } +// MockCache is a mock of Cache interface +type MockCache struct { + ctrl *gomock.Controller + recorder *MockCacheMockRecorder +} + +// MockCacheMockRecorder is the mock recorder for MockCache +type MockCacheMockRecorder struct { + mock *MockCache +} + +// NewMockCache creates a new mock instance +func NewMockCache(ctrl *gomock.Controller) *MockCache { + mock := &MockCache{ctrl: ctrl} + mock.recorder = &MockCacheMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockCache) EXPECT() *MockCacheMockRecorder { + return m.recorder +} + +// InfoFilesForNamespace mocks base method +func (m *MockCache) InfoFilesForNamespace(ns namespace.Metadata) (InfoFileResultsPerShard, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InfoFilesForNamespace", ns) + ret0, _ := ret[0].(InfoFileResultsPerShard) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InfoFilesForNamespace indicates an expected call of InfoFilesForNamespace +func (mr *MockCacheMockRecorder) InfoFilesForNamespace(ns interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InfoFilesForNamespace", reflect.TypeOf((*MockCache)(nil).InfoFilesForNamespace), ns) +} + +// InfoFilesForShard mocks base method +func (m *MockCache) InfoFilesForShard(ns namespace.Metadata, shard uint32) ([]fs.ReadInfoFileResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InfoFilesForShard", ns, shard) + ret0, _ := ret[0].([]fs.ReadInfoFileResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InfoFilesForShard indicates an expected call of InfoFilesForShard +func (mr *MockCacheMockRecorder) InfoFilesForShard(ns, shard interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InfoFilesForShard", reflect.TypeOf((*MockCache)(nil).InfoFilesForShard), ns, shard) +} + +// ReadInfoFiles mocks base method +func (m *MockCache) ReadInfoFiles() InfoFilesByNamespace { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ReadInfoFiles") + ret0, _ := ret[0].(InfoFilesByNamespace) + return ret0 +} + +// ReadInfoFiles indicates an expected call of ReadInfoFiles +func (mr *MockCacheMockRecorder) ReadInfoFiles() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ReadInfoFiles", reflect.TypeOf((*MockCache)(nil).ReadInfoFiles)) +} + // MockCacheOptions is a mock of CacheOptions interface type MockCacheOptions struct { ctrl *gomock.Controller