Skip to content

Commit

Permalink
Cleanup coldflush manager and add a test.
Browse files Browse the repository at this point in the history
  • Loading branch information
notbdu committed Jul 28, 2020
1 parent 2e455a4 commit 945ca7c
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 10 deletions.
8 changes: 2 additions & 6 deletions src/dbnode/storage/coldflush.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"time"

"github.com/m3db/m3/src/dbnode/persist"
"github.com/m3db/m3/src/dbnode/persist/fs/commitlog"
xerrors "github.com/m3db/m3/src/x/errors"
"github.com/m3db/m3/src/x/instrument"

Expand All @@ -34,7 +33,6 @@ import (
)

type coldFlushManager struct {
databaseFlushManager
databaseCleanupManager
sync.RWMutex

Expand All @@ -52,16 +50,14 @@ type coldFlushManager struct {
func newColdFlushManager(
database database,
pm persist.Manager,
commitLog commitlog.CommitLog,
opts Options,
) databaseColdFlushManager {
instrumentOpts := opts.InstrumentOptions()
scope := instrumentOpts.MetricsScope().SubScope("fs")
fm := newFlushManager(database, commitLog, scope)
cm := newCleanupManager(database, commitLog, scope)
// NB(bodu): cold flush cleanup doesn't require commit logs.
cm := newCleanupManager(database, nil, scope)

return &coldFlushManager{
databaseFlushManager: fm,
databaseCleanupManager: cm,
log: instrumentOpts.Logger(),
database: database,
Expand Down
34 changes: 34 additions & 0 deletions src/dbnode/storage/coldflush_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package storage

import (
"errors"
"testing"

"github.com/golang/mock/gomock"
"github.com/m3db/m3/src/dbnode/persist"
"github.com/stretchr/testify/require"
)

func TestColdFlushManagerFlushDoneFlushError(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

var (
fakeErr = errors.New("fake error while marking flush done")
mockPersistManager = persist.NewMockManager(ctrl)
mockFlushPersist = persist.NewMockFlushPreparer(ctrl)
)

mockFlushPersist.EXPECT().DoneFlush().Return(fakeErr)
mockPersistManager.EXPECT().StartFlushPersist().Return(mockFlushPersist, nil)

testOpts := DefaultTestOptions().SetPersistManager(mockPersistManager)
db := newMockdatabase(ctrl)
db.EXPECT().Options().Return(testOpts).AnyTimes()
db.EXPECT().OwnedNamespaces().Return(nil, nil)

cfm := newColdFlushManager(db, mockPersistManager, testOpts).(*coldFlushManager)
cfm.pm = mockPersistManager

require.EqualError(t, fakeErr, cfm.coldFlush().Error())
}
3 changes: 0 additions & 3 deletions src/dbnode/storage/flush_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,6 @@ func TestFlushManagerNamespaceFlushTimesErr(t *testing.T) {
ns.EXPECT().Options().Return(nsOpts).AnyTimes()
ns.EXPECT().ID().Return(defaultTestNs1ID).AnyTimes()
ns.EXPECT().NeedsFlush(gomock.Any(), gomock.Any()).Return(false, fakeErr).AnyTimes()
ns.EXPECT().ColdFlush(gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().Snapshot(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
db.EXPECT().OwnedNamespaces().Return([]databaseNamespace{ns}, nil)

Expand Down Expand Up @@ -322,7 +321,6 @@ func TestFlushManagerSkipNamespaceIndexingDisabled(t *testing.T) {
ns.EXPECT().ID().Return(defaultTestNs1ID).AnyTimes()
ns.EXPECT().NeedsFlush(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
ns.EXPECT().WarmFlush(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().ColdFlush(gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().Snapshot(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()

var (
Expand Down Expand Up @@ -366,7 +364,6 @@ func TestFlushManagerNamespaceIndexingEnabled(t *testing.T) {
ns.EXPECT().ID().Return(defaultTestNs1ID).AnyTimes()
ns.EXPECT().NeedsFlush(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
ns.EXPECT().WarmFlush(gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().ColdFlush(gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().Snapshot(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
ns.EXPECT().FlushIndex(gomock.Any()).Return(nil)

Expand Down
2 changes: 1 addition & 1 deletion src/dbnode/storage/mediator.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func newMediator(database database, commitlog commitlog.CommitLog, opts Options)
if err != nil {
return nil, err
}
cfm := newColdFlushManager(database, pm, commitlog, opts)
cfm := newColdFlushManager(database, pm, opts)
d.databaseColdFlushManager = cfm

d.databaseRepairer = newNoopDatabaseRepairer()
Expand Down

0 comments on commit 945ca7c

Please sign in to comment.