Skip to content

Commit

Permalink
Add regression test for and fix deadlock. (#2169)
Browse files Browse the repository at this point in the history
  • Loading branch information
notbdu authored Feb 26, 2020
1 parent 4154ff1 commit 05064b4
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
34 changes: 34 additions & 0 deletions src/dbnode/integration/dynamic_namespace_add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,17 @@
package integration

import (
"sync"
"testing"
"time"

"github.com/m3db/m3/src/cluster/integration/etcd"
"github.com/m3db/m3/src/dbnode/integration/generate"
"github.com/m3db/m3/src/dbnode/namespace"
"github.com/m3db/m3/src/dbnode/storage/block"
xtime "github.com/m3db/m3/src/x/time"

"github.com/golang/mock/gomock"
"github.com/golang/protobuf/proto"
"github.com/stretchr/testify/require"
)
Expand All @@ -39,6 +42,8 @@ func TestDynamicNamespaceAdd(t *testing.T) {
if testing.Short() {
t.SkipNow() // Just skip if we're doing a short run
}
ctrl := gomock.NewController(t)
defer ctrl.Finish()

// test options
testOpts := newTestOptions(t).
Expand Down Expand Up @@ -86,11 +91,40 @@ func TestDynamicNamespaceAdd(t *testing.T) {
require.NoError(t, testSetup.startServer())

// Stop the server
stopped := false
defer func() {
stopped = true
require.NoError(t, testSetup.stopServer())
log.Info("server is now down")
}()

// NB(bodu): concurrently do work on the leaseManager to ensure no race cond deadlock regressions by
// calling OpenLease or OpenLatestLease (blocked on DB lock).
var wg sync.WaitGroup
leaser := block.NewMockLeaser(ctrl)
leaseState := block.LeaseState{}
for i := 0; i < 100; i++ {
leaseDescriptor := block.LeaseDescriptor{
Namespace: ns0.ID(),
Shard: uint32(0),
BlockStart: time.Now().Truncate(ns0.Options().RetentionOptions().BlockSize()),
}
wg.Add(2)
go func() {
wg.Done()
for !stopped {
testSetup.blockLeaseManager.OpenLease(leaser, leaseDescriptor, leaseState)
}
}()
go func() {
wg.Done()
for !stopped {
testSetup.blockLeaseManager.OpenLatestLease(leaser, leaseDescriptor)
}
}()
}
wg.Wait()

// Write test data
blockSize := ns0.Options().RetentionOptions().BlockSize()
now := testSetup.getNowFn()
Expand Down
12 changes: 10 additions & 2 deletions src/dbnode/storage/block/lease.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,21 @@ func (m *leaseManager) OpenLease(
) error {
// NB(r): Take exclusive lock so that upgrade leases can't be called
// while we are verifying a lease (racey)
// NB(bodu): We don't use defer here since the lease verifier takes out a
// a db lock when retrieving flush states resulting in a potential deadlock.
m.Lock()
defer m.Unlock()

if m.verifier == nil {
m.Unlock()
return errOpenLeaseVerifierNotSet
}

if !m.isRegistered(leaser) {
m.Unlock()
return errLeaserNotRegistered
}

m.Unlock()
return m.verifier.VerifyLease(descriptor, state)
}

Expand All @@ -108,17 +112,21 @@ func (m *leaseManager) OpenLatestLease(
) (LeaseState, error) {
// NB(r): Take exclusive lock so that upgrade leases can't be called
// while we are verifying a lease (racey)
// NB(bodu): We don't use defer here since the lease verifier takes out a
// a db lock when retrieving flush states resulting in a potential deadlock.
m.Lock()
defer m.Unlock()

if m.verifier == nil {
m.Unlock()
return LeaseState{}, errOpenLeaseVerifierNotSet
}

if !m.isRegistered(leaser) {
m.Unlock()
return LeaseState{}, errLeaserNotRegistered
}

m.Unlock()
return m.verifier.LatestState(descriptor)
}

Expand Down

0 comments on commit 05064b4

Please sign in to comment.