Add revive sub-lints and fix existing problems (#27495)

Signed-off-by: Congqi Xia <[email protected]>

.golangci.yml

@@ -53,6 +53,34 @@ linters-settings:
         disabled: false
         arguments:
           - ["ID"] # Allow list
+      - name: context-as-argument
+        severity: warning
+        disabled: false
+        arguments:
+          - allowTypesBefore: "*testing.T"
+      - name: datarace
+        severity: warning
+        disabled: false
+      - name: duplicated-imports
+        severity: warning
+        disabled: false
+      - name: waitgroup-by-value
+        severity: warning
+        disabled: false
+      - name: indent-error-flow
+        severity: warning
+        disabled: false
+        arguments:
+          - "preserveScope"
+      - name: range-val-in-closure
+        severity: warning
+        disabled: false
+      - name: range-val-address
+        severity: warning
+        disabled: false
+      - name: string-of-int
+        severity: warning
+        disabled: false
   misspell:
     locale: US
   gocritic:

internal/datacoord/index_meta_test.go

@@ -28,7 +28,6 @@ import (
 	"github.com/stretchr/testify/mock"
 
 	"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
-	"github.com/milvus-io/milvus/internal/kv/mocks"
 	mockkv "github.com/milvus-io/milvus/internal/kv/mocks"
 	"github.com/milvus-io/milvus/internal/metastore/kv/datacoord"
 	catalogmocks "github.com/milvus-io/milvus/internal/metastore/mocks"
@@ -731,7 +730,7 @@ func TestMeta_MarkIndexAsDeleted(t *testing.T) {
 }
 
 func TestMeta_GetSegmentIndexes(t *testing.T) {
-	m := createMetaTable(&datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)})
+	m := createMetaTable(&datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)})
 
 	t.Run("success", func(t *testing.T) {
 		segIndexes := m.GetSegmentIndexes(segID)

internal/datacoord/index_service_test.go

@@ -27,7 +27,6 @@ import (
 
 	"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
 	"github.com/milvus-io/milvus-proto/go-api/v2/msgpb"
-	"github.com/milvus-io/milvus/internal/kv/mocks"
 	mockkv "github.com/milvus-io/milvus/internal/kv/mocks"
 	"github.com/milvus-io/milvus/internal/metastore/kv/datacoord"
 	catalogmocks "github.com/milvus-io/milvus/internal/metastore/mocks"
@@ -183,7 +182,7 @@ func TestServer_GetIndexState(t *testing.T) {
 	)
 	s := &Server{
 		meta: &meta{
-			catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+			catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 		},
 		allocator:       newMockAllocator(),
 		notifyIndexChan: make(chan UniqueID, 1),
@@ -204,7 +203,7 @@ func TestServer_GetIndexState(t *testing.T) {
 	})
 
 	s.meta = &meta{
-		catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+		catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 		indexes: map[UniqueID]map[UniqueID]*model.Index{
 			collID: {
 				indexID: {
@@ -255,7 +254,7 @@ func TestServer_GetIndexState(t *testing.T) {
 	})
 
 	s.meta = &meta{
-		catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+		catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 		indexes: map[UniqueID]map[UniqueID]*model.Index{
 			collID: {
 				indexID: {
@@ -373,7 +372,7 @@ func TestServer_GetSegmentIndexState(t *testing.T) {
 	)
 	s := &Server{
 		meta: &meta{
-			catalog:  &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+			catalog:  &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 			indexes:  map[UniqueID]map[UniqueID]*model.Index{},
 			segments: &SegmentsInfo{map[UniqueID]*SegmentInfo{}},
 		},
@@ -508,7 +507,7 @@ func TestServer_GetIndexBuildProgress(t *testing.T) {
 
 	s := &Server{
 		meta: &meta{
-			catalog:  &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+			catalog:  &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 			indexes:  map[UniqueID]map[UniqueID]*model.Index{},
 			segments: &SegmentsInfo{map[UniqueID]*SegmentInfo{}},
 		},
@@ -1540,7 +1539,7 @@ func TestServer_GetIndexInfos(t *testing.T) {
 
 	s := &Server{
 		meta: &meta{
-			catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
+			catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
 			indexes: map[UniqueID]map[UniqueID]*model.Index{
 				collID: {
 					// finished

internal/datanode/binlog_io.go

@@ -101,9 +101,8 @@ func (b *binlogIO) download(ctx context.Context, paths []string) ([]*Blob, error
 	for i := range futures {
 		if !futures[i].OK() {
 			return nil, futures[i].Err()
-		} else {
-			resp[i] = &Blob{Value: futures[i].Value().([]byte)}
 		}
+		resp[i] = &Blob{Value: futures[i].Value().([]byte)}
 	}
 
 	return resp, nil

internal/kv/tikv/txn_tikv.go

@@ -73,7 +73,7 @@ func tiTxnBegin(txn *txnkv.Client) (*transaction.KVTxn, error) {
 	return txn.Begin()
 }
 
-func tiTxnCommit(txn *transaction.KVTxn, ctx context.Context) error {
+func tiTxnCommit(ctx context.Context, txn *transaction.KVTxn) error {
 	return txn.Commit(ctx)
 }
 
@@ -143,10 +143,9 @@ func (kv *txnTiKV) Has(key string) (bool, error) {
 		// Dont error out if not present unless failed call to tikv
 		if common.IsKeyNotExistError(err) {
 			return false, nil
-		} else {
-			loggingErr = errors.Wrap(err, fmt.Sprintf("Failed to read key: %s", key))
-			return false, loggingErr
 		}
+		loggingErr = errors.Wrap(err, fmt.Sprintf("Failed to read key: %s", key))
+		return false, loggingErr
 	}
 	CheckElapseAndWarn(start, "Slow txnTiKV Has() operation", zap.String("key", key))
 	return true, nil
@@ -348,7 +347,7 @@ func (kv *txnTiKV) MultiSave(kvs map[string]string) error {
 			return loggingErr
 		}
 	}
-	err = kv.executeTxn(txn, ctx)
+	err = kv.executeTxn(ctx, txn)
 	if err != nil {
 		loggingErr = errors.Wrap(err, "Failed to commit for MultiSave()")
 		return loggingErr
@@ -397,7 +396,7 @@ func (kv *txnTiKV) MultiRemove(keys []string) error {
 		}
 	}
 
-	err = kv.executeTxn(txn, ctx)
+	err = kv.executeTxn(ctx, txn)
 	if err != nil {
 		loggingErr = errors.Wrap(err, "Failed to commit for MultiRemove()")
 		return loggingErr
@@ -481,7 +480,7 @@ func (kv *txnTiKV) MultiSaveAndRemove(saves map[string]string, removals []string
 		}
 	}
 
-	err = kv.executeTxn(txn, ctx)
+	err = kv.executeTxn(ctx, txn)
 	if err != nil {
 		loggingErr = errors.Wrap(err, "Failed to commit for MultiSaveAndRemove")
 		return loggingErr
@@ -567,7 +566,7 @@ func (kv *txnTiKV) MultiSaveAndRemoveWithPrefix(saves map[string]string, removal
 			}
 		}
 	}
-	err = kv.executeTxn(txn, ctx)
+	err = kv.executeTxn(ctx, txn)
 	if err != nil {
 		loggingErr = errors.Wrap(err, "Failed to commit for MultiSaveAndRemoveWithPrefix")
 		return loggingErr
@@ -620,12 +619,12 @@ func (kv *txnTiKV) WalkWithPrefix(prefix string, paginationSize int, fn func([]b
 	return nil
 }
 
-func (kv *txnTiKV) executeTxn(txn *transaction.KVTxn, ctx context.Context) error {
+func (kv *txnTiKV) executeTxn(ctx context.Context, txn *transaction.KVTxn) error {
 	start := timerecord.NewTimeRecorder("executeTxn")
 
 	elapsed := start.ElapseSpan()
 	metrics.MetaOpCounter.WithLabelValues(metrics.MetaTxnLabel, metrics.TotalLabel).Inc()
-	err := commitTxn(txn, ctx)
+	err := commitTxn(ctx, txn)
 	if err == nil {
 		metrics.MetaRequestLatency.WithLabelValues(metrics.MetaTxnLabel).Observe(float64(elapsed.Milliseconds()))
 		metrics.MetaOpCounter.WithLabelValues(metrics.MetaTxnLabel, metrics.SuccessLabel).Inc()
@@ -651,10 +650,9 @@ func (kv *txnTiKV) getTiKVMeta(ctx context.Context, key string) (string, error)
 		if err == tikverr.ErrNotExist {
 			// If key is missing
 			return "", common.NewKeyNotExistError(key)
-		} else {
-			// If call to tikv fails
-			return "", errors.Wrap(err, fmt.Sprintf("Failed to get value for key %s in getTiKVMeta", key))
 		}
+		// If call to tikv fails
+		return "", errors.Wrap(err, fmt.Sprintf("Failed to get value for key %s in getTiKVMeta", key))
 	}
 
 	// Check if value is the empty placeholder
@@ -692,7 +690,7 @@ func (kv *txnTiKV) putTiKVMeta(ctx context.Context, key, val string) error {
 	if err != nil {
 		return errors.Wrap(err, fmt.Sprintf("Failed to set value for key %s in putTiKVMeta", key))
 	}
-	err = commitTxn(txn, ctx1)
+	err = commitTxn(ctx1, txn)
 
 	elapsed := start.ElapseSpan()
 	metrics.MetaOpCounter.WithLabelValues(metrics.MetaPutLabel, metrics.TotalLabel).Inc()
@@ -724,7 +722,7 @@ func (kv *txnTiKV) removeTiKVMeta(ctx context.Context, key string) error {
 	if err != nil {
 		return errors.Wrap(err, fmt.Sprintf("Failed to remove key %s in removeTiKVMeta", key))
 	}
-	err = commitTxn(txn, ctx1)
+	err = commitTxn(ctx1, txn)
 
 	elapsed := start.ElapseSpan()
 	metrics.MetaOpCounter.WithLabelValues(metrics.MetaRemoveLabel, metrics.TotalLabel).Inc()
@@ -765,9 +763,8 @@ func isEmptyByte(value []byte) bool {
 func convertEmptyByteToString(value []byte) string {
 	if isEmptyByte(value) {
 		return ""
-	} else {
-		return string(value)
 	}
+	return string(value)
 }
 
 // Convert string into EmptyValue if empty else cast to []byte. Will throw error if value is equal

internal/kv/tikv/txn_tikv_test.go

@@ -325,7 +325,7 @@ func TestTiKVLoad(te *testing.T) {
 		kv := NewTiKV(txnClient, rootPath)
 		defer kv.Close()
 
-		commitTxn = func(txn *transaction.KVTxn, ctx context.Context) error {
+		commitTxn = func(ctx context.Context, txn *transaction.KVTxn) error {
 			return fmt.Errorf("bad txn commit!")
 		}
 		defer func() {

internal/parser/planparserv2/plan_parser_v2.go

@@ -137,14 +137,13 @@ func CreateSearchPlan(schemaPb *schemapb.CollectionSchema, exprStr string, vecto
 	var vectorType planpb.VectorType
 	if !typeutil.IsVectorType(dataType) {
 		return nil, fmt.Errorf("field (%s) to search is not of vector data type", vectorFieldName)
+	}
+	if dataType == schemapb.DataType_FloatVector {
+		vectorType = planpb.VectorType_FloatVector
+	} else if dataType == schemapb.DataType_BinaryVector {
+		vectorType = planpb.VectorType_BinaryVector
 	} else {
-		if dataType == schemapb.DataType_FloatVector {
-			vectorType = planpb.VectorType_FloatVector
-		} else if dataType == schemapb.DataType_BinaryVector {
-			vectorType = planpb.VectorType_BinaryVector
-		} else {
-			vectorType = planpb.VectorType_Float16Vector
-		}
+		vectorType = planpb.VectorType_Float16Vector
 	}
 	planNode := &planpb.PlanNode{
 		Node: &planpb.PlanNode_VectorAnns{

internal/proxy/impl_test.go

@@ -391,34 +391,34 @@ func TestProxy_FlushAll_DbCollection(t *testing.T) {
 	globalMetaCache = cache
 
 	for _, test := range tests {
-		factory := dependency.NewDefaultFactory(true)
-		ctx := context.Background()
-		paramtable.Init()
+		t.Run(test.testName, func(t *testing.T) {
+			factory := dependency.NewDefaultFactory(true)
+			ctx := context.Background()
+			paramtable.Init()
 
-		node, err := NewProxy(ctx, factory)
-		assert.NoError(t, err)
-		node.stateCode.Store(commonpb.StateCode_Healthy)
-		node.tsoAllocator = &timestampAllocator{
-			tso: newMockTimestampAllocatorInterface(),
-		}
+			node, err := NewProxy(ctx, factory)
+			assert.NoError(t, err)
+			node.stateCode.Store(commonpb.StateCode_Healthy)
+			node.tsoAllocator = &timestampAllocator{
+				tso: newMockTimestampAllocatorInterface(),
+			}
 
-		Params.Save(Params.ProxyCfg.MaxTaskNum.Key, "1000")
-		node.sched, err = newTaskScheduler(ctx, node.tsoAllocator, node.factory)
-		assert.NoError(t, err)
-		err = node.sched.Start()
-		assert.NoError(t, err)
-		defer node.sched.Close()
-		node.dataCoord = mocks.NewMockDataCoordClient(t)
-		node.rootCoord = mocks.NewMockRootCoordClient(t)
-		successStatus := &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success}
-		node.dataCoord.(*mocks.MockDataCoordClient).EXPECT().Flush(mock.Anything, mock.Anything).
-			Return(&datapb.FlushResponse{Status: successStatus}, nil).Maybe()
-		node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ShowCollections(mock.Anything, mock.Anything).
-			Return(&milvuspb.ShowCollectionsResponse{Status: successStatus, CollectionNames: []string{"col-0"}}, nil).Maybe()
-		node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ListDatabases(mock.Anything, mock.Anything).
-			Return(&milvuspb.ListDatabasesResponse{Status: successStatus, DbNames: []string{"default"}}, nil).Maybe()
+			Params.Save(Params.ProxyCfg.MaxTaskNum.Key, "1000")
+			node.sched, err = newTaskScheduler(ctx, node.tsoAllocator, node.factory)
+			assert.NoError(t, err)
+			err = node.sched.Start()
+			assert.NoError(t, err)
+			defer node.sched.Close()
+			node.dataCoord = mocks.NewMockDataCoordClient(t)
+			node.rootCoord = mocks.NewMockRootCoordClient(t)
+			successStatus := &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success}
+			node.dataCoord.(*mocks.MockDataCoordClient).EXPECT().Flush(mock.Anything, mock.Anything).
+				Return(&datapb.FlushResponse{Status: successStatus}, nil).Maybe()
+			node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ShowCollections(mock.Anything, mock.Anything).
+				Return(&milvuspb.ShowCollectionsResponse{Status: successStatus, CollectionNames: []string{"col-0"}}, nil).Maybe()
+			node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ListDatabases(mock.Anything, mock.Anything).
+				Return(&milvuspb.ListDatabasesResponse{Status: successStatus, DbNames: []string{"default"}}, nil).Maybe()
 
-		t.Run(test.testName, func(t *testing.T) {
 			resp, err := node.FlushAll(ctx, test.FlushRequest)
 			assert.NoError(t, err)
 			if test.ExpectedSuccess {

internal/proxy/plan_parser.go

@@ -884,14 +884,13 @@ func createQueryPlan(schemaPb *schemapb.CollectionSchema, exprStr string, vector
 	var vectorType planpb.VectorType
 	if !typeutil.IsVectorType(dataType) {
 		return nil, fmt.Errorf("field (%s) to search is not of vector data type", vectorFieldName)
+	}
+	if dataType == schemapb.DataType_FloatVector {
+		vectorType = planpb.VectorType_FloatVector
+	} else if dataType == schemapb.DataType_BinaryVector {
+		vectorType = planpb.VectorType_BinaryVector
 	} else {
-		if dataType == schemapb.DataType_FloatVector {
-			vectorType = planpb.VectorType_FloatVector
-		} else if dataType == schemapb.DataType_BinaryVector {
-			vectorType = planpb.VectorType_BinaryVector
-		} else {
-			vectorType = planpb.VectorType_Float16Vector
-		}
+		vectorType = planpb.VectorType_Float16Vector
 	}
 
 	planNode := &planpb.PlanNode{