From e09689693223a8d27dba247f5fc9cf5daa1ac9ff Mon Sep 17 00:00:00 2001
From: Julius Volz <julius@soundcloud.com>
Date: Tue, 26 Mar 2013 14:46:02 +0100
Subject: [PATCH] PR comment fixups.

---
 storage/metric/interface.go   |  2 ++
 storage/metric/leveldb.go     | 19 +++++++++++--------
 storage/metric/memory.go      |  9 ++++-----
 storage/metric/tiered.go      | 14 ++++++++------
 storage/metric/tiered_test.go |  2 ++
 5 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/storage/metric/interface.go b/storage/metric/interface.go
index a1823f7e2f..8188bac5ee 100644
--- a/storage/metric/interface.go
+++ b/storage/metric/interface.go
@@ -44,6 +44,7 @@ type MetricPersistence interface {
 	// name.
 	GetFingerprintsForLabelName(model.LabelName) (model.Fingerprints, error)
 
+	// Get the metric associated with the provided fingerprint.
 	GetMetricForFingerprint(model.Fingerprint) (*model.Metric, error)
 
 	GetValueAtTime(model.Fingerprint, time.Time, StalenessPolicy) (*model.Sample, error)
@@ -52,6 +53,7 @@ type MetricPersistence interface {
 
 	ForEachSample(IteratorsForFingerprintBuilder) (err error)
 
+	// Get all label values that are associated with a given label name.
 	GetAllValuesForLabel(model.LabelName) (model.LabelValues, error)
 
 	// Requests the storage stack to build a materialized View of the values
diff --git a/storage/metric/leveldb.go b/storage/metric/leveldb.go
index be1841e0b3..654ae978b9 100644
--- a/storage/metric/leveldb.go
+++ b/storage/metric/leveldb.go
@@ -1221,13 +1221,16 @@ func (l *LevelDBMetricPersistence) GetRangeValues(fp model.Fingerprint, i model.
 type MetricKeyDecoder struct{}
 
 func (d *MetricKeyDecoder) DecodeKey(in interface{}) (out interface{}, err error) {
-	unmarshaled := &dto.LabelPair{}
-	err = proto.Unmarshal(in.([]byte), unmarshaled)
+	unmarshaled := dto.LabelPair{}
+	err = proto.Unmarshal(in.([]byte), &unmarshaled)
 	if err != nil {
 		return
 	}
 
-	out = unmarshaled
+	out = model.LabelPair{
+		Name:  model.LabelName(*unmarshaled.Name),
+		Value: model.LabelValue(*unmarshaled.Value),
+	}
 
 	return
 }
@@ -1240,9 +1243,9 @@ type LabelNameFilter struct {
 	labelName model.LabelName
 }
 
-func (f *LabelNameFilter) Filter(key, value interface{}) (filterResult storage.FilterResult) {
-	unmarshaled, ok := key.(*dto.LabelPair)
-	if ok && model.LabelName(*unmarshaled.Name) == f.labelName {
+func (f LabelNameFilter) Filter(key, value interface{}) (filterResult storage.FilterResult) {
+	labelPair, ok := key.(model.LabelPair)
+	if ok && labelPair.Name == f.labelName {
 		return storage.ACCEPT
 	}
 	return storage.SKIP
@@ -1253,8 +1256,8 @@ type CollectLabelValuesOp struct {
 }
 
 func (op *CollectLabelValuesOp) Operate(key, value interface{}) (err *storage.OperatorError) {
-	unmarshaled := key.(*dto.LabelPair)
-	op.labelValues = append(op.labelValues, model.LabelValue(*unmarshaled.Value))
+	labelPair := key.(model.LabelPair)
+	op.labelValues = append(op.labelValues, model.LabelValue(labelPair.Value))
 	return
 }
 
diff --git a/storage/metric/memory.go b/storage/metric/memory.go
index b7372d6728..7d4c2fb1c9 100644
--- a/storage/metric/memory.go
+++ b/storage/metric/memory.go
@@ -352,13 +352,12 @@ func (s memorySeriesStorage) GetAllValuesForLabel(labelName model.LabelName) (va
 	valueSet := map[model.LabelValue]bool{}
 	for _, series := range s.fingerprintToSeries {
 		if value, ok := series.metric[labelName]; ok {
-			valueSet[value] = true
+			if !valueSet[value] {
+				values = append(values, value)
+				valueSet[value] = true
+			}
 		}
 	}
-	for value := range valueSet {
-		values = append(values, value)
-	}
-	sort.Sort(values)
 	return
 }
 
diff --git a/storage/metric/tiered.go b/storage/metric/tiered.go
index a5212aa577..311caef6d3 100644
--- a/storage/metric/tiered.go
+++ b/storage/metric/tiered.go
@@ -63,9 +63,12 @@ type Storage interface {
 	Flush()
 	Close()
 
-	// MetricPersistence proxy methods.
+	// Get all label values that are associated with the provided label name.
 	GetAllValuesForLabel(model.LabelName) (model.LabelValues, error)
+	// Get all of the metric fingerprints that are associated with the provided
+	// label set.
 	GetFingerprintsForLabelSet(model.LabelSet) (model.Fingerprints, error)
+	// Get the metric associated with the provided fingerprint.
 	GetMetricForFingerprint(model.Fingerprint) (m *model.Metric, err error)
 }
 
@@ -530,12 +533,11 @@ func (t *tieredStorage) GetAllValuesForLabel(labelName model.LabelName) (values
 
 	valueSet := map[model.LabelValue]bool{}
 	for _, value := range append(diskValues, memoryValues...) {
-		valueSet[value] = true
+		if !valueSet[value] {
+			values = append(values, value)
+			valueSet[value] = true
+		}
 	}
-	for value := range valueSet {
-		values = append(values, value)
-	}
-	sort.Sort(values)
 
 	return
 }
diff --git a/storage/metric/tiered_test.go b/storage/metric/tiered_test.go
index ab01f7c71c..a5e0b9bfaa 100644
--- a/storage/metric/tiered_test.go
+++ b/storage/metric/tiered_test.go
@@ -19,6 +19,7 @@ import (
 	"github.com/prometheus/prometheus/utility/test"
 	"io/ioutil"
 	"os"
+	"sort"
 	"testing"
 	"time"
 )
@@ -518,6 +519,7 @@ func TestGetAllValuesForLabel(t *testing.T) {
 			t.Fatalf("%d. Expected metric count %d, got %d", i, len(scenario.out), len(metricNames))
 		}
 
+		sort.Sort(metricNames)
 		for j, expected := range scenario.out {
 			if expected != string(metricNames[j]) {
 				t.Fatalf("%d.%d. Expected metric %s, got %s", i, j, expected, metricNames[j])