From ab870ac6534114384c634037b6dfccdf01c83d04 Mon Sep 17 00:00:00 2001 From: Julius Volz Date: Thu, 6 Oct 2016 21:53:40 +0200 Subject: [PATCH 1/4] Clean up some doc comments --- storage/local/chunk/delta.go | 12 ++++++------ storage/local/chunk/doubledelta.go | 12 ++++++------ storage/local/chunk/varbit.go | 14 +++++++------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/storage/local/chunk/delta.go b/storage/local/chunk/delta.go index 911ab63fa..4e04bca47 100644 --- a/storage/local/chunk/delta.go +++ b/storage/local/chunk/delta.go @@ -72,7 +72,7 @@ func newDeltaEncodedChunk(tb, vb deltaBytes, isInt bool, length int) *deltaEncod return &c } -// add implements chunk. +// Add implements chunk. func (c deltaEncodedChunk) Add(s model.SamplePair) ([]Chunk, error) { // TODO(beorn7): Since we return &c, this method might cause an unnecessary allocation. if c.len() == 0 { @@ -177,7 +177,7 @@ func (c deltaEncodedChunk) Add(s model.SamplePair) ([]Chunk, error) { return []Chunk{&c}, nil } -// clone implements chunk. +// Clone implements chunk. func (c deltaEncodedChunk) Clone() Chunk { clone := make(deltaEncodedChunk, len(c), cap(c)) copy(clone, c) @@ -201,7 +201,7 @@ func (c *deltaEncodedChunk) NewIterator() Iterator { }) } -// marshal implements chunk. +// Marshal implements chunk. func (c deltaEncodedChunk) Marshal(w io.Writer) error { if len(c) > math.MaxUint16 { panic("chunk buffer length would overflow a 16 bit uint.") @@ -232,7 +232,7 @@ func (c deltaEncodedChunk) MarshalToBuf(buf []byte) error { return nil } -// unmarshal implements chunk. +// Unmarshal implements chunk. func (c *deltaEncodedChunk) Unmarshal(r io.Reader) error { *c = (*c)[:cap(*c)] if _, err := io.ReadFull(r, *c); err != nil { @@ -249,7 +249,7 @@ func (c *deltaEncodedChunk) Unmarshal(r io.Reader) error { return nil } -// unmarshalFromBuf implements chunk. +// UnmarshalFromBuf implements chunk. func (c *deltaEncodedChunk) UnmarshalFromBuf(buf []byte) error { *c = (*c)[:cap(*c)] copy(*c, buf) @@ -264,7 +264,7 @@ func (c *deltaEncodedChunk) UnmarshalFromBuf(buf []byte) error { return nil } -// encoding implements chunk. +// Encoding implements chunk. func (c deltaEncodedChunk) Encoding() Encoding { return Delta } // Utilization implements chunk. diff --git a/storage/local/chunk/doubledelta.go b/storage/local/chunk/doubledelta.go index 51cc60346..9a4744130 100644 --- a/storage/local/chunk/doubledelta.go +++ b/storage/local/chunk/doubledelta.go @@ -80,7 +80,7 @@ func newDoubleDeltaEncodedChunk(tb, vb deltaBytes, isInt bool, length int) *doub return &c } -// add implements chunk. +// Add implements chunk. func (c doubleDeltaEncodedChunk) Add(s model.SamplePair) ([]Chunk, error) { // TODO(beorn7): Since we return &c, this method might cause an unnecessary allocation. if c.len() == 0 { @@ -184,7 +184,7 @@ func (c doubleDeltaEncodedChunk) Add(s model.SamplePair) ([]Chunk, error) { return []Chunk{&c}, nil } -// clone implements chunk. +// Clone implements chunk. func (c doubleDeltaEncodedChunk) Clone() Chunk { clone := make(doubleDeltaEncodedChunk, len(c), cap(c)) copy(clone, c) @@ -210,7 +210,7 @@ func (c *doubleDeltaEncodedChunk) NewIterator() Iterator { }) } -// marshal implements chunk. +// Marshal implements chunk. func (c doubleDeltaEncodedChunk) Marshal(w io.Writer) error { if len(c) > math.MaxUint16 { panic("chunk buffer length would overflow a 16 bit uint") @@ -241,7 +241,7 @@ func (c doubleDeltaEncodedChunk) MarshalToBuf(buf []byte) error { return nil } -// unmarshal implements chunk. +// Unmarshal implements chunk. func (c *doubleDeltaEncodedChunk) Unmarshal(r io.Reader) error { *c = (*c)[:cap(*c)] if _, err := io.ReadFull(r, *c); err != nil { @@ -259,7 +259,7 @@ func (c *doubleDeltaEncodedChunk) Unmarshal(r io.Reader) error { return nil } -// unmarshalFromBuf implements chunk. +// UnmarshalFromBuf implements chunk. func (c *doubleDeltaEncodedChunk) UnmarshalFromBuf(buf []byte) error { *c = (*c)[:cap(*c)] copy(*c, buf) @@ -274,7 +274,7 @@ func (c *doubleDeltaEncodedChunk) UnmarshalFromBuf(buf []byte) error { return nil } -// encoding implements chunk. +// Encoding implements chunk. func (c doubleDeltaEncodedChunk) Encoding() Encoding { return DoubleDelta } // Utilization implements chunk. diff --git a/storage/local/chunk/varbit.go b/storage/local/chunk/varbit.go index 21f7cd759..f9d135e73 100644 --- a/storage/local/chunk/varbit.go +++ b/storage/local/chunk/varbit.go @@ -256,7 +256,7 @@ func newVarbitChunk(enc varbitValueEncoding) *varbitChunk { return &c } -// add implements chunk. +// Add implements chunk. func (c *varbitChunk) Add(s model.SamplePair) ([]Chunk, error) { offset := c.nextSampleOffset() switch { @@ -272,7 +272,7 @@ func (c *varbitChunk) Add(s model.SamplePair) ([]Chunk, error) { return c.addLaterSample(s, offset) } -// clone implements chunk. +// Clone implements chunk. func (c varbitChunk) Clone() Chunk { clone := make(varbitChunk, len(c)) copy(clone, c) @@ -284,7 +284,7 @@ func (c varbitChunk) NewIterator() Iterator { return newVarbitChunkIterator(c) } -// marshal implements chunk. +// Marshal implements chunk. func (c varbitChunk) Marshal(w io.Writer) error { n, err := w.Write(c) if err != nil { @@ -296,7 +296,7 @@ func (c varbitChunk) Marshal(w io.Writer) error { return nil } -// marshalToBuf implements chunk. +// MarshalToBuf implements chunk. func (c varbitChunk) MarshalToBuf(buf []byte) error { n := copy(buf, c) if n != len(c) { @@ -305,13 +305,13 @@ func (c varbitChunk) MarshalToBuf(buf []byte) error { return nil } -// unmarshal implements chunk. +// Unmarshal implements chunk. func (c varbitChunk) Unmarshal(r io.Reader) error { _, err := io.ReadFull(r, c) return err } -// unmarshalFromBuf implements chunk. +// UnmarshalFromBuf implements chunk. func (c varbitChunk) UnmarshalFromBuf(buf []byte) error { if copied := copy(c, buf); copied != cap(c) { return fmt.Errorf("insufficient bytes copied from buffer during unmarshaling, want %d, got %d", cap(c), copied) @@ -319,7 +319,7 @@ func (c varbitChunk) UnmarshalFromBuf(buf []byte) error { return nil } -// encoding implements chunk. +// Encoding implements chunk. func (c varbitChunk) Encoding() Encoding { return Varbit } // Utilization implements chunk. From 3f02e33e34e2c312db7bbe5ab7b991046f452c23 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Mon, 10 Oct 2016 16:30:10 +0200 Subject: [PATCH 2/4] Re-add counting of evict chunk ops and decrementing NumMemChunks Also, modify test to expose the regression. --- storage/local/chunk/chunk.go | 2 ++ storage/local/storage_test.go | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/storage/local/chunk/chunk.go b/storage/local/chunk/chunk.go index 738e678f1..11c42b39d 100644 --- a/storage/local/chunk/chunk.go +++ b/storage/local/chunk/chunk.go @@ -252,6 +252,8 @@ func (d *Desc) MaybeEvict() bool { panic("ChunkLastTime not populated for evicted chunk") } d.C = nil + Ops.WithLabelValues(Evict).Inc() + atomic.AddInt64(&NumMemChunks, -1) return true } diff --git a/storage/local/storage_test.go b/storage/local/storage_test.go index ce7ed2bd6..e3b31620b 100644 --- a/storage/local/storage_test.go +++ b/storage/local/storage_test.go @@ -20,6 +20,7 @@ import ( "math/rand" "os" "strconv" + "sync/atomic" "testing" "testing/quick" "time" @@ -1412,6 +1413,10 @@ func testEvictAndLoadChunkDescs(t *testing.T, encoding chunk.Encoding) { Value: model.SampleValue(3.14), } + // Sadly, chunk.NumMemChunks is a global variable. We have to reset it + // explicitly here. + atomic.StoreInt64(&chunk.NumMemChunks, 0) + s, closer := NewTestStorage(t, encoding) defer closer.Close() @@ -1441,6 +1446,9 @@ func testEvictAndLoadChunkDescs(t *testing.T, encoding chunk.Encoding) { if oldLen <= len(series.chunkDescs) { t.Errorf("Expected number of chunkDescs to decrease, old number %d, current number %d.", oldLen, len(series.chunkDescs)) } + if int64(len(series.chunkDescs)) < atomic.LoadInt64(&chunk.NumMemChunks) { + t.Errorf("NumMemChunks is larger than number of chunk descs, number of chunk descs: %d, NumMemChunks: %d.", len(series.chunkDescs), atomic.LoadInt64(&chunk.NumMemChunks)) + } // Load everything back. it := s.preloadChunksForRange(makeFingerprintSeriesPair(s, fp), 0, 100000) From dc0ad8ee341fd7533e0fca6888fa862be33ffc26 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Mon, 10 Oct 2016 17:02:36 +0200 Subject: [PATCH 3/4] Use Go1.7.1 with Promu --- .promu.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.promu.yml b/.promu.yml index 27aa3aaed..d4ec92803 100644 --- a/.promu.yml +++ b/.promu.yml @@ -1,3 +1,4 @@ +go: 1.7.1 repository: path: github.com/prometheus/prometheus build: From ddcd92b76cab730b1d7cf22de7602cc617f5ae06 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Mon, 10 Oct 2016 16:54:19 +0200 Subject: [PATCH 4/4] Cut v1.2.1 --- CHANGELOG.md | 6 ++++++ VERSION | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a6c00035..3e399132c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.2.1 / 2016-10-10 + +* [BUGFIX] Count chunk evictions properly so that the server doesn't + assume it runs out of memory and subsequencly throttles ingestion. +* [BUGFIX] Use Go1.7.1 for prebuilt binaries to fix issues on MacOS Sierra. + ## 1.2.0 / 2016-10-07 * [FEATURE] Cleaner encoding of query parameters in `/graph` URLs. diff --git a/VERSION b/VERSION index 26aaba0e8..6085e9465 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.2.0 +1.2.1