Skip to content

Commit

Permalink
use IsBootstrappedAndDurable() instead of IsBootstrapped() otherw…
Browse files Browse the repository at this point in the history
…ise warm and cold flushes might fail because some shards might still be not bootstrapped.
  • Loading branch information
soundvibe committed Mar 30, 2021
1 parent 9eacaa7 commit 4fe5430
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 10 deletions.
4 changes: 2 additions & 2 deletions src/dbnode/storage/coldflush.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (m *coldFlushManager) Run(t time.Time) bool {
// and not caught in CI or integration tests.
// When an invariant occurs in CI tests it panics so as to fail
// the build.
if err := m.ColdFlushCleanup(t, m.database.IsBootstrapped()); err != nil {
if err := m.ColdFlushCleanup(t, m.database.IsBootstrappedAndDurable()); err != nil {
instrument.EmitAndLogInvariantViolation(m.opts.InstrumentOptions(),
func(l *zap.Logger) {
l.Error("error when cleaning up cold flush data", zap.Time("time", t), zap.Error(err))
Expand Down Expand Up @@ -195,7 +195,7 @@ func (m *coldFlushManager) Report() {
}

func (m *coldFlushManager) shouldRunWithLock() bool {
return m.enabled && m.status != fileOpInProgress && m.database.IsBootstrapped()
return m.enabled && m.status != fileOpInProgress && m.database.IsBootstrappedAndDurable()
}

type coldFlushNsOpts struct {
Expand Down
2 changes: 1 addition & 1 deletion src/dbnode/storage/coldflush_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestColdFlushManagerFlushAlreadyInProgress(t *testing.T) {
testOpts := DefaultTestOptions().SetPersistManager(mockPersistManager)
db := newMockdatabase(ctrl)
db.EXPECT().Options().Return(testOpts).AnyTimes()
db.EXPECT().IsBootstrapped().Return(true).AnyTimes()
db.EXPECT().IsBootstrappedAndDurable().Return(true).AnyTimes()
db.EXPECT().OwnedNamespaces().Return(nil, nil).AnyTimes()

cfm := newColdFlushManager(db, mockPersistManager, testOpts).(*coldFlushManager)
Expand Down
4 changes: 2 additions & 2 deletions src/dbnode/storage/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (m *fileSystemManager) Run(
// and not caught in CI or integration tests.
// When an invariant occurs in CI tests it panics so as to fail
// the build.
if err := m.WarmFlushCleanup(t, m.database.IsBootstrapped()); err != nil {
if err := m.WarmFlushCleanup(t, m.database.IsBootstrappedAndDurable()); err != nil {
instrument.EmitAndLogInvariantViolation(m.opts.InstrumentOptions(),
func(l *zap.Logger) {
l.Error("error when cleaning up data", zap.Time("time", t), zap.Error(err))
Expand Down Expand Up @@ -189,5 +189,5 @@ func (m *fileSystemManager) Report() {
}

func (m *fileSystemManager) shouldRunWithLock() bool {
return m.enabled && m.status != fileOpInProgress && m.database.IsBootstrapped()
return m.enabled && m.status != fileOpInProgress && m.database.IsBootstrappedAndDurable()
}
10 changes: 5 additions & 5 deletions src/dbnode/storage/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ func TestFileSystemManagerShouldRunDuringBootstrap(t *testing.T) {
fsm := newFileSystemManager(database, nil, DefaultTestOptions())
mgr := fsm.(*fileSystemManager)

database.EXPECT().IsBootstrapped().Return(false)
database.EXPECT().IsBootstrappedAndDurable().Return(false)
require.False(t, mgr.shouldRunWithLock())

database.EXPECT().IsBootstrapped().Return(true)
database.EXPECT().IsBootstrappedAndDurable().Return(true)
require.True(t, mgr.shouldRunWithLock())
}

Expand All @@ -52,7 +52,7 @@ func TestFileSystemManagerShouldRunWhileRunning(t *testing.T) {
database := newMockdatabase(ctrl)
fsm := newFileSystemManager(database, nil, DefaultTestOptions())
mgr := fsm.(*fileSystemManager)
database.EXPECT().IsBootstrapped().Return(true)
database.EXPECT().IsBootstrappedAndDurable().Return(true)
require.True(t, mgr.shouldRunWithLock())
mgr.status = fileOpInProgress
require.False(t, mgr.shouldRunWithLock())
Expand All @@ -64,7 +64,7 @@ func TestFileSystemManagerShouldRunEnableDisable(t *testing.T) {
database := newMockdatabase(ctrl)
fsm := newFileSystemManager(database, nil, DefaultTestOptions())
mgr := fsm.(*fileSystemManager)
database.EXPECT().IsBootstrapped().Return(true).AnyTimes()
database.EXPECT().IsBootstrappedAndDurable().Return(true).AnyTimes()
require.True(t, mgr.shouldRunWithLock())
require.NotEqual(t, fileOpInProgress, mgr.Disable())
require.False(t, mgr.shouldRunWithLock())
Expand All @@ -76,7 +76,7 @@ func TestFileSystemManagerRun(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
database := newMockdatabase(ctrl)
database.EXPECT().IsBootstrapped().Return(true).AnyTimes()
database.EXPECT().IsBootstrappedAndDurable().Return(true).AnyTimes()

fm := NewMockdatabaseFlushManager(ctrl)
cm := NewMockdatabaseCleanupManager(ctrl)
Expand Down

0 comments on commit 4fe5430

Please sign in to comment.