Skip to content

Commit 5d55862

Browse files
congqixiacongqixia
authored
Add revive sub-lints and fix existing problems (milvus-io#27495)
Signed-off-by: Congqi Xia <congqi.xia@zilliz.com>
1 parent 2607357 commit 5d55862

File tree

14 files changed

+123
-109
lines changed

14 files changed

+123
-109
lines changed

‎.golangci.yml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,34 @@ linters-settings:
5353
disabled: false
5454
arguments:
5555
- ["ID"] # Allow list
56+
- name: context-as-argument
57+
severity: warning
58+
disabled: false
59+
arguments:
60+
- allowTypesBefore: "*testing.T"
61+
- name: datarace
62+
severity: warning
63+
disabled: false
64+
- name: duplicated-imports
65+
severity: warning
66+
disabled: false
67+
- name: waitgroup-by-value
68+
severity: warning
69+
disabled: false
70+
- name: indent-error-flow
71+
severity: warning
72+
disabled: false
73+
arguments:
74+
- "preserveScope"
75+
- name: range-val-in-closure
76+
severity: warning
77+
disabled: false
78+
- name: range-val-address
79+
severity: warning
80+
disabled: false
81+
- name: string-of-int
82+
severity: warning
83+
disabled: false
5684
misspell:
5785
locale: US
5886
gocritic:

‎internal/datacoord/index_meta_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import (
2828
"github.com/stretchr/testify/mock"
2929

3030
"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
31-
"github.com/milvus-io/milvus/internal/kv/mocks"
3231
mockkv "github.com/milvus-io/milvus/internal/kv/mocks"
3332
"github.com/milvus-io/milvus/internal/metastore/kv/datacoord"
3433
catalogmocks "github.com/milvus-io/milvus/internal/metastore/mocks"
@@ -731,7 +730,7 @@ func TestMeta_MarkIndexAsDeleted(t *testing.T) {
731730
}
732731

733732
func TestMeta_GetSegmentIndexes(t *testing.T) {
734-
m := createMetaTable(&datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)})
733+
m := createMetaTable(&datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)})
735734

736735
t.Run("success", func(t *testing.T) {
737736
segIndexes := m.GetSegmentIndexes(segID)

‎internal/datacoord/index_service_test.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ import (
2727

2828
"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
2929
"github.com/milvus-io/milvus-proto/go-api/v2/msgpb"
30-
"github.com/milvus-io/milvus/internal/kv/mocks"
3130
mockkv "github.com/milvus-io/milvus/internal/kv/mocks"
3231
"github.com/milvus-io/milvus/internal/metastore/kv/datacoord"
3332
catalogmocks "github.com/milvus-io/milvus/internal/metastore/mocks"
@@ -183,7 +182,7 @@ func TestServer_GetIndexState(t *testing.T) {
183182
)
184183
s := &Server{
185184
meta: &meta{
186-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
185+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
187186
},
188187
allocator: newMockAllocator(),
189188
notifyIndexChan: make(chan UniqueID, 1),
@@ -204,7 +203,7 @@ func TestServer_GetIndexState(t *testing.T) {
204203
})
205204

206205
s.meta = &meta{
207-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
206+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
208207
indexes: map[UniqueID]map[UniqueID]*model.Index{
209208
collID: {
210209
indexID: {
@@ -255,7 +254,7 @@ func TestServer_GetIndexState(t *testing.T) {
255254
})
256255

257256
s.meta = &meta{
258-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
257+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
259258
indexes: map[UniqueID]map[UniqueID]*model.Index{
260259
collID: {
261260
indexID: {
@@ -373,7 +372,7 @@ func TestServer_GetSegmentIndexState(t *testing.T) {
373372
)
374373
s := &Server{
375374
meta: &meta{
376-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
375+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
377376
indexes: map[UniqueID]map[UniqueID]*model.Index{},
378377
segments: &SegmentsInfo{map[UniqueID]*SegmentInfo{}},
379378
},
@@ -508,7 +507,7 @@ func TestServer_GetIndexBuildProgress(t *testing.T) {
508507

509508
s := &Server{
510509
meta: &meta{
511-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
510+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
512511
indexes: map[UniqueID]map[UniqueID]*model.Index{},
513512
segments: &SegmentsInfo{map[UniqueID]*SegmentInfo{}},
514513
},
@@ -1540,7 +1539,7 @@ func TestServer_GetIndexInfos(t *testing.T) {
15401539

15411540
s := &Server{
15421541
meta: &meta{
1543-
catalog: &datacoord.Catalog{MetaKv: mocks.NewMetaKv(t)},
1542+
catalog: &datacoord.Catalog{MetaKv: mockkv.NewMetaKv(t)},
15441543
indexes: map[UniqueID]map[UniqueID]*model.Index{
15451544
collID: {
15461545
// finished

‎internal/datanode/binlog_io.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,8 @@ func (b *binlogIO) download(ctx context.Context, paths []string) ([]*Blob, error
101101
for i := range futures {
102102
if !futures[i].OK() {
103103
return nil, futures[i].Err()
104-
} else {
105-
resp[i] = &Blob{Value: futures[i].Value().([]byte)}
106104
}
105+
resp[i] = &Blob{Value: futures[i].Value().([]byte)}
107106
}
108107

109108
return resp, nil

‎internal/kv/tikv/txn_tikv.go‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func tiTxnBegin(txn *txnkv.Client) (*transaction.KVTxn, error) {
7373
return txn.Begin()
7474
}
7575

76-
func tiTxnCommit(txn *transaction.KVTxn, ctx context.Context) error {
76+
func tiTxnCommit(ctx context.Context, txn *transaction.KVTxn) error {
7777
return txn.Commit(ctx)
7878
}
7979

@@ -143,10 +143,9 @@ func (kv *txnTiKV) Has(key string) (bool, error) {
143143
// Dont error out if not present unless failed call to tikv
144144
if common.IsKeyNotExistError(err) {
145145
return false, nil
146-
} else {
147-
loggingErr = errors.Wrap(err, fmt.Sprintf("Failed to read key: %s", key))
148-
return false, loggingErr
149146
}
147+
loggingErr = errors.Wrap(err, fmt.Sprintf("Failed to read key: %s", key))
148+
return false, loggingErr
150149
}
151150
CheckElapseAndWarn(start, "Slow txnTiKV Has() operation", zap.String("key", key))
152151
return true, nil
@@ -348,7 +347,7 @@ func (kv *txnTiKV) MultiSave(kvs map[string]string) error {
348347
return loggingErr
349348
}
350349
}
351-
err = kv.executeTxn(txn, ctx)
350+
err = kv.executeTxn(ctx, txn)
352351
if err != nil {
353352
loggingErr = errors.Wrap(err, "Failed to commit for MultiSave()")
354353
return loggingErr
@@ -397,7 +396,7 @@ func (kv *txnTiKV) MultiRemove(keys []string) error {
397396
}
398397
}
399398

400-
err = kv.executeTxn(txn, ctx)
399+
err = kv.executeTxn(ctx, txn)
401400
if err != nil {
402401
loggingErr = errors.Wrap(err, "Failed to commit for MultiRemove()")
403402
return loggingErr
@@ -481,7 +480,7 @@ func (kv *txnTiKV) MultiSaveAndRemove(saves map[string]string, removals []string
481480
}
482481
}
483482

484-
err = kv.executeTxn(txn, ctx)
483+
err = kv.executeTxn(ctx, txn)
485484
if err != nil {
486485
loggingErr = errors.Wrap(err, "Failed to commit for MultiSaveAndRemove")
487486
return loggingErr
@@ -567,7 +566,7 @@ func (kv *txnTiKV) MultiSaveAndRemoveWithPrefix(saves map[string]string, removal
567566
}
568567
}
569568
}
570-
err = kv.executeTxn(txn, ctx)
569+
err = kv.executeTxn(ctx, txn)
571570
if err != nil {
572571
loggingErr = errors.Wrap(err, "Failed to commit for MultiSaveAndRemoveWithPrefix")
573572
return loggingErr
@@ -620,12 +619,12 @@ func (kv *txnTiKV) WalkWithPrefix(prefix string, paginationSize int, fn func([]b
620619
return nil
621620
}
622621

623-
func (kv *txnTiKV) executeTxn(txn *transaction.KVTxn, ctx context.Context) error {
622+
func (kv *txnTiKV) executeTxn(ctx context.Context, txn *transaction.KVTxn) error {
624623
start := timerecord.NewTimeRecorder("executeTxn")
625624

626625
elapsed := start.ElapseSpan()
627626
metrics.MetaOpCounter.WithLabelValues(metrics.MetaTxnLabel, metrics.TotalLabel).Inc()
628-
err := commitTxn(txn, ctx)
627+
err := commitTxn(ctx, txn)
629628
if err == nil {
630629
metrics.MetaRequestLatency.WithLabelValues(metrics.MetaTxnLabel).Observe(float64(elapsed.Milliseconds()))
631630
metrics.MetaOpCounter.WithLabelValues(metrics.MetaTxnLabel, metrics.SuccessLabel).Inc()
@@ -651,10 +650,9 @@ func (kv *txnTiKV) getTiKVMeta(ctx context.Context, key string) (string, error)
651650
if err == tikverr.ErrNotExist {
652651
// If key is missing
653652
return "", common.NewKeyNotExistError(key)
654-
} else {
655-
// If call to tikv fails
656-
return "", errors.Wrap(err, fmt.Sprintf("Failed to get value for key %s in getTiKVMeta", key))
657653
}
654+
// If call to tikv fails
655+
return "", errors.Wrap(err, fmt.Sprintf("Failed to get value for key %s in getTiKVMeta", key))
658656
}
659657

660658
// Check if value is the empty placeholder
@@ -692,7 +690,7 @@ func (kv *txnTiKV) putTiKVMeta(ctx context.Context, key, val string) error {
692690
if err != nil {
693691
return errors.Wrap(err, fmt.Sprintf("Failed to set value for key %s in putTiKVMeta", key))
694692
}
695-
err = commitTxn(txn, ctx1)
693+
err = commitTxn(ctx1, txn)
696694

697695
elapsed := start.ElapseSpan()
698696
metrics.MetaOpCounter.WithLabelValues(metrics.MetaPutLabel, metrics.TotalLabel).Inc()
@@ -724,7 +722,7 @@ func (kv *txnTiKV) removeTiKVMeta(ctx context.Context, key string) error {
724722
if err != nil {
725723
return errors.Wrap(err, fmt.Sprintf("Failed to remove key %s in removeTiKVMeta", key))
726724
}
727-
err = commitTxn(txn, ctx1)
725+
err = commitTxn(ctx1, txn)
728726

729727
elapsed := start.ElapseSpan()
730728
metrics.MetaOpCounter.WithLabelValues(metrics.MetaRemoveLabel, metrics.TotalLabel).Inc()
@@ -765,9 +763,8 @@ func isEmptyByte(value []byte) bool {
765763
func convertEmptyByteToString(value []byte) string {
766764
if isEmptyByte(value) {
767765
return ""
768-
} else {
769-
return string(value)
770766
}
767+
return string(value)
771768
}
772769

773770
// Convert string into EmptyValue if empty else cast to []byte. Will throw error if value is equal

‎internal/kv/tikv/txn_tikv_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func TestTiKVLoad(te *testing.T) {
325325
kv := NewTiKV(txnClient, rootPath)
326326
defer kv.Close()
327327

328-
commitTxn = func(txn *transaction.KVTxn, ctx context.Context) error {
328+
commitTxn = func(ctx context.Context, txn *transaction.KVTxn) error {
329329
return fmt.Errorf("bad txn commit!")
330330
}
331331
defer func() {

‎internal/parser/planparserv2/plan_parser_v2.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,13 @@ func CreateSearchPlan(schemaPb *schemapb.CollectionSchema, exprStr string, vecto
137137
var vectorType planpb.VectorType
138138
if !typeutil.IsVectorType(dataType) {
139139
return nil, fmt.Errorf("field (%s) to search is not of vector data type", vectorFieldName)
140+
}
141+
if dataType == schemapb.DataType_FloatVector {
142+
vectorType = planpb.VectorType_FloatVector
143+
} else if dataType == schemapb.DataType_BinaryVector {
144+
vectorType = planpb.VectorType_BinaryVector
140145
} else {
141-
if dataType == schemapb.DataType_FloatVector {
142-
vectorType = planpb.VectorType_FloatVector
143-
} else if dataType == schemapb.DataType_BinaryVector {
144-
vectorType = planpb.VectorType_BinaryVector
145-
} else {
146-
vectorType = planpb.VectorType_Float16Vector
147-
}
146+
vectorType = planpb.VectorType_Float16Vector
148147
}
149148
planNode := &planpb.PlanNode{
150149
Node: &planpb.PlanNode_VectorAnns{

‎internal/proxy/impl_test.go‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -391,34 +391,34 @@ func TestProxy_FlushAll_DbCollection(t *testing.T) {
391391
globalMetaCache = cache
392392

393393
for _, test := range tests {
394-
factory := dependency.NewDefaultFactory(true)
395-
ctx := context.Background()
396-
paramtable.Init()
394+
t.Run(test.testName, func(t *testing.T) {
395+
factory := dependency.NewDefaultFactory(true)
396+
ctx := context.Background()
397+
paramtable.Init()
397398

398-
node, err := NewProxy(ctx, factory)
399-
assert.NoError(t, err)
400-
node.stateCode.Store(commonpb.StateCode_Healthy)
401-
node.tsoAllocator = &timestampAllocator{
402-
tso: newMockTimestampAllocatorInterface(),
403-
}
399+
node, err := NewProxy(ctx, factory)
400+
assert.NoError(t, err)
401+
node.stateCode.Store(commonpb.StateCode_Healthy)
402+
node.tsoAllocator = &timestampAllocator{
403+
tso: newMockTimestampAllocatorInterface(),
404+
}
404405

405-
Params.Save(Params.ProxyCfg.MaxTaskNum.Key, "1000")
406-
node.sched, err = newTaskScheduler(ctx, node.tsoAllocator, node.factory)
407-
assert.NoError(t, err)
408-
err = node.sched.Start()
409-
assert.NoError(t, err)
410-
defer node.sched.Close()
411-
node.dataCoord = mocks.NewMockDataCoordClient(t)
412-
node.rootCoord = mocks.NewMockRootCoordClient(t)
413-
successStatus := &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success}
414-
node.dataCoord.(*mocks.MockDataCoordClient).EXPECT().Flush(mock.Anything, mock.Anything).
415-
Return(&datapb.FlushResponse{Status: successStatus}, nil).Maybe()
416-
node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ShowCollections(mock.Anything, mock.Anything).
417-
Return(&milvuspb.ShowCollectionsResponse{Status: successStatus, CollectionNames: []string{"col-0"}}, nil).Maybe()
418-
node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ListDatabases(mock.Anything, mock.Anything).
419-
Return(&milvuspb.ListDatabasesResponse{Status: successStatus, DbNames: []string{"default"}}, nil).Maybe()
406+
Params.Save(Params.ProxyCfg.MaxTaskNum.Key, "1000")
407+
node.sched, err = newTaskScheduler(ctx, node.tsoAllocator, node.factory)
408+
assert.NoError(t, err)
409+
err = node.sched.Start()
410+
assert.NoError(t, err)
411+
defer node.sched.Close()
412+
node.dataCoord = mocks.NewMockDataCoordClient(t)
413+
node.rootCoord = mocks.NewMockRootCoordClient(t)
414+
successStatus := &commonpb.Status{ErrorCode: commonpb.ErrorCode_Success}
415+
node.dataCoord.(*mocks.MockDataCoordClient).EXPECT().Flush(mock.Anything, mock.Anything).
416+
Return(&datapb.FlushResponse{Status: successStatus}, nil).Maybe()
417+
node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ShowCollections(mock.Anything, mock.Anything).
418+
Return(&milvuspb.ShowCollectionsResponse{Status: successStatus, CollectionNames: []string{"col-0"}}, nil).Maybe()
419+
node.rootCoord.(*mocks.MockRootCoordClient).EXPECT().ListDatabases(mock.Anything, mock.Anything).
420+
Return(&milvuspb.ListDatabasesResponse{Status: successStatus, DbNames: []string{"default"}}, nil).Maybe()
420421

421-
t.Run(test.testName, func(t *testing.T) {
422422
resp, err := node.FlushAll(ctx, test.FlushRequest)
423423
assert.NoError(t, err)
424424
if test.ExpectedSuccess {

‎internal/proxy/plan_parser.go‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -884,14 +884,13 @@ func createQueryPlan(schemaPb *schemapb.CollectionSchema, exprStr string, vector
884884
var vectorType planpb.VectorType
885885
if !typeutil.IsVectorType(dataType) {
886886
return nil, fmt.Errorf("field (%s) to search is not of vector data type", vectorFieldName)
887+
}
888+
if dataType == schemapb.DataType_FloatVector {
889+
vectorType = planpb.VectorType_FloatVector
890+
} else if dataType == schemapb.DataType_BinaryVector {
891+
vectorType = planpb.VectorType_BinaryVector
887892
} else {
888-
if dataType == schemapb.DataType_FloatVector {
889-
vectorType = planpb.VectorType_FloatVector
890-
} else if dataType == schemapb.DataType_BinaryVector {
891-
vectorType = planpb.VectorType_BinaryVector
892-
} else {
893-
vectorType = planpb.VectorType_Float16Vector
894-
}
893+
vectorType = planpb.VectorType_Float16Vector
895894
}
896895

897896
planNode := &planpb.PlanNode{

0 commit comments

Comments
 (0)