promql benchmarks are broken

[tcolgate]

What did you do?

cd promql
got test -run=NONE -bench=Benchmark.

What did you expect to see?

Run each of the benchmarks

What did you see instead? Under which circumstances?

panic: cannot unpin already unpinned chunk

goroutine 104 [running]:
panic(0x787f00, 0xc420a9e780)
        /usr/local/go/src/runtime/panic.go:500 +0x1a1
github.com/prometheus/prometheus/storage/local.(*chunkDesc).unpin(0xc421322880, 0xc421032360)
        /home/tristan/go/src/github.com/prometheus/prometheus/storage/local/chunk.go:150 +0x115
github.com/prometheus/prometheus/storage/local.(*memorySeriesIterator).Close(0xc4210fc380)
        /home/tristan/go/src/github.com/prometheus/prometheus/storage/local/series.go:670 +0x59
github.com/prometheus/prometheus/storage/local.(*boundedIterator).Close(0xc4200f2f60)
        /home/tristan/go/src/github.com/prometheus/prometheus/storage/local/storage.go:482 +0x33
github.com/prometheus/prometheus/promql.(*Engine).closeIterators.func1(0x7f02964d4318, 0xc420678640, 0x40ca0f)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/engine.go:534 +0x96
github.com/prometheus/prometheus/promql.inspector.Visit(0x855870, 0x7f02964d4318, 0xc420678640, 0xf6c43a35000003bf, 0x0)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/ast.go:306 +0x3a
github.com/prometheus/prometheus/promql.Walk(0x9dd820, 0x855870, 0x7f02964d4318, 0xc420678640)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/ast.go:255 +0x7b
github.com/prometheus/prometheus/promql.Walk(0x9dd820, 0x855870, 0x9dd7a0, 0xc4200f3840)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/ast.go:275 +0x234
github.com/prometheus/prometheus/promql.Walk(0x9dd820, 0x855870, 0x7f02964d42c0, 0xc420ec6140)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/ast.go:285 +0x74a
github.com/prometheus/prometheus/promql.Inspect(0x7f02964d42c0, 0xc420ec6140, 0x855870)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/ast.go:316 +0x4b
github.com/prometheus/prometheus/promql.(*Engine).closeIterators(0xc4210c0d80, 0xc4210fa780)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/engine.go:538 +0x67
github.com/prometheus/prometheus/promql.(*Engine).execEvalStmt(0xc4210c0d80, 0x7f02964dd1a8, 0xc4201c87e0, 0xc420ed7df8, 0xc4210fa780, 0x9e03a0, 0xc4200f3820, 0x0, 0x0)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/engine.go:398 +0x11af
github.com/prometheus/prometheus/promql.(*Engine).exec(0xc4210c0d80, 0xc420ed7df8, 0x0, 0x0, 0x0, 0x0)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/engine.go:357 +0x392
github.com/prometheus/prometheus/promql.(*query).Exec(0xc420ed7df8, 0x0)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/engine.go:196 +0x38
github.com/prometheus/prometheus/promql.(*Test).exec(0xc420678550, 0x9dbea0, 0xc42000cc80, 0x9dbda0, 0xc420a9e640)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/test.go:466 +0x1fd
github.com/prometheus/prometheus/promql.(*Test).RunAsBenchmark(0xc420678550, 0xc4200f2240, 0x9dbda0, 0xc420a9e640)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/test.go:422 +0xa3
github.com/prometheus/prometheus/promql.(*Benchmark).Run(0xc4200f2240)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/bench.go:42 +0x65
github.com/prometheus/prometheus/promql.BenchmarkHoltWinters4Week5Min(0xc420084280)
        /home/tristan/go/src/github.com/prometheus/prometheus/promql/functions_test.go:29 +0x4e
testing.(*B).runN(0xc420084280, 0x64)
        /usr/local/go/src/testing/benchmark.go:139 +0xaa
testing.(*B).launch(0xc420084280)
        /usr/local/go/src/testing/benchmark.go:277 +0x118
created by testing.(*B).doBench
        /usr/local/go/src/testing/benchmark.go:245 +0x70
exit status 2
FAIL    github.com/prometheus/prometheus/promql 0.060s

Running a git bisect points at
[3bfec97d46676812d6cb7e4225abd2ba6376d4f3] Make the storage interface higher-level.

Environment

• System information:

Linux pinot 4.7.0-rc7-ARCH

• Prometheus version:

prometheus, version 1.0.1 (branch: HEAD, revision: 3bfec97d46676812d6cb7e4225abd2ba6376d4f3)
  build user:       tristan@pinot
  build date:       20160823-18:47:54
  go version:       go1.7

Have had the same error on a go1.6 build, and with 1.0 (via the bisect)

• Prometheus configuration file:

n/a , running benchmarks

• Logs:

[beorn7]

Arguably, this could also be in the storage layer. Thanks @tcolgate for the bisection. I guess @juliusv will have a look once PromCon is over.

[tcolgate]

I've started having a bit of a dig around. Some of the test stuff seems to
call unpin directly, so that may be at fault. It's not the easiest bit of
the codebase to start out with though :)

On Wed, 24 Aug 2016 at 11:30 Björn Rabenstein notifications@github.com
wrote:

Arguably, this could also be in the storage layer. Thanks @tcolgate
https://github.com/tcolgate for the bisection. I guess @juliusv
https://github.com/juliusv will have a look once PromCon is over.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1912 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEo8z0NZzBqHI1KkTwzkbX1S0YooM25ks5qjB1FgaJpZM4JrQ4P
.

[beorn7]

The promql tests shouldn't be concerned with pinning at all. Where do you see unpin called directly in a test?

This could very well affect normal operation, not only tests.

[tcolgate]

I had a quick look at the gocode callstacks routes to unpin. I was going to
have a look around local.NewTestStorage. I've really not dug into this
much, I'm just wondering if memory storage used via the tests is being
cleaned up in a different way (or more often), than in normal use

storage/local/chunk.go|148 col 1| Found a call path from root to (*
github.com/prometheus/prometheus/storage/local.chunkDesc).unpin
storage/local/chunk.go|145 col 22| (*
github.com/prometheus/prometheus/storage/local.chunkDesc).unpin
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage.go|1317 col 12|
static method call from (*
github.com/prometheus/prometheus/storage/local.MemorySeriesStorage).writeMemorySeries$1
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage.go|1307 col 2|
deferred static function closure call from (*
github.com/prometheus/prometheus/storage/local.MemorySeriesStorage).writeMemorySeries
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage.go|1252 col 24|
static method call from (*
github.com/prometheus/prometheus/storage/local.MemorySeriesStorage).maintainMemorySeries
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage.go|1171 col 29|
static method call from (*
github.com/prometheus/prometheus/storage/local.MemorySeriesStorage).loop
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage.go|374 col 2|
concurrent static method call from (*
github.com/prometheus/prometheus/storage/local.MemorySeriesStorage).Start
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/test_helpers.go|56 col 25|
static method call from
github.com/prometheus/prometheus/storage/local.NewTestStorage
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage_test.go|790 col 29|
static function call from
github.com/prometheus/prometheus/storage/local.testChunk
/home/tristan/go/src/
github.com/prometheus/prometheus/storage/local/storage_test.go|837 col 11|
static function call from
github.com/prometheus/prometheus/storage/local.TestChunkType2
/usr/local/go/src/testing/testing.go|610 col 4| dynamic function call from
testing.tRunner
/usr/local/go/src/testing/testing.go|791 col 10| static function call from
testing.RunTests
/usr/local/go/src/testing/testing.go|743 col 20| static function call from
(*testing.M).Run
/usr/local/go/src/testing/testing.go|708 col 65| static method call from
testing.Main

On Wed, 24 Aug 2016 at 11:39 Björn Rabenstein notifications@github.com
wrote:

The promql tests shouldn't be concerned with pinning at all. Where do you
see unpin called directly in a test?

This could very well affect normal operation, not only tests.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1912 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEo8zskjm5JuHdNLOG6BfhU_Qz285cxks5qjB9vgaJpZM4JrQ4P
.

[beorn7]

I think I got it. Fix incoming.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.