fix: Speed up multi-object delete by taking bulk locks (#8974)

Change distributed locking to allow taking bulk locks across objects, reduces usually 1000 calls to 1. Also allows for situations where multiple clients sends delete requests to objects with following names ``` {1,2,3,4,5} ``` ``` {5,4,3,2,1} ``` will block and ensure that we do not fail the request on each other.
2025-11-09 21:49:46 -05:00 · 2020-02-21 11:29:57 +05:30
parent 852fb320f7
commit ab7d3cd508
24 changed files with 305 additions and 580 deletions
--- a/pkg/dsync/drwmutex.go
+++ b/pkg/dsync/drwmutex.go
@@ -19,13 +19,10 @@ package dsync
 import (
 	"context"
 	"errors"
-	"fmt"
 	golog "log"
 	"math"
 	"math/rand"
 	"os"
-	"path"
-	"runtime"
 	"sync"
 	"time"
 )
@@ -51,7 +48,7 @@ const drwMutexInfinite = time.Duration(1<<63 - 1)

 // A DRWMutex is a distributed mutual exclusion lock.
 type DRWMutex struct {
-	Name         string
+	Names        []string
 	writeLocks   []string   // Array of nodes that granted a write lock
 	readersLocks [][]string // Array of array of nodes that granted reader locks
 	m            sync.Mutex // Mutex to prevent multiple simultaneous locks from this node
@@ -74,10 +71,10 @@ func isLocked(uid string) bool {
 }

 // NewDRWMutex - initializes a new dsync RW mutex.
-func NewDRWMutex(ctx context.Context, name string, clnt *Dsync) *DRWMutex {
+func NewDRWMutex(ctx context.Context, clnt *Dsync, names ...string) *DRWMutex {
 	return &DRWMutex{
-		Name:       name,
 		writeLocks: make([]string, len(clnt.GetLockersFn())),
+		Names:      names,
 		clnt:       clnt,
 		ctx:        ctx,
 	}
@@ -149,7 +146,7 @@ func (dm *DRWMutex) lockBlocking(timeout time.Duration, id, source string, isRea
 		locks := make([]string, len(restClnts))

 		// Try to acquire the lock.
-		success := lock(dm.clnt, &locks, dm.Name, id, source, isReadLock)
+		success := lock(dm.clnt, &locks, id, source, isReadLock, dm.Names...)
 		if success {
 			dm.m.Lock()

@@ -176,7 +173,7 @@ func (dm *DRWMutex) lockBlocking(timeout time.Duration, id, source string, isRea
 }

 // lock tries to acquire the distributed lock, returning true or false.
-func lock(ds *Dsync, locks *[]string, lockName, id, source string, isReadLock bool) bool {
+func lock(ds *Dsync, locks *[]string, id, source string, isReadLock bool, lockNames ...string) bool {

 	restClnts := ds.GetLockersFn()

@@ -199,9 +196,9 @@ func lock(ds *Dsync, locks *[]string, lockName, id, source string, isReadLock bo
 			}

 			args := LockArgs{
-				UID:      id,
-				Resource: lockName,
-				Source:   source,
+				UID:       id,
+				Resources: lockNames,
+				Source:    source,
 			}

 			var locked bool
@@ -259,7 +256,7 @@ func lock(ds *Dsync, locks *[]string, lockName, id, source string, isReadLock bo
 						done = true
 						// Increment the number of grants received from the buffered channel.
 						i++
-						releaseAll(ds, locks, lockName, isReadLock, restClnts)
+						releaseAll(ds, locks, isReadLock, restClnts, lockNames...)
 					}
 				}
 			case <-timeout:
@@ -267,7 +264,7 @@ func lock(ds *Dsync, locks *[]string, lockName, id, source string, isReadLock bo
 				// timeout happened, maybe one of the nodes is slow, count
 				// number of locks to check whether we have quorum or not
 				if !quorumMet(locks, isReadLock, dquorum, dquorumReads) {
-					releaseAll(ds, locks, lockName, isReadLock, restClnts)
+					releaseAll(ds, locks, isReadLock, restClnts, lockNames...)
 				}
 			}

@@ -289,8 +286,8 @@ func lock(ds *Dsync, locks *[]string, lockName, id, source string, isReadLock bo
 			grantToBeReleased := <-ch
 			if grantToBeReleased.isLocked() {
 				// release lock
-				sendRelease(ds, restClnts[grantToBeReleased.index], lockName,
-					grantToBeReleased.lockUID, isReadLock)
+				sendRelease(ds, restClnts[grantToBeReleased.index],
+					grantToBeReleased.lockUID, isReadLock, lockNames...)
 			}
 		}
 	}(isReadLock)
@@ -321,10 +318,10 @@ func quorumMet(locks *[]string, isReadLock bool, quorum, quorumReads int) bool {
 }

 // releaseAll releases all locks that are marked as locked
-func releaseAll(ds *Dsync, locks *[]string, lockName string, isReadLock bool, restClnts []NetLocker) {
-	for lock := 0; lock < len(restClnts); lock++ {
+func releaseAll(ds *Dsync, locks *[]string, isReadLock bool, restClnts []NetLocker, lockNames ...string) {
+	for lock := range restClnts {
 		if isLocked((*locks)[lock]) {
-			sendRelease(ds, restClnts[lock], lockName, (*locks)[lock], isReadLock)
+			sendRelease(ds, restClnts[lock], (*locks)[lock], isReadLock, lockNames...)
 			(*locks)[lock] = ""
 		}
 	}
@@ -362,7 +359,7 @@ func (dm *DRWMutex) Unlock() {
 	}

 	isReadLock := false
-	unlock(dm.clnt, locks, dm.Name, isReadLock, restClnts)
+	unlock(dm.clnt, locks, isReadLock, restClnts, dm.Names...)
 }

 // RUnlock releases a read lock held on dm.
@@ -387,10 +384,10 @@ func (dm *DRWMutex) RUnlock() {
 	}

 	isReadLock := true
-	unlock(dm.clnt, locks, dm.Name, isReadLock, restClnts)
+	unlock(dm.clnt, locks, isReadLock, restClnts, dm.Names...)
 }

-func unlock(ds *Dsync, locks []string, name string, isReadLock bool, restClnts []NetLocker) {
+func unlock(ds *Dsync, locks []string, isReadLock bool, restClnts []NetLocker, names ...string) {

 	// We don't need to synchronously wait until we have released all the locks (or the quorum)
 	// (a subsequent lock will retry automatically in case it would fail to get quorum)
@@ -399,21 +396,21 @@ func unlock(ds *Dsync, locks []string, name string, isReadLock bool, restClnts [

 		if isLocked(locks[index]) {
 			// broadcast lock release to all nodes that granted the lock
-			sendRelease(ds, c, name, locks[index], isReadLock)
+			sendRelease(ds, c, locks[index], isReadLock, names...)
 		}
 	}
 }

 // sendRelease sends a release message to a node that previously granted a lock
-func sendRelease(ds *Dsync, c NetLocker, name, uid string, isReadLock bool) {
+func sendRelease(ds *Dsync, c NetLocker, uid string, isReadLock bool, names ...string) {
 	if c == nil {
 		log("Unable to call RUnlock", errors.New("netLocker is offline"))
 		return
 	}

 	args := LockArgs{
-		UID:      uid,
-		Resource: name,
+		UID:       uid,
+		Resources: names,
 	}
 	if isReadLock {
 		if _, err := c.RUnlock(args); err != nil {
@@ -425,38 +422,3 @@ func sendRelease(ds *Dsync, c NetLocker, name, uid string, isReadLock bool) {
 		}
 	}
 }
-
-// DRLocker returns a sync.Locker interface that implements
-// the Lock and Unlock methods by calling drw.RLock and drw.RUnlock.
-func (dm *DRWMutex) DRLocker() sync.Locker {
-	return (*drlocker)(dm)
-}
-
-type drlocker DRWMutex
-
-var letterRunes = []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ")
-
-func randString(n int) string {
-	b := make([]rune, n)
-	for i := range b {
-		b[i] = letterRunes[rand.Intn(len(letterRunes))]
-	}
-	return string(b)
-}
-
-func getSource() string {
-	var funcName string
-	pc, filename, lineNum, ok := runtime.Caller(2)
-	if ok {
-		filename = path.Base(filename)
-		funcName = runtime.FuncForPC(pc).Name()
-	} else {
-		filename = "<unknown>"
-		lineNum = 0
-	}
-
-	return fmt.Sprintf("[%s:%d:%s()]", filename, lineNum, funcName)
-}
-
-func (dr *drlocker) Lock()   { (*DRWMutex)(dr).RLock(randString(16), getSource()) }
-func (dr *drlocker) Unlock() { (*DRWMutex)(dr).RUnlock() }
--- a/pkg/dsync/drwmutex_test.go
+++ b/pkg/dsync/drwmutex_test.go
@@ -22,7 +22,6 @@ import (
 	"context"
 	"fmt"
 	"runtime"
-	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -37,7 +36,7 @@ const (

 func testSimpleWriteLock(t *testing.T, duration time.Duration) (locked bool) {

-	drwm := NewDRWMutex(context.Background(), "simplelock", ds)
+	drwm := NewDRWMutex(context.Background(), ds, "simplelock")

 	if !drwm.GetRLock(id, source, time.Second) {
 		panic("Failed to acquire read lock")
@@ -93,7 +92,7 @@ func TestSimpleWriteLockTimedOut(t *testing.T) {

 func testDualWriteLock(t *testing.T, duration time.Duration) (locked bool) {

-	drwm := NewDRWMutex(context.Background(), "duallock", ds)
+	drwm := NewDRWMutex(context.Background(), ds, "duallock")

 	// fmt.Println("Getting initial write lock")
 	if !drwm.GetLock(id, source, time.Second) {
@@ -153,7 +152,7 @@ func parallelReader(m *DRWMutex, clocked, cunlock, cdone chan bool) {
 // Borrowed from rwmutex_test.go
 func doTestParallelReaders(numReaders, gomaxprocs int) {
 	runtime.GOMAXPROCS(gomaxprocs)
-	m := NewDRWMutex(context.Background(), "test-parallel", ds)
+	m := NewDRWMutex(context.Background(), ds, "test-parallel")

 	clocked := make(chan bool)
 	cunlock := make(chan bool)
@@ -221,7 +220,7 @@ func HammerRWMutex(gomaxprocs, numReaders, numIterations int) {
 	runtime.GOMAXPROCS(gomaxprocs)
 	// Number of active readers + 10000 * number of active writers.
 	var activity int32
-	rwm := NewDRWMutex(context.Background(), "test", ds)
+	rwm := NewDRWMutex(context.Background(), ds, "test")
 	cdone := make(chan bool)
 	go writer(rwm, numIterations, &activity, cdone)
 	var i int
@@ -257,42 +256,6 @@ func TestRWMutex(t *testing.T) {
 	HammerRWMutex(10, 5, n)
 }

-// Borrowed from rwmutex_test.go
-func TestDRLocker(t *testing.T) {
-	wl := NewDRWMutex(context.Background(), "test", ds)
-	var rl sync.Locker
-	wlocked := make(chan bool, 1)
-	rlocked := make(chan bool, 1)
-	rl = wl.DRLocker()
-	n := 10
-	go func() {
-		for i := 0; i < n; i++ {
-			rl.Lock()
-			rl.Lock()
-			rlocked <- true
-			wl.Lock(id, source)
-			wlocked <- true
-		}
-	}()
-	for i := 0; i < n; i++ {
-		<-rlocked
-		rl.Unlock()
-		select {
-		case <-wlocked:
-			t.Fatal("RLocker() didn't read-lock it")
-		default:
-		}
-		rl.Unlock()
-		<-wlocked
-		select {
-		case <-rlocked:
-			t.Fatal("RLocker() didn't respect the write lock")
-		default:
-		}
-		wl.Unlock()
-	}
-}
-
 // Borrowed from rwmutex_test.go
 func TestUnlockPanic(t *testing.T) {
 	defer func() {
@@ -300,7 +263,7 @@ func TestUnlockPanic(t *testing.T) {
 			t.Fatalf("unlock of unlocked RWMutex did not panic")
 		}
 	}()
-	mu := NewDRWMutex(context.Background(), "test", ds)
+	mu := NewDRWMutex(context.Background(), ds, "test")
 	mu.Unlock()
 }

@@ -311,7 +274,7 @@ func TestUnlockPanic2(t *testing.T) {
 			t.Fatalf("unlock of unlocked RWMutex did not panic")
 		}
 	}()
-	mu := NewDRWMutex(context.Background(), "test-unlock-panic-2", ds)
+	mu := NewDRWMutex(context.Background(), ds, "test-unlock-panic-2")
 	mu.RLock(id, source)
 	mu.Unlock()
 }
@@ -323,7 +286,7 @@ func TestRUnlockPanic(t *testing.T) {
 			t.Fatalf("read unlock of unlocked RWMutex did not panic")
 		}
 	}()
-	mu := NewDRWMutex(context.Background(), "test", ds)
+	mu := NewDRWMutex(context.Background(), ds, "test")
 	mu.RUnlock()
 }

@@ -334,14 +297,14 @@ func TestRUnlockPanic2(t *testing.T) {
 			t.Fatalf("read unlock of unlocked RWMutex did not panic")
 		}
 	}()
-	mu := NewDRWMutex(context.Background(), "test-runlock-panic-2", ds)
+	mu := NewDRWMutex(context.Background(), ds, "test-runlock-panic-2")
 	mu.Lock(id, source)
 	mu.RUnlock()
 }

 // Borrowed from rwmutex_test.go
 func benchmarkRWMutex(b *testing.B, localWork, writeRatio int) {
-	rwm := NewDRWMutex(context.Background(), "test", ds)
+	rwm := NewDRWMutex(context.Background(), ds, "test")
 	b.RunParallel(func(pb *testing.PB) {
 		foo := 0
 		for pb.Next() {
--- a/pkg/dsync/dsync-server_test.go
+++ b/pkg/dsync/dsync-server_test.go
@@ -35,8 +35,8 @@ type lockServer struct {
 func (l *lockServer) Lock(args *LockArgs, reply *bool) error {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
-	if _, *reply = l.lockMap[args.Resource]; !*reply {
-		l.lockMap[args.Resource] = WriteLock // No locks held on the given name, so claim write lock
+	if _, *reply = l.lockMap[args.Resources[0]]; !*reply {
+		l.lockMap[args.Resources[0]] = WriteLock // No locks held on the given name, so claim write lock
 	}
 	*reply = !*reply // Negate *reply to return true when lock is granted or false otherwise
 	return nil
@@ -46,13 +46,13 @@ func (l *lockServer) Unlock(args *LockArgs, reply *bool) error {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	var locksHeld int64
-	if locksHeld, *reply = l.lockMap[args.Resource]; !*reply { // No lock is held on the given name
-		return fmt.Errorf("Unlock attempted on an unlocked entity: %s", args.Resource)
+	if locksHeld, *reply = l.lockMap[args.Resources[0]]; !*reply { // No lock is held on the given name
+		return fmt.Errorf("Unlock attempted on an unlocked entity: %s", args.Resources[0])
 	}
 	if *reply = locksHeld == WriteLock; !*reply { // Unless it is a write lock
-		return fmt.Errorf("Unlock attempted on a read locked entity: %s (%d read locks active)", args.Resource, locksHeld)
+		return fmt.Errorf("Unlock attempted on a read locked entity: %s (%d read locks active)", args.Resources[0], locksHeld)
 	}
-	delete(l.lockMap, args.Resource) // Remove the write lock
+	delete(l.lockMap, args.Resources[0]) // Remove the write lock
 	return nil
 }

@@ -62,12 +62,12 @@ func (l *lockServer) RLock(args *LockArgs, reply *bool) error {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	var locksHeld int64
-	if locksHeld, *reply = l.lockMap[args.Resource]; !*reply {
-		l.lockMap[args.Resource] = ReadLock // No locks held on the given name, so claim (first) read lock
+	if locksHeld, *reply = l.lockMap[args.Resources[0]]; !*reply {
+		l.lockMap[args.Resources[0]] = ReadLock // No locks held on the given name, so claim (first) read lock
 		*reply = true
 	} else {
 		if *reply = locksHeld != WriteLock; *reply { // Unless there is a write lock
-			l.lockMap[args.Resource] = locksHeld + ReadLock // Grant another read lock
+			l.lockMap[args.Resources[0]] = locksHeld + ReadLock // Grant another read lock
 		}
 	}
 	return nil
@@ -77,16 +77,16 @@ func (l *lockServer) RUnlock(args *LockArgs, reply *bool) error {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	var locksHeld int64
-	if locksHeld, *reply = l.lockMap[args.Resource]; !*reply { // No lock is held on the given name
-		return fmt.Errorf("RUnlock attempted on an unlocked entity: %s", args.Resource)
+	if locksHeld, *reply = l.lockMap[args.Resources[0]]; !*reply { // No lock is held on the given name
+		return fmt.Errorf("RUnlock attempted on an unlocked entity: %s", args.Resources[0])
 	}
 	if *reply = locksHeld != WriteLock; !*reply { // A write-lock is held, cannot release a read lock
-		return fmt.Errorf("RUnlock attempted on a write locked entity: %s", args.Resource)
+		return fmt.Errorf("RUnlock attempted on a write locked entity: %s", args.Resources[0])
 	}
 	if locksHeld > ReadLock {
-		l.lockMap[args.Resource] = locksHeld - ReadLock // Remove one of the read locks held
+		l.lockMap[args.Resources[0]] = locksHeld - ReadLock // Remove one of the read locks held
 	} else {
-		delete(l.lockMap, args.Resource) // Remove the (last) read lock
+		delete(l.lockMap, args.Resources[0]) // Remove the (last) read lock
 	}
 	return nil
 }
@@ -97,7 +97,7 @@ func (l *lockServer) ForceUnlock(args *LockArgs, reply *bool) error {
 	if len(args.UID) != 0 {
 		return fmt.Errorf("ForceUnlock called with non-empty UID: %s", args.UID)
 	}
-	delete(l.lockMap, args.Resource) // Remove the lock (irrespective of write or read lock)
+	delete(l.lockMap, args.Resources[0]) // Remove the lock (irrespective of write or read lock)
 	*reply = true
 	return nil
 }
--- a/pkg/dsync/dsync_test.go
+++ b/pkg/dsync/dsync_test.go
@@ -89,7 +89,7 @@ func TestMain(m *testing.M) {

 func TestSimpleLock(t *testing.T) {

-	dm := NewDRWMutex(context.Background(), "test", ds)
+	dm := NewDRWMutex(context.Background(), ds, "test")

 	dm.Lock(id, source)

@@ -101,7 +101,7 @@ func TestSimpleLock(t *testing.T) {

 func TestSimpleLockUnlockMultipleTimes(t *testing.T) {

-	dm := NewDRWMutex(context.Background(), "test", ds)
+	dm := NewDRWMutex(context.Background(), ds, "test")

 	dm.Lock(id, source)
 	time.Sleep(time.Duration(10+(rand.Float32()*50)) * time.Millisecond)
@@ -127,8 +127,8 @@ func TestSimpleLockUnlockMultipleTimes(t *testing.T) {
 // Test two locks for same resource, one succeeds, one fails (after timeout)
 func TestTwoSimultaneousLocksForSameResource(t *testing.T) {

-	dm1st := NewDRWMutex(context.Background(), "aap", ds)
-	dm2nd := NewDRWMutex(context.Background(), "aap", ds)
+	dm1st := NewDRWMutex(context.Background(), ds, "aap")
+	dm2nd := NewDRWMutex(context.Background(), ds, "aap")

 	dm1st.Lock(id, source)

@@ -151,9 +151,9 @@ func TestTwoSimultaneousLocksForSameResource(t *testing.T) {
 // Test three locks for same resource, one succeeds, one fails (after timeout)
 func TestThreeSimultaneousLocksForSameResource(t *testing.T) {

-	dm1st := NewDRWMutex(context.Background(), "aap", ds)
-	dm2nd := NewDRWMutex(context.Background(), "aap", ds)
-	dm3rd := NewDRWMutex(context.Background(), "aap", ds)
+	dm1st := NewDRWMutex(context.Background(), ds, "aap")
+	dm2nd := NewDRWMutex(context.Background(), ds, "aap")
+	dm3rd := NewDRWMutex(context.Background(), ds, "aap")

 	dm1st.Lock(id, source)

@@ -216,8 +216,8 @@ func TestThreeSimultaneousLocksForSameResource(t *testing.T) {
 // Test two locks for different resources, both succeed
 func TestTwoSimultaneousLocksForDifferentResources(t *testing.T) {

-	dm1 := NewDRWMutex(context.Background(), "aap", ds)
-	dm2 := NewDRWMutex(context.Background(), "noot", ds)
+	dm1 := NewDRWMutex(context.Background(), ds, "aap")
+	dm2 := NewDRWMutex(context.Background(), ds, "noot")

 	dm1.Lock(id, source)
 	dm2.Lock(id, source)
@@ -243,7 +243,7 @@ func HammerMutex(m *DRWMutex, loops int, cdone chan bool) {
 // Borrowed from mutex_test.go
 func TestMutex(t *testing.T) {
 	c := make(chan bool)
-	m := NewDRWMutex(context.Background(), "test", ds)
+	m := NewDRWMutex(context.Background(), ds, "test")
 	for i := 0; i < 10; i++ {
 		go HammerMutex(m, 1000, c)
 	}
@@ -257,7 +257,7 @@ func BenchmarkMutexUncontended(b *testing.B) {
 		*DRWMutex
 	}
 	b.RunParallel(func(pb *testing.PB) {
-		var mu = PaddedMutex{NewDRWMutex(context.Background(), "", ds)}
+		var mu = PaddedMutex{NewDRWMutex(context.Background(), ds, "")}
 		for pb.Next() {
 			mu.Lock(id, source)
 			mu.Unlock()
@@ -266,7 +266,7 @@ func BenchmarkMutexUncontended(b *testing.B) {
 }

 func benchmarkMutex(b *testing.B, slack, work bool) {
-	mu := NewDRWMutex(context.Background(), "", ds)
+	mu := NewDRWMutex(context.Background(), ds, "")
 	if slack {
 		b.SetParallelism(10)
 	}
@@ -309,7 +309,7 @@ func BenchmarkMutexNoSpin(b *testing.B) {
 	// These goroutines yield during local work, so that switching from
 	// a blocked goroutine to other goroutines is profitable.
 	// As a matter of fact, this benchmark still triggers some spinning in the mutex.
-	m := NewDRWMutex(context.Background(), "", ds)
+	m := NewDRWMutex(context.Background(), ds, "")
 	var acc0, acc1 uint64
 	b.SetParallelism(4)
 	b.RunParallel(func(pb *testing.PB) {
@@ -341,7 +341,7 @@ func BenchmarkMutexSpin(b *testing.B) {
 	// profitable. To achieve this we create a goroutine per-proc.
 	// These goroutines access considerable amount of local data so that
 	// unnecessary rescheduling is penalized by cache misses.
-	m := NewDRWMutex(context.Background(), "", ds)
+	m := NewDRWMutex(context.Background(), ds, "")
 	var acc0, acc1 uint64
 	b.RunParallel(func(pb *testing.PB) {
 		var data [16 << 10]uint64
--- a/pkg/dsync/rpc-client-interface.go
+++ b/pkg/dsync/rpc-client-interface.go
@@ -21,8 +21,8 @@ type LockArgs struct {
 	// Unique ID of lock/unlock request.
 	UID string

-	// Resource contains a entity to be locked/unlocked.
-	Resource string
+	// Resources contains single or multiple entries to be locked/unlocked.
+	Resources []string

 	// Source contains the line number, function and file name of the code
 	// on the client node that requested the lock.