Address review comments

Signed-off-by: jojohappy <sarahdj0917@gmail.com>
thanos-io · Oct 9, 2019 · de9a47f · de9a47f
1 parent d27bb87
commit de9a47f
Show file tree

Hide file tree

Showing 6 changed files with 39 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,9 +21,9 @@ We use *breaking* word for marking changes that are not backward compatible (rel
 - [#1533](https://github.com/thanos-io/thanos/pull/1533) Thanos inspect now supports the timeout flag.
 - [#1362](https://github.com/thanos-io/thanos/pull/1362) Optional `replicaLabels` param for `/query` and `/query_range` querier endpoints. When provided overwrite the `query.replica-label` cli flags.
 - [#1583](https://github.com/thanos-io/thanos/pull/1583) Thanos sharding:
-  - Add relabel config (`--selector.relabel-config-file` and `selector.relabel-config`) into Thanos
+  - Add relabel config (`--selector.relabel-config-file` and `selector.relabel-config`) into Thanos Store and Compact components.
+  - For store gateway, advertise labels from "approved" blocks.
   - Selecting blocks to serve depends on the result of block labels relabeling.
-  - For store gateway, expose advertise labels after relabeling.
 
 ### Changed
 

diff --git a/cmd/thanos/flags.go b/cmd/thanos/flags.go
@@ -77,7 +77,7 @@ func regSelectorRelabelFlags(cmd *kingpin.CmdClause) *extflag.PathOrContent {
 	return extflag.RegisterPathOrContent(
 		cmd,
 		"selector.relabel-config",
-		"YAML file that contains seletor relabeling configuration. It follows native Prometheus relabel-config syntax. See format details: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config ",
+		"YAML file that contains relabeling configuration that allows selecting blocks. It follows native Prometheus relabel-config syntax. See format details: https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config ",
 		false,
 	)
 }
diff --git a/docs/components/compact.md b/docs/components/compact.md
@@ -121,17 +121,17 @@ Flags:
       --compact.concurrency=1  Number of goroutines to use when compacting
                                groups.
       --selector.relabel-config-file=<file-path>
-                               Path to YAML file that contains seletor
-                               relabeling configuration. It follows native
-                               Prometheus relabel-config syntax. See format
-                               details:
+                               Path to YAML file that contains relabeling
+                               configuration that allows selecting blocks. It
+                               follows native Prometheus relabel-config syntax.
+                               See format details:
                                https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config
       --selector.relabel-config=<content>
                                Alternative to 'selector.relabel-config-file'
                                flag (lower priority). Content of YAML file that
-                               contains seletor relabeling configuration. It
-                               follows native Prometheus relabel-config syntax.
-                               See format details:
+                               contains relabeling configuration that allows
+                               selecting blocks. It follows native Prometheus
+                               relabel-config syntax. See format details:
                                https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config
 
 ```
diff --git a/docs/components/store.md b/docs/components/store.md
@@ -106,17 +106,18 @@ Flags:
                                  current time, such as -1d or 2h45m. Valid
                                  duration units are ms, s, m, h, d, w, y.
       --selector.relabel-config-file=<file-path>
-                                 Path to YAML file that contains seletor
-                                 relabeling configuration. It follows native
-                                 Prometheus relabel-config syntax. See format
-                                 details:
+                                 Path to YAML file that contains relabeling
+                                 configuration that allows selecting blocks. It
+                                 follows native Prometheus relabel-config
+                                 syntax. See format details:
                                  https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config
       --selector.relabel-config=<content>
                                  Alternative to 'selector.relabel-config-file'
                                  flag (lower priority). Content of YAML file
-                                 that contains seletor relabeling configuration.
-                                 It follows native Prometheus relabel-config
-                                 syntax. See format details:
+                                 that contains relabeling configuration that
+                                 allows selecting blocks. It follows native
+                                 Prometheus relabel-config syntax. See format
+                                 details:
                                  https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config
 
 ```

diff --git a/pkg/query/storeset.go b/pkg/query/storeset.go
@@ -218,10 +218,7 @@ func (s *StoreSet) Update(ctx context.Context) {
 	// Record the number of occurrences of external label combinations for current store slice.
 	externalLabelOccurrencesInStores := map[string]int{}
 	for _, st := range healthyStores {
-		if st.storeType != nil && (st.storeType.ToProto() == storepb.StoreType_QUERY ||
-			st.storeType.ToProto() == storepb.StoreType_SIDECAR) {
-			externalLabelOccurrencesInStores[externalLabelsFromStore(st)]++
-		}
+		externalLabelOccurrencesInStores[externalLabelsFromStore(st)]++
 	}
 	level.Debug(s.logger).Log("msg", "updating healthy stores", "externalLabelOccurrencesInStores", fmt.Sprintf("%#+v", externalLabelOccurrencesInStores))
 
@@ -257,8 +254,7 @@ func (s *StoreSet) Update(ctx context.Context) {
 		externalLabels := externalLabelsFromStore(store)
 		if len(store.LabelSets()) > 0 &&
 			store.storeType != nil &&
-			(store.storeType.ToProto() == storepb.StoreType_QUERY ||
-				store.storeType.ToProto() == storepb.StoreType_SIDECAR) &&
+			store.storeType.ToProto() != storepb.StoreType_STORE &&
 			externalLabelOccurrencesInStores[externalLabels] != 1 {
 			store.close()
 			s.updateStoreStatus(store, errors.New(droppingStoreMessage))

diff --git a/pkg/store/bucket.go b/pkg/store/bucket.go
@@ -221,6 +221,8 @@ type BucketStore struct {
 
 	filterConfig  *FilterConfig
 	relabelConfig []*relabel.Config
+
+	labelSets map[uint64]labels.Labels
 }
 
 // NewBucketStore creates a new bucket backed store that implements the store API against
@@ -367,6 +369,14 @@ func (s *BucketStore) SyncBlocks(ctx context.Context) error {
 		s.metrics.blockDrops.Inc()
 	}
 
+	// Sync advertise labels.
+	s.mtx.Lock()
+	s.labelSets = make(map[uint64]labels.Labels, len(s.blocks))
+	for _, bs := range s.blocks {
+		s.labelSets[bs.labels.Hash()] = append(labels.Labels(nil), bs.labels...)
+	}
+	s.mtx.Unlock()
+
 	return nil
 }
 
@@ -465,8 +475,7 @@ func (s *BucketStore) addBlock(ctx context.Context, id ulid.ULID) (err error) {
 
 	// Check for block labels by relabeling.
 	// If output is empty, the block will be dropped.
-	processedLabels := relabel.Process(promlabels.FromMap(lset.Map()), s.relabelConfig...)
-	if processedLabels == nil {
+	if processedLabels := relabel.Process(promlabels.FromMap(lset.Map()), s.relabelConfig...); processedLabels == nil {
 		level.Debug(s.logger).Log("msg", "dropping block(drop in relabeling)", "block", id)
 		return os.RemoveAll(dir)
 	}
@@ -542,24 +551,16 @@ func (s *BucketStore) Info(context.Context, *storepb.InfoRequest) (*storepb.Info
 		MaxTime:   maxt,
 	}
 
-	labelSets := make(map[uint64][]storepb.Label, len(s.blocks))
-	for _, bs := range s.blocks {
-		ls := map[string]string{}
-		for _, l := range bs.labels {
-			ls[l.Name] = l.Value
-		}
-
-		res := []storepb.Label{}
-		for k, v := range ls {
-			res = append(res, storepb.Label{Name: k, Value: v})
+	s.mtx.RLock()
+	res.LabelSets = make([]storepb.LabelSet, 0, len(s.labelSets))
+	for _, ls := range s.labelSets {
+		lset := []storepb.Label{}
+		for _, l := range ls {
+			lset = append(lset, storepb.Label{Name: l.Name, Value: l.Value})
 		}
-		labelSets[bs.labels.Hash()] = res
-	}
-
-	res.LabelSets = make([]storepb.LabelSet, 0, len(labelSets))
-	for _, v := range labelSets {
-		res.LabelSets = append(res.LabelSets, storepb.LabelSet{Labels: v})
+		res.LabelSets = append(res.LabelSets, storepb.LabelSet{Labels: lset})
 	}
+	s.mtx.RUnlock()
 
 	return res, nil
 }