enhance: Log error instead of panicking if load lock wait timeout

Related to milvus-io#39205 Previous PR milvus-io#39206 This PR change wait timeout behavior to log error and return to avoid making other collection read failure in only some collections have deadlock Signed-off-by: Congqi Xia <[email protected]>
congqixia · Jan 15, 2025 · 0ce7ae0 · 0ce7ae0
1 parent 76773bf
commit 0ce7ae0
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 5 deletions.
diff --git a/internal/querynodev2/segments/segment.go b/internal/querynodev2/segments/segment.go
@@ -1156,11 +1156,19 @@ func (s *LocalSegment) WarmupChunkCache(ctx context.Context, fieldID int64, mmap
 			return nil, nil
 		}).Await()
 	case "async":
+<<<<<<< HEAD
 		GetWarmupPool().Submit(func() (any, error) {
 			// bad implemtation, warmup is async at another goroutine and hold the rlock.
 			// the state transition of segment in segment loader will blocked.
 			// add a waiter to avoid it.
 			s.ptrLock.BlockUntilDataLoadedOrReleased()
+=======
+		task := func() (any, error) {
+			// failed to wait for state update, return directly
+			if !s.ptrLock.BlockUntilDataLoadedOrReleased() {
+				return nil, nil
+			}
+>>>>>>> 3408d2f261 (enhance: Log error instead of panicking if load lock wait timeout)
 			if s.PinIfNotReleased() != nil {
 				return nil, nil
 			}

diff --git a/internal/querynodev2/segments/state/load_state_lock.go b/internal/querynodev2/segments/state/load_state_lock.go
@@ -7,7 +7,9 @@ import (
 
 	"github.com/cockroachdb/errors"
 	"go.uber.org/atomic"
+	"go.uber.org/zap"
 
+	"github.com/milvus-io/milvus/pkg/log"
 	"github.com/milvus-io/milvus/pkg/util/paramtable"
 )
 
@@ -168,10 +170,12 @@ func (ls *LoadStateLock) StartReleaseAll() (g LoadStateLockGuard) {
 }
 
 // blockUntilDataLoadedOrReleased blocks until the segment is loaded or released.
-func (ls *LoadStateLock) BlockUntilDataLoadedOrReleased() {
+func (ls *LoadStateLock) BlockUntilDataLoadedOrReleased() bool {
+	var ok bool
 	ls.waitOrPanic(func(state loadStateEnum) bool {
 		return state == LoadStateDataLoaded || state == LoadStateReleased
-	}, noop)
+	}, func() { ok = true })
+	return ok
 }
 
 // waitUntilCanReleaseData waits until segment is release data able.
@@ -199,7 +203,7 @@ func (ls *LoadStateLock) waitOrPanic(ready func(state loadStateEnum) bool, then
 
 	select {
 	case <-time.After(maxWaitTime):
-		panic(fmt.Sprintf("max WLock wait time(%v) excceeded", maxWaitTime))
+		log.Error("load state lock wait timeout", zap.Duration("maxWaitTime", maxWaitTime))
 	case <-ch:
 	}
 }

diff --git a/internal/querynodev2/segments/state/load_state_lock_test.go b/internal/querynodev2/segments/state/load_state_lock_test.go
@@ -197,25 +197,30 @@ func TestWaitOrPanic(t *testing.T) {
 		defer paramtable.Get().Reset(paramtable.Get().CommonCfg.MaxWLockConditionalWaitTime.Key)
 
 		l := NewLoadStateLock(LoadStateDataLoaded)
+		executed := false
 
 		assert.NotPanics(t, func() {
 			l.waitOrPanic(func(state loadStateEnum) bool {
 				return state == LoadStateDataLoaded
-			}, noop)
+			}, func() { executed = true })
 		})
+
+		assert.True(t, executed)
 	})
 
 	t.Run("timeout_panic", func(t *testing.T) {
 		paramtable.Get().Save(paramtable.Get().CommonCfg.MaxWLockConditionalWaitTime.Key, "1")
 		defer paramtable.Get().Reset(paramtable.Get().CommonCfg.MaxWLockConditionalWaitTime.Key)
 
 		l := NewLoadStateLock(LoadStateOnlyMeta)
+		executed := false
 
-		assert.Panics(t, func() {
+		assert.NotPanics(t, func() {
 			l.waitOrPanic(func(state loadStateEnum) bool {
 				return state == LoadStateDataLoaded
 			}, noop)
 		})
+		assert.False(t, executed)
 	})
 }