Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxy Store should not lock when all clients error. #563

Merged
merged 3 commits into from
Oct 10, 2018
Merged

Proxy Store should not lock when all clients error. #563

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
01ed9bf
respCh should no longer cause us to lock
domgreen Oct 10, 2018
4652312
adding back
domgreen Oct 10, 2018
bea48c0
update changelog
domgreen Oct 10, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pkg/store/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,13 @@ func (s *ProxyStore) Series(r *storepb.SeriesRequest, srv storepb.Store_SeriesSe
return nil
}

stores, err := s.stores(srv.Context())
var (
respCh = make(chan *storepb.SeriesResponse, 10)
seriesSet []storepb.SeriesSet
respCh = make(chan *storepb.SeriesResponse, len(stores) + 1)
g errgroup.Group
)

stores, err := s.stores(srv.Context())
if err != nil {
level.Error(s.logger).Log("err", err)
return status.Errorf(codes.Unknown, err.Error())
Expand Down
44 changes: 42 additions & 2 deletions pkg/store/proxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,45 @@ func TestQueryStore_Series_SameExtSet(t *testing.T) {
testutil.Equals(t, 0, len(s1.Warnings))
}

func TestQueryStore_Series_FillResponseChannel(t *testing.T) {
defer leaktest.CheckTimeout(t, 10*time.Second)()

var cls []Client
for i := 0; i < 10; i++ {
cls = append(cls, &testClient{
StoreClient: &storeClient{
RespSet: []*storepb.SeriesResponse{
storeSeriesResponse(t, labels.FromStrings("a", "b"), []sample{{1, 1}, {2, 2}, {3, 3}}),
},
RespError: errors.New("test error"),
},
minTime: 1,
maxTime: 300,
})
}

q := NewProxyStore(nil,
func(context.Context) ([]Client, error) { return cls, nil },
tlabels.FromStrings("fed", "a"),
)

ctx := context.Background()
s1 := newStoreSeriesServer(ctx)

// This should return empty response, since there is external label mismatch.
err := q.Series(
&storepb.SeriesRequest{
MinTime: 1,
MaxTime: 300,
Matchers: []storepb.LabelMatcher{{Name: "fed", Value: "a", Type: storepb.LabelMatcher_EQ}},
}, s1,
)
testutil.Ok(t, err)
testutil.Equals(t, 0, len(s1.SeriesSet))
testutil.Equals(t, 0, len(s1.Warnings))
}


type rawSeries struct {
lset []storepb.Label
samples []sample
Expand Down Expand Up @@ -342,15 +381,16 @@ func (s *storeSeriesServer) Context() context.Context {
type storeClient struct {
Values map[string][]string

RespSet []*storepb.SeriesResponse
RespSet []*storepb.SeriesResponse
RespError error
}

func (s *storeClient) Info(ctx context.Context, req *storepb.InfoRequest, _ ...grpc.CallOption) (*storepb.InfoResponse, error) {
return nil, status.Error(codes.Unimplemented, "not implemented")
}

func (s *storeClient) Series(ctx context.Context, req *storepb.SeriesRequest, _ ...grpc.CallOption) (storepb.Store_SeriesClient, error) {
return &StoreSeriesClient{ctx: ctx, respSet: s.RespSet}, nil
return &StoreSeriesClient{ctx: ctx, respSet: s.RespSet}, s.RespError
}

func (s *storeClient) LabelNames(ctx context.Context, req *storepb.LabelNamesRequest, _ ...grpc.CallOption) (*storepb.LabelNamesResponse, error) {
Expand Down