From bceb5c1b1614d1c1a4856bb680a033dee674d467 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 12 Apr 2017 16:25:32 +0100 Subject: [PATCH] When checking for amended points, do it in terms of bits. NaN != NaN, so the previous code would incorrectly report it as changed. There's also plans to take advantage of the NaN payload, so look at the entire value. --- head.go | 2 +- head_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/head.go b/head.go index a34c1351c..5818dbbe4 100644 --- a/head.go +++ b/head.go @@ -357,7 +357,7 @@ func (a *headAppender) AddFast(ref uint64, t int64, v float64) error { if t < c.maxTime { return ErrOutOfOrderSample } - if c.maxTime == t && ms.lastValue != v { + if c.maxTime == t && math.Float64bits(ms.lastValue) != math.Float64bits(v) { return ErrAmendSample } } diff --git a/head_test.go b/head_test.go index 1cd4bc0b8..63f6da6f7 100644 --- a/head_test.go +++ b/head_test.go @@ -15,6 +15,7 @@ package tsdb import ( "io/ioutil" + "math" "os" "testing" "unsafe" @@ -85,3 +86,78 @@ func readPrometheusLabels(fn string, n int) ([]labels.Labels, error) { } return mets, nil } + +func TestAmendDatapointCausesError(t *testing.T) { + tmpdir, _ := ioutil.TempDir("", "test") + defer os.RemoveAll(tmpdir) + + hb, err := createHeadBlock(tmpdir+"/hb", 0, nil, 0, 1000) + if err != nil { + t.Fatalf("Error creating head block: %s", err) + } + + app := hb.Appender() + _, err = app.Add(labels.Labels{}, 0, 0) + if err != nil { + t.Fatalf("Failed to add sample: %s", err) + } + if err = app.Commit(); err != nil { + t.Fatalf("Unexpected error committing appender: %s", err) + } + + app = hb.Appender() + _, err = app.Add(labels.Labels{}, 0, 1) + if err != ErrAmendSample { + t.Fatalf("Expected error amending sample, got: %s", err) + } +} + +func TestDuplicateNaNDatapointNoAmendError(t *testing.T) { + tmpdir, _ := ioutil.TempDir("", "test") + defer os.RemoveAll(tmpdir) + + hb, err := createHeadBlock(tmpdir+"/hb", 0, nil, 0, 1000) + if err != nil { + t.Fatalf("Error creating head block: %s", err) + } + + app := hb.Appender() + _, err = app.Add(labels.Labels{}, 0, math.NaN()) + if err != nil { + t.Fatalf("Failed to add sample: %s", err) + } + if err = app.Commit(); err != nil { + t.Fatalf("Unexpected error committing appender: %s", err) + } + + app = hb.Appender() + _, err = app.Add(labels.Labels{}, 0, math.NaN()) + if err != nil { + t.Fatalf("Unexpected error adding duplicate NaN sample, got: %s", err) + } +} + +func TestNonDuplicateNaNDatapointsCausesAmendError(t *testing.T) { + tmpdir, _ := ioutil.TempDir("", "test") + defer os.RemoveAll(tmpdir) + + hb, err := createHeadBlock(tmpdir+"/hb", 0, nil, 0, 1000) + if err != nil { + t.Fatalf("Error creating head block: %s", err) + } + + app := hb.Appender() + _, err = app.Add(labels.Labels{}, 0, math.Float64frombits(0x7ff0000000000001)) + if err != nil { + t.Fatalf("Failed to add sample: %s", err) + } + if err = app.Commit(); err != nil { + t.Fatalf("Unexpected error committing appender: %s", err) + } + + app = hb.Appender() + _, err = app.Add(labels.Labels{}, 0, math.Float64frombits(0x7ff0000000000002)) + if err != ErrAmendSample { + t.Fatalf("Expected error amending sample, got: %s", err) + } +}