Skip to content

Commit

Permalink
fix: Make GetShardLeaders skip retry on not loaded collection (#37684)
Browse files Browse the repository at this point in the history
issue: #37532

Signed-off-by: Wei Liu <[email protected]>
  • Loading branch information
weiliu1031 authored Nov 14, 2024
1 parent 1b4f7e3 commit c5485bb
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 2 deletions.
3 changes: 1 addition & 2 deletions internal/proxy/lb_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,8 @@ func (lb *LBPolicyImpl) GetShardLeaders(ctx context.Context, dbName string, coll
var err error
shardLeaders, err = globalMetaCache.GetShards(ctx, withCache, dbName, collName, collectionID)
if err != nil {
return !errors.Is(err, merr.ErrCollectionLoaded), err
return !errors.Is(err, merr.ErrCollectionNotLoaded), err
}

return false, nil
})

Expand Down
39 changes: 39 additions & 0 deletions internal/proxy/lb_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@ import (
"testing"

"github.com/cockroachdb/errors"
"github.com/pingcap/log"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/suite"
"go.uber.org/atomic"
"go.uber.org/zap"
"google.golang.org/grpc"
"google.golang.org/protobuf/proto"

"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
Expand Down Expand Up @@ -456,6 +459,42 @@ func (s *LBPolicySuite) TestNewLBPolicy() {
policy.Close()
}

func (s *LBPolicySuite) TestGetShardLeaders() {
ctx := context.Background()

// ErrCollectionNotFullyLoaded is retriable, expected to retry until ctx done or success
counter := atomic.NewInt64(0)
globalMetaCache.DeprecateShardCache(dbName, s.collectionName)
s.qc.ExpectedCalls = nil
s.qc.EXPECT().GetShardLeaders(mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, req *querypb.GetShardLeadersRequest, opts ...grpc.CallOption) (*querypb.GetShardLeadersResponse, error) {
counter.Inc()
return nil, merr.ErrCollectionNotFullyLoaded
}).Times(5)
s.qc.EXPECT().GetShardLeaders(mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, req *querypb.GetShardLeadersRequest, opts ...grpc.CallOption) (*querypb.GetShardLeadersResponse, error) {
log.Info("return rpc success")
return nil, nil
}).Times(5)
_, err := s.lbPolicy.GetShardLeaders(ctx, dbName, s.collectionName, s.collectionID, true)
s.NoError(err)
s.Equal(int64(5), counter.Load())

// ErrServiceUnavailable is not retriable, expected to fail fast
counter.Store(0)
globalMetaCache.DeprecateShardCache(dbName, s.collectionName)
s.qc.ExpectedCalls = nil
s.qc.EXPECT().GetShardLeaders(mock.Anything, mock.Anything).RunAndReturn(
func(ctx context.Context, req *querypb.GetShardLeadersRequest, opts ...grpc.CallOption) (*querypb.GetShardLeadersResponse, error) {
counter.Inc()
return nil, merr.ErrCollectionNotLoaded
})
_, err = s.lbPolicy.GetShardLeaders(ctx, dbName, s.collectionName, s.collectionID, true)
log.Info("check err", zap.Error(err))
s.Error(err)
s.Equal(int64(1), counter.Load())
}

func TestLBPolicySuite(t *testing.T) {
suite.Run(t, new(LBPolicySuite))
}

0 comments on commit c5485bb

Please sign in to comment.