Addressed review comments

Signed-off-by: jojohappy <[email protected]>
thanos-io · Apr 13, 2019 · 460009a · 460009a
1 parent 14a3d33
commit 460009a
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 13 deletions.
diff --git a/go.sum b/go.sum
@@ -11,8 +11,6 @@ github.com/Azure/azure-storage-blob-go v0.0.0-20181022225951-5152f14ace1c h1:Y5u
 github.com/Azure/azure-storage-blob-go v0.0.0-20181022225951-5152f14ace1c/go.mod h1:oGfmITT1V6x//CswqY2gtAHND+xIP64/qL7a5QJix0Y=
 github.com/Azure/go-autorest v10.8.1+incompatible h1:u0jVQf+a6k6x8A+sT60l6EY9XZu+kHdnZVPAYqpVRo0=
 github.com/Azure/go-autorest v10.8.1+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24=
-github.com/NYTimes/gziphandler v1.0.1 h1:iLrQrdwjDd52kHDA5op2UBJFjmOb9g+7scBan4RN8F0=
-github.com/NYTimes/gziphandler v1.0.1/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=
 github.com/NYTimes/gziphandler v1.1.1/go.mod h1:n/CVRwUEOgIxrgPvAQhUUr9oeUtvrhMomdKFjzJNB0c=
 github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE=
@@ -272,6 +270,7 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
+github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 go.opencensus.io v0.18.0/go.mod h1:vKdFvxhtzZ9onBp9VKHK8z/sRpBMnKAsufL7wlDrCOA=
 go.opencensus.io v0.19.0 h1:+jrnNy8MR4GZXvwF9PEuSyHxA4NaTf6601oNRwCSXq0=

diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go
@@ -856,7 +856,7 @@ func chunksSize(chks []storepb.AggrChunk) (size int) {
 }
 
 // LabelNames implements the storepb.StoreServer interface.
-func (s *BucketStore) LabelNames(ctx context.Context, _ *storepb.LabelNamesRequest) (*storepb.LabelNamesResponse, error) {
+func (s *BucketStore) LabelNames(ctx context.Context, r *storepb.LabelNamesRequest) (*storepb.LabelNamesResponse, error) {
 	g, gctx := errgroup.WithContext(ctx)
 
 	s.mtx.RLock()
@@ -869,19 +869,24 @@ func (s *BucketStore) LabelNames(ctx context.Context, _ *storepb.LabelNamesReque
 		g.Go(func() error {
 			defer runutil.CloseWithLogOnErr(s.logger, indexr, "label names")
 
-			res := indexr.LabelNames()
-
+			res, err := indexr.LabelNames(gctx)
+			sort.Strings(res)
 			mtx.Lock()
 			sets = append(sets, res)
 			mtx.Unlock()
-
-			return nil
+			return err
 		})
 	}
 
 	s.mtx.RUnlock()
 
 	if err := g.Wait(); err != nil {
+		if !r.PartialResponseDisabled {
+			return &storepb.LabelNamesResponse{
+				Names:    strutil.MergeSlices(sets...),
+				Warnings: []string{err.Error()},
+			}, nil
+		}
 		return nil, status.Error(codes.Aborted, err.Error())
 	}
 	return &storepb.LabelNamesResponse{
@@ -1627,12 +1632,15 @@ func (r *bucketIndexReader) LabelValues(name string) []string {
 }
 
 // LabelNames returns a list of label names.
-func (r *bucketIndexReader) LabelNames() []string {
+func (r *bucketIndexReader) LabelNames(ctx context.Context) ([]string, error) {
 	res := make([]string, 0, len(r.block.lvals))
 	for ln, _ := range r.block.lvals {
+		if ctx.Err() != nil {
+			return res, ctx.Err()
+		}
 		res = append(res, ln)
 	}
-	return res
+	return res, nil
 }
 
 // Close released the underlying resources of the reader.

diff --git a/pkg/store/prometheus.go b/pkg/store/prometheus.go
@@ -379,12 +379,12 @@ func (p *PrometheusStore) LabelNames(ctx context.Context, r *storepb.LabelNamesR
 
 	if m.Status != "success" {
 		if !r.PartialResponseDisabled {
-			return &storepb.LabelNamesResponse{Names: m.Data}, nil
+			return &storepb.LabelNamesResponse{Names: m.Data, Warnings: []string{m.Error}}, nil
 		}
 
 		code, exists := statusToCode[resp.StatusCode]
 		if !exists {
-			code = codes.Internal
+			return nil, status.Error(codes.Internal, m.Error)
 		}
 		return nil, status.Error(code, m.Error)
 	}
@@ -435,12 +435,12 @@ func (p *PrometheusStore) LabelValues(ctx context.Context, r *storepb.LabelValue
 
 	if m.Status != "success" {
 		if !r.PartialResponseDisabled {
-			return &storepb.LabelValuesResponse{Values: m.Data}, nil
+			return &storepb.LabelValuesResponse{Values: m.Data, Warnings: []string{m.Error}}, nil
 		}
 
 		code, exists := statusToCode[resp.StatusCode]
 		if !exists {
-			code = codes.Internal
+			return nil, status.Error(codes.Internal, m.Error)
 		}
 		return nil, status.Error(code, m.Error)
 	}