From ebac14eff3b7c0ff993fe1a77874dc7686ff02cd Mon Sep 17 00:00:00 2001 From: beorn7 Date: Mon, 2 Mar 2015 20:02:37 +0100 Subject: [PATCH 1/4] Add version guard to persistence. --- storage/local/persistence.go | 50 ++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/storage/local/persistence.go b/storage/local/persistence.go index ca6f822b4..dc993d6c4 100644 --- a/storage/local/persistence.go +++ b/storage/local/persistence.go @@ -21,6 +21,8 @@ import ( "os" "path" "path/filepath" + "strconv" + "strings" "sync" "time" @@ -36,6 +38,11 @@ import ( ) const ( + // Version of the storage, as it can be found in the version file. + // Increment to protect against icompatible changes. + Version = 1 + versionFileName = "VERSION" + seriesFileSuffix = ".db" seriesTempFileSuffix = ".db.tmp" seriesDirNameLen = 2 // How many bytes of the fingerprint in dir name. @@ -114,10 +121,49 @@ type persistence struct { // newPersistence returns a newly allocated persistence backed by local disk storage, ready to use. func newPersistence(basePath string, chunkLen int, dirty bool) (*persistence, error) { - if err := os.MkdirAll(basePath, 0700); err != nil { + dirtyPath := filepath.Join(basePath, dirtyFileName) + versionPath := filepath.Join(basePath, versionFileName) + + if file, err := os.Open(versionPath); err == nil { + defer file.Close() + data := make([]byte, 8) + n, err := file.Read(data) + if err != nil { + return nil, err + } + if persistedVersion, err := strconv.Atoi(strings.TrimSpace(string(data[:n]))); err != nil { + return nil, fmt.Errorf("cannot parse content of %s: %s", versionPath, data[:n]) + } else if persistedVersion != Version { + return nil, fmt.Errorf("found storage version %d on disk, need version %d - please wipe storage or run a version of Prometheus compatible with storage version %d", persistedVersion, Version, persistedVersion) + } + } else if os.IsNotExist(err) { + // No version file found. Let's create the directory (in case + // it's not there yet) and then check if it is actually + // empty. If not, we have found an old storage directory without + // version file, so we have to bail out. + if err := os.MkdirAll(basePath, 0700); err != nil { + return nil, err + } + d, err := os.Open(basePath) + if err != nil { + return nil, err + } + defer d.Close() + if fis, _ := d.Readdir(1); len(fis) > 0 { + return nil, fmt.Errorf("could not detect storage version on disk, assuming version 0, need version %d - please wipe storage or run a version of Prometheus compatible with storage version 0", Version) + } + // Finally we can write our own version into a new version file. + file, err := os.Create(versionPath) + if err != nil { + return nil, err + } + defer file.Close() + if _, err := fmt.Fprintf(file, "%d\n", Version); err != nil { + return nil, err + } + } else { return nil, err } - dirtyPath := filepath.Join(basePath, dirtyFileName) fLock, dirtyfileExisted, err := flock.New(dirtyPath) if err != nil { From 9e85ab0eef4b972301dbfceaf6bced0beabef587 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 3 Mar 2015 17:31:49 +0100 Subject: [PATCH 2/4] Apply the new signature/fingerprinting functions from client_golang. This requires the new version of client_golang (vendoring will follow in the next commit), which changes the fingerprinting for clientmodel.Metric. --- rules/ast/ast.go | 24 +++---------------- rules/ast/functions.go | 10 ++++---- rules/ast/quantile.go | 52 ++++++------------------------------------ 3 files changed, 15 insertions(+), 71 deletions(-) diff --git a/rules/ast/ast.go b/rules/ast/ast.go index 84c4aa284..0eeb3aae4 100644 --- a/rules/ast/ast.go +++ b/rules/ast/ast.go @@ -17,7 +17,6 @@ import ( "errors" "flag" "fmt" - "hash/fnv" "math" "time" @@ -383,23 +382,6 @@ func (node *ScalarFunctionCall) Eval(timestamp clientmodel.Timestamp) clientmode return node.function.callFn(timestamp, node.args).(clientmodel.SampleValue) } -// hashForLabels returns a hash value taken from the label/value pairs of the -// specified labels in the metric. -func hashForLabels(metric clientmodel.Metric, labels clientmodel.LabelNames) uint64 { - var result uint64 - s := fnv.New64a() - - for _, label := range labels { - s.Write([]byte(label)) - s.Write([]byte{clientmodel.SeparatorByte}) - s.Write([]byte(metric[label])) - result ^= s.Sum64() - s.Reset() - } - - return result -} - // EvalVectorInstant evaluates a VectorNode with an instant query. func EvalVectorInstant(node VectorNode, timestamp clientmodel.Timestamp, storage local.Storage, queryStats *stats.TimerGroup) (Vector, error) { totalEvalTimer := queryStats.GetTimer(stats.TotalEvalTime).Start() @@ -503,7 +485,7 @@ func (node *VectorAggregation) Eval(timestamp clientmodel.Timestamp) Vector { vector := node.vector.Eval(timestamp) result := map[uint64]*groupedAggregation{} for _, sample := range vector { - groupingKey := hashForLabels(sample.Metric.Metric, node.groupBy) + groupingKey := clientmodel.SignatureForLabels(sample.Metric.Metric, node.groupBy) if groupedResult, ok := result[groupingKey]; ok { if node.keepExtraLabels { groupedResult.labels = labelIntersection(groupedResult.labels, sample.Metric) @@ -879,7 +861,7 @@ func (node *VectorArithExpr) evalVectors(timestamp clientmodel.Timestamp, lhs, r metric := node.resultMetric(ls, rs) // Check if the same label set has been added for a many-to-one matching before. if node.matchCardinality == MatchManyToOne || node.matchCardinality == MatchOneToMany { - insHash := hashForLabels(metric.Metric, node.includeLabels) + insHash := clientmodel.SignatureForLabels(metric.Metric, node.includeLabels) if ihs, exists := added[hash]; exists { for _, ih := range ihs { if ih == insHash { @@ -971,7 +953,7 @@ func (node *VectorArithExpr) hashForMetric(metric clientmodel.Metric) uint64 { } } } - return hashForLabels(metric, labels) + return clientmodel.SignatureForLabels(metric, labels) } // Eval implements the MatrixNode interface and returns the value of diff --git a/rules/ast/functions.go b/rules/ast/functions.go index 3a207b976..79908cd5b 100644 --- a/rules/ast/functions.go +++ b/rules/ast/functions.go @@ -504,7 +504,7 @@ func histogramQuantileImpl(timestamp clientmodel.Timestamp, args []Node) interfa q := args[0].(ScalarNode).Eval(timestamp) inVec := args[1].(VectorNode).Eval(timestamp) outVec := Vector{} - fpToMetricWithBuckets := map[clientmodel.Fingerprint]*metricWithBuckets{} + signatureToMetricWithBuckets := map[uint64]*metricWithBuckets{} for _, el := range inVec { upperBound, err := strconv.ParseFloat( string(el.Metric.Metric[clientmodel.BucketLabel]), 64, @@ -514,18 +514,18 @@ func histogramQuantileImpl(timestamp clientmodel.Timestamp, args []Node) interfa // TODO(beorn7): Issue a warning somehow. continue } - fp := bucketFingerprint(el.Metric.Metric) - mb, ok := fpToMetricWithBuckets[fp] + signature := clientmodel.SignatureWithoutLabels(el.Metric.Metric, excludedLabels) + mb, ok := signatureToMetricWithBuckets[signature] if !ok { el.Metric.Delete(clientmodel.BucketLabel) el.Metric.Delete(clientmodel.MetricNameLabel) mb = &metricWithBuckets{el.Metric, nil} - fpToMetricWithBuckets[fp] = mb + signatureToMetricWithBuckets[signature] = mb } mb.buckets = append(mb.buckets, bucket{upperBound, el.Value}) } - for _, mb := range fpToMetricWithBuckets { + for _, mb := range signatureToMetricWithBuckets { outVec = append(outVec, &Sample{ Metric: mb.metric, Value: clientmodel.SampleValue(quantile(q, mb.buckets)), diff --git a/rules/ast/quantile.go b/rules/ast/quantile.go index 0f628153f..bf43c777d 100644 --- a/rules/ast/quantile.go +++ b/rules/ast/quantile.go @@ -14,8 +14,6 @@ package ast import ( - "encoding/binary" - "hash/fnv" "math" "sort" @@ -24,6 +22,13 @@ import ( // Helpers to calculate quantiles. +// excludedLabels are the labels to exclude from signature calculation for +// quantiles. +var excludedLabels = map[clientmodel.LabelName]struct{}{ + clientmodel.MetricNameLabel: struct{}{}, + clientmodel.BucketLabel: struct{}{}, +} + type bucket struct { upperBound float64 count clientmodel.SampleValue @@ -99,46 +104,3 @@ func quantile(q clientmodel.SampleValue, buckets buckets) float64 { } return bucketStart + (bucketEnd-bucketStart)*float64(rank/count) } - -// bucketFingerprint works like the Fingerprint method of Metric, but ignores -// the name and the bucket label. -func bucketFingerprint(m clientmodel.Metric) clientmodel.Fingerprint { - numLabels := 0 - if len(m) > 2 { - numLabels = len(m) - 2 - } - labelNames := make([]string, 0, numLabels) - maxLength := 0 - - for labelName, labelValue := range m { - if labelName == clientmodel.MetricNameLabel || labelName == clientmodel.BucketLabel { - continue - } - labelNames = append(labelNames, string(labelName)) - if len(labelName) > maxLength { - maxLength = len(labelName) - } - if len(labelValue) > maxLength { - maxLength = len(labelValue) - } - } - - sort.Strings(labelNames) - - summer := fnv.New64a() - buf := make([]byte, maxLength) - - for _, labelName := range labelNames { - labelValue := m[clientmodel.LabelName(labelName)] - - copy(buf, labelName) - summer.Write(buf[:len(labelName)]) - summer.Write([]byte{clientmodel.SeparatorByte}) - - copy(buf, labelValue) - summer.Write(buf[:len(labelValue)]) - summer.Write([]byte{clientmodel.SeparatorByte}) - } - - return clientmodel.Fingerprint(binary.LittleEndian.Uint64(summer.Sum(nil))) -} From a18cb29fa8e021571700f1dd3fed1ff13c0792ed Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 3 Mar 2015 18:33:39 +0100 Subject: [PATCH 3/4] Update vendored client_golang to v0.3.0. --- Godeps/Godeps.json | 16 +- .../extraction/metricfamilyprocessor.go | 6 +- .../prometheus/client_golang/model/metric.go | 34 +--- .../client_golang/model/metric_test.go | 14 +- .../client_golang/model/signature.go | 74 ++++++- .../client_golang/model/signature_test.go | 180 ++++++++++++++---- .../client_golang/model/timestamp.go | 1 + .../client_golang/prometheus/desc.go | 3 +- .../prometheus/example_selfcollector_test.go | 4 +- .../client_golang/prometheus/examples_test.go | 6 +- .../client_golang/prometheus/expvar_test.go | 3 +- .../client_golang/prometheus/histogram.go | 1 + .../client_golang/prometheus/registry.go | 4 +- .../client_golang/prometheus/summary.go | 4 +- .../client_golang/prometheus/value.go | 3 +- .../client_golang/text/bench_test.go | 5 +- .../prometheus/client_golang/text/parse.go | 3 +- .../client_golang/text/parse_test.go | 2 +- .../prometheus/client_golang/text/proto.go | 1 + 19 files changed, 250 insertions(+), 114 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 3adf64689..485b64143 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -29,23 +29,23 @@ }, { "ImportPath": "github.com/prometheus/client_golang/extraction", - "Comment": "0.2.0-3-ga6c5e0f", - "Rev": "a6c5e0f955686b0e7bbfd791edecfc65aa72593a" + "Comment": "0.3.0", + "Rev": "dbbb6c9e1dbb3bf19d0f9a61baa852cbfcee1896" }, { "ImportPath": "github.com/prometheus/client_golang/model", - "Comment": "0.2.0-3-ga6c5e0f", - "Rev": "a6c5e0f955686b0e7bbfd791edecfc65aa72593a" + "Comment": "0.3.0", + "Rev": "dbbb6c9e1dbb3bf19d0f9a61baa852cbfcee1896" }, { "ImportPath": "github.com/prometheus/client_golang/prometheus", - "Comment": "0.2.0-3-ga6c5e0f", - "Rev": "a6c5e0f955686b0e7bbfd791edecfc65aa72593a" + "Comment": "0.3.0", + "Rev": "dbbb6c9e1dbb3bf19d0f9a61baa852cbfcee1896" }, { "ImportPath": "github.com/prometheus/client_golang/text", - "Comment": "0.2.0-3-ga6c5e0f", - "Rev": "a6c5e0f955686b0e7bbfd791edecfc65aa72593a" + "Comment": "0.3.0", + "Rev": "dbbb6c9e1dbb3bf19d0f9a61baa852cbfcee1896" }, { "ImportPath": "github.com/prometheus/client_model/go", diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/extraction/metricfamilyprocessor.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/extraction/metricfamilyprocessor.go index 7ec92a188..5edb49c24 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/extraction/metricfamilyprocessor.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/extraction/metricfamilyprocessor.go @@ -18,9 +18,11 @@ import ( "io" "math" - "github.com/matttproud/golang_protobuf_extensions/ext" - "github.com/prometheus/client_golang/model" dto "github.com/prometheus/client_model/go" + + "github.com/matttproud/golang_protobuf_extensions/ext" + + "github.com/prometheus/client_golang/model" ) type metricFamilyProcessor struct{} diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric.go index 74f394b92..48210ab8b 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric.go @@ -14,10 +14,8 @@ package model import ( - "encoding/binary" "encoding/json" "fmt" - "hash/fnv" "sort" "strings" ) @@ -66,37 +64,7 @@ func (m Metric) String() string { // Fingerprint returns a Metric's Fingerprint. func (m Metric) Fingerprint() Fingerprint { - labelNames := make([]string, 0, len(m)) - maxLength := 0 - - for labelName, labelValue := range m { - labelNames = append(labelNames, string(labelName)) - if len(labelName) > maxLength { - maxLength = len(labelName) - } - if len(labelValue) > maxLength { - maxLength = len(labelValue) - } - } - - sort.Strings(labelNames) - - summer := fnv.New64a() - buf := make([]byte, maxLength) - - for _, labelName := range labelNames { - labelValue := m[LabelName(labelName)] - - copy(buf, labelName) - summer.Write(buf[:len(labelName)]) - - summer.Write(separator) - - copy(buf, labelValue) - summer.Write(buf[:len(labelValue)]) - } - - return Fingerprint(binary.LittleEndian.Uint64(summer.Sum(nil))) + return metricToFingerprint(m) } // Clone returns a copy of the Metric. diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric_test.go index 7c31bdfb4..f4cd6ee2b 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/metric_test.go @@ -22,7 +22,7 @@ func testMetric(t testing.TB) { }{ { input: Metric{}, - fingerprint: 2676020557754725067, + fingerprint: 14695981039346656037, }, { input: Metric{ @@ -30,31 +30,27 @@ func testMetric(t testing.TB) { "occupation": "robot", "manufacturer": "westinghouse", }, - fingerprint: 13260944541294022935, + fingerprint: 11310079640881077873, }, { input: Metric{ "x": "y", }, - fingerprint: 1470933794305433534, + fingerprint: 13948396922932177635, }, - // The following two demonstrate a bug in fingerprinting. They - // should not have the same fingerprint with a sane - // fingerprinting function. See - // https://github.com/prometheus/client_golang/issues/74 . { input: Metric{ "a": "bb", "b": "c", }, - fingerprint: 3734646176939799877, + fingerprint: 3198632812309449502, }, { input: Metric{ "a": "b", "bb": "c", }, - fingerprint: 3734646176939799877, + fingerprint: 5774953389407657638, }, } diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature.go index 4b392fb18..ab77ae847 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature.go @@ -63,10 +63,10 @@ func LabelsToSignature(labels map[string]string) uint64 { hb := getHashAndBuf() defer putHashAndBuf(hb) - for k, v := range labels { - hb.b.WriteString(k) + for labelName, labelValue := range labels { + hb.b.WriteString(labelName) hb.b.WriteByte(SeparatorByte) - hb.b.WriteString(v) + hb.b.WriteString(labelValue) hb.h.Write(hb.b.Bytes()) result ^= hb.h.Sum64() hb.h.Reset() @@ -75,10 +75,34 @@ func LabelsToSignature(labels map[string]string) uint64 { return result } -// LabelValuesToSignature returns a unique signature (i.e., fingerprint) for the -// values of a given label set. -func LabelValuesToSignature(labels map[string]string) uint64 { - if len(labels) == 0 { +// metricToFingerprint works exactly as LabelsToSignature but takes a Metric as +// parameter (rather than a label map) and returns a Fingerprint. +func metricToFingerprint(m Metric) Fingerprint { + if len(m) == 0 { + return Fingerprint(emptyLabelSignature) + } + + var result uint64 + hb := getHashAndBuf() + defer putHashAndBuf(hb) + + for labelName, labelValue := range m { + hb.b.WriteString(string(labelName)) + hb.b.WriteByte(SeparatorByte) + hb.b.WriteString(string(labelValue)) + hb.h.Write(hb.b.Bytes()) + result ^= hb.h.Sum64() + hb.h.Reset() + hb.b.Reset() + } + return Fingerprint(result) +} + +// SignatureForLabels works like LabelsToSignature but takes a Metric as +// parameter (rather than a label map) and only includes the labels with the +// specified LabelNames into the signature calculation. +func SignatureForLabels(m Metric, labels LabelNames) uint64 { + if len(m) == 0 || len(labels) == 0 { return emptyLabelSignature } @@ -86,8 +110,10 @@ func LabelValuesToSignature(labels map[string]string) uint64 { hb := getHashAndBuf() defer putHashAndBuf(hb) - for _, v := range labels { - hb.b.WriteString(v) + for _, label := range labels { + hb.b.WriteString(string(label)) + hb.b.WriteByte(SeparatorByte) + hb.b.WriteString(string(m[label])) hb.h.Write(hb.b.Bytes()) result ^= hb.h.Sum64() hb.h.Reset() @@ -95,3 +121,33 @@ func LabelValuesToSignature(labels map[string]string) uint64 { } return result } + +// SignatureWithoutLabels works like LabelsToSignature but takes a Metric as +// parameter (rather than a label map) and excludes the labels with any of the +// specified LabelNames from the signature calculation. +func SignatureWithoutLabels(m Metric, labels map[LabelName]struct{}) uint64 { + if len(m) == 0 { + return emptyLabelSignature + } + + var result uint64 + hb := getHashAndBuf() + defer putHashAndBuf(hb) + + for labelName, labelValue := range m { + if _, exclude := labels[labelName]; exclude { + continue + } + hb.b.WriteString(string(labelName)) + hb.b.WriteByte(SeparatorByte) + hb.b.WriteString(string(labelValue)) + hb.h.Write(hb.b.Bytes()) + result ^= hb.h.Sum64() + hb.h.Reset() + hb.b.Reset() + } + if result == 0 { + return emptyLabelSignature + } + return result +} diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature_test.go index ad20fa3ff..be31998d6 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/signature_test.go @@ -18,7 +18,7 @@ import ( "testing" ) -func testLabelsToSignature(t testing.TB) { +func TestLabelsToSignature(t *testing.T) { var scenarios = []struct { in map[string]string out uint64 @@ -42,57 +42,112 @@ func testLabelsToSignature(t testing.TB) { } } -func TestLabelToSignature(t *testing.T) { - testLabelsToSignature(t) -} - -func TestEmptyLabelSignature(t *testing.T) { - input := []map[string]string{nil, {}} - - var ms runtime.MemStats - runtime.ReadMemStats(&ms) - - alloc := ms.Alloc - - for _, labels := range input { - LabelsToSignature(labels) +func TestMetricToFingerprint(t *testing.T) { + var scenarios = []struct { + in Metric + out Fingerprint + }{ + { + in: Metric{}, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + out: 12952432476264840823, + }, } - runtime.ReadMemStats(&ms) + for i, scenario := range scenarios { + actual := metricToFingerprint(scenario.in) - if got := ms.Alloc; alloc != got { - t.Fatal("expected LabelsToSignature with empty labels not to perform allocations") - } -} - -func BenchmarkLabelToSignature(b *testing.B) { - for i := 0; i < b.N; i++ { - testLabelsToSignature(b) - } -} - -func benchmarkLabelValuesToSignature(b *testing.B, l map[string]string, e uint64) { - for i := 0; i < b.N; i++ { - if a := LabelValuesToSignature(l); a != e { - b.Fatalf("expected signature of %d for %s, got %d", e, l, a) + if actual != scenario.out { + t.Errorf("%d. expected %d, got %d", i, scenario.out, actual) } } } -func BenchmarkLabelValuesToSignatureScalar(b *testing.B) { - benchmarkLabelValuesToSignature(b, nil, 14695981039346656037) +func TestSignatureForLabels(t *testing.T) { + var scenarios = []struct { + in Metric + labels LabelNames + out uint64 + }{ + { + in: Metric{}, + labels: nil, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: LabelNames{"fear", "name"}, + out: 12952432476264840823, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough", "foo": "bar"}, + labels: LabelNames{"fear", "name"}, + out: 12952432476264840823, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: LabelNames{}, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: nil, + out: 14695981039346656037, + }, + } + + for i, scenario := range scenarios { + actual := SignatureForLabels(scenario.in, scenario.labels) + + if actual != scenario.out { + t.Errorf("%d. expected %d, got %d", i, scenario.out, actual) + } + } } -func BenchmarkLabelValuesToSignatureSingle(b *testing.B) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value"}, 2653746141194979650) -} +func TestSignatureWithoutLabels(t *testing.T) { + var scenarios = []struct { + in Metric + labels map[LabelName]struct{} + out uint64 + }{ + { + in: Metric{}, + labels: nil, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: map[LabelName]struct{}{"fear": struct{}{}, "name": struct{}{}}, + out: 14695981039346656037, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough", "foo": "bar"}, + labels: map[LabelName]struct{}{"foo": struct{}{}}, + out: 12952432476264840823, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: map[LabelName]struct{}{}, + out: 12952432476264840823, + }, + { + in: Metric{"name": "garland, briggs", "fear": "love is not enough"}, + labels: nil, + out: 12952432476264840823, + }, + } -func BenchmarkLabelValuesToSignatureDouble(b *testing.B) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value"}, 8893559499616767364) -} + for i, scenario := range scenarios { + actual := SignatureWithoutLabels(scenario.in, scenario.labels) -func BenchmarkLabelValuesToSignatureTriple(b *testing.B) { - benchmarkLabelValuesToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 1685970066862087833) + if actual != scenario.out { + t.Errorf("%d. expected %d, got %d", i, scenario.out, actual) + } + } } func benchmarkLabelToSignature(b *testing.B, l map[string]string, e uint64) { @@ -118,3 +173,46 @@ func BenchmarkLabelToSignatureDouble(b *testing.B) { func BenchmarkLabelToSignatureTriple(b *testing.B) { benchmarkLabelToSignature(b, map[string]string{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 15738406913934009676) } + +func benchmarkMetricToFingerprint(b *testing.B, m Metric, e Fingerprint) { + for i := 0; i < b.N; i++ { + if a := metricToFingerprint(m); a != e { + b.Fatalf("expected signature of %d for %s, got %d", e, m, a) + } + } +} + +func BenchmarkMetricToFingerprintScalar(b *testing.B) { + benchmarkMetricToFingerprint(b, nil, 14695981039346656037) +} + +func BenchmarkMetricToFingerprintSingle(b *testing.B) { + benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value"}, 5147259542624943964) +} + +func BenchmarkMetricToFingerprintDouble(b *testing.B) { + benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value", "second-label": "second-label-value"}, 18269973311206963528) +} + +func BenchmarkMetricToFingerprintTriple(b *testing.B) { + benchmarkMetricToFingerprint(b, Metric{"first-label": "first-label-value", "second-label": "second-label-value", "third-label": "third-label-value"}, 15738406913934009676) +} + +func TestEmptyLabelSignature(t *testing.T) { + input := []map[string]string{nil, {}} + + var ms runtime.MemStats + runtime.ReadMemStats(&ms) + + alloc := ms.Alloc + + for _, labels := range input { + LabelsToSignature(labels) + } + + runtime.ReadMemStats(&ms) + + if got := ms.Alloc; alloc != got { + t.Fatal("expected LabelsToSignature with empty labels not to perform allocations") + } +} diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/timestamp.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/timestamp.go index 09bd87710..afffdcf75 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/model/timestamp.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/model/timestamp.go @@ -88,6 +88,7 @@ func (t Timestamp) String() string { return strconv.FormatFloat(float64(t)/float64(second), 'f', -1, 64) } +// MarshalJSON implements the json.Marshaler interface. func (t Timestamp) MarshalJSON() ([]byte, error) { return []byte(t.String()), nil } diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/desc.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/desc.go index 46a71cb98..7e1f9e853 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/desc.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/desc.go @@ -11,8 +11,9 @@ import ( "github.com/prometheus/client_golang/model" - "github.com/golang/protobuf/proto" dto "github.com/prometheus/client_model/go" + + "github.com/golang/protobuf/proto" ) var ( diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/example_selfcollector_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/example_selfcollector_test.go index a7a5a3f97..608deeb02 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/example_selfcollector_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/example_selfcollector_test.go @@ -17,8 +17,10 @@ import ( "runtime" "github.com/golang/protobuf/proto" - "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/client_golang/prometheus" ) func NewCallbackMetric(desc *prometheus.Desc, callback func() float64) *CallbackMetric { diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/examples_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/examples_test.go index 37359cc2d..5e62967b0 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/examples_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/examples_test.go @@ -21,9 +21,11 @@ import ( "runtime" "sort" - "github.com/golang/protobuf/proto" - "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" + + "github.com/golang/protobuf/proto" + + "github.com/prometheus/client_golang/prometheus" ) func ExampleGauge() { diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/expvar_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/expvar_test.go index 00af8a4a2..5d3128fae 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/expvar_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/expvar_test.go @@ -19,8 +19,9 @@ import ( "sort" "strings" - "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/client_golang/prometheus" ) func ExampleExpvarCollector() { diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/histogram.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/histogram.go index e0c0042fe..9b36a6115 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/histogram.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/histogram.go @@ -21,6 +21,7 @@ import ( "sync/atomic" "github.com/golang/protobuf/proto" + "github.com/prometheus/client_golang/model" dto "github.com/prometheus/client_model/go" ) diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/registry.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/registry.go index 1c445cba2..550754172 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/registry.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/registry.go @@ -35,9 +35,11 @@ import ( "bitbucket.org/ww/goautoneg" "github.com/golang/protobuf/proto" + + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/client_golang/model" "github.com/prometheus/client_golang/text" - dto "github.com/prometheus/client_model/go" ) var ( diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/summary.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/summary.go index 0ebc5bc01..a731f4e8d 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/summary.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/summary.go @@ -22,8 +22,10 @@ import ( "github.com/beorn7/perks/quantile" "github.com/golang/protobuf/proto" - "github.com/prometheus/client_golang/model" + dto "github.com/prometheus/client_model/go" + + "github.com/prometheus/client_golang/model" ) // A Summary captures individual observations from an event or sample stream and diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/value.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/value.go index 8d48b964e..107d43e37 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/value.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/prometheus/value.go @@ -20,8 +20,9 @@ import ( "sort" "sync/atomic" - "github.com/golang/protobuf/proto" dto "github.com/prometheus/client_model/go" + + "github.com/golang/protobuf/proto" ) // ValueType is an enumeration of metric types that represent a simple value. diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/bench_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/bench_test.go index 5fe44f3d3..df887e0fc 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/bench_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/bench_test.go @@ -16,11 +16,12 @@ package text import ( "bytes" "compress/gzip" - "github.com/matttproud/golang_protobuf_extensions/ext" - dto "github.com/prometheus/client_model/go" "io" "io/ioutil" "testing" + dto "github.com/prometheus/client_model/go" + + "github.com/matttproud/golang_protobuf_extensions/ext" ) // Benchmarks to show how much penalty text format parsing actually inflicts. diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse.go index 8a5af97d1..e317d6850 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse.go @@ -22,9 +22,10 @@ import ( "strconv" "strings" + dto "github.com/prometheus/client_model/go" + "github.com/golang/protobuf/proto" "github.com/prometheus/client_golang/model" - dto "github.com/prometheus/client_model/go" ) // A stateFn is a function that represents a state in a state machine. By diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse_test.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse_test.go index cc3e6470f..6b7adfff5 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse_test.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/parse_test.go @@ -385,7 +385,7 @@ request_duration_microseconds_count 2693 }, }, }, - }, + }, } for i, scenario := range scenarios { diff --git a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/proto.go b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/proto.go index bf6f324f2..058adfc6f 100644 --- a/Godeps/_workspace/src/github.com/prometheus/client_golang/text/proto.go +++ b/Godeps/_workspace/src/github.com/prometheus/client_golang/text/proto.go @@ -19,6 +19,7 @@ import ( "github.com/golang/protobuf/proto" "github.com/matttproud/golang_protobuf_extensions/ext" + dto "github.com/prometheus/client_model/go" ) From 0167083da61f3ae4122d7989290e46012adf5927 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Tue, 3 Mar 2015 18:59:39 +0100 Subject: [PATCH 4/4] Improvements after review. --- storage/local/persistence.go | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/storage/local/persistence.go b/storage/local/persistence.go index dc993d6c4..f8bb94466 100644 --- a/storage/local/persistence.go +++ b/storage/local/persistence.go @@ -18,6 +18,7 @@ import ( "encoding/binary" "fmt" "io" + "io/ioutil" "os" "path" "path/filepath" @@ -38,8 +39,8 @@ import ( ) const ( - // Version of the storage, as it can be found in the version file. - // Increment to protect against icompatible changes. + // Version of the storage as it can be found in the version file. + // Increment to protect against incompatible changes. Version = 1 versionFileName = "VERSION" @@ -124,15 +125,9 @@ func newPersistence(basePath string, chunkLen int, dirty bool) (*persistence, er dirtyPath := filepath.Join(basePath, dirtyFileName) versionPath := filepath.Join(basePath, versionFileName) - if file, err := os.Open(versionPath); err == nil { - defer file.Close() - data := make([]byte, 8) - n, err := file.Read(data) - if err != nil { - return nil, err - } - if persistedVersion, err := strconv.Atoi(strings.TrimSpace(string(data[:n]))); err != nil { - return nil, fmt.Errorf("cannot parse content of %s: %s", versionPath, data[:n]) + if versionData, err := ioutil.ReadFile(versionPath); err == nil { + if persistedVersion, err := strconv.Atoi(strings.TrimSpace(string(versionData))); err != nil { + return nil, fmt.Errorf("cannot parse content of %s: %s", versionPath, versionData) } else if persistedVersion != Version { return nil, fmt.Errorf("found storage version %d on disk, need version %d - please wipe storage or run a version of Prometheus compatible with storage version %d", persistedVersion, Version, persistedVersion) } @@ -144,12 +139,11 @@ func newPersistence(basePath string, chunkLen int, dirty bool) (*persistence, er if err := os.MkdirAll(basePath, 0700); err != nil { return nil, err } - d, err := os.Open(basePath) + fis, err := ioutil.ReadDir(basePath) if err != nil { return nil, err } - defer d.Close() - if fis, _ := d.Readdir(1); len(fis) > 0 { + if len(fis) > 0 { return nil, fmt.Errorf("could not detect storage version on disk, assuming version 0, need version %d - please wipe storage or run a version of Prometheus compatible with storage version 0", Version) } // Finally we can write our own version into a new version file.