diff --git a/src/dbnode/integration/dynamic_namespace_add_test.go b/src/dbnode/integration/dynamic_namespace_add_test.go index 44889dd8bb..bf5f0be2ba 100644 --- a/src/dbnode/integration/dynamic_namespace_add_test.go +++ b/src/dbnode/integration/dynamic_namespace_add_test.go @@ -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" ) @@ -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). @@ -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() diff --git a/src/dbnode/storage/block/lease.go b/src/dbnode/storage/block/lease.go index c37ae8088a..0f684f28c8 100644 --- a/src/dbnode/storage/block/lease.go +++ b/src/dbnode/storage/block/lease.go @@ -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) } @@ -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) }