Expression evaluation code is not goroutine-safe

[peterbourgon]

To reproduce, create a small program which launches concurrent requests to prometheus/api/query?json=JSON&expr=<anything>. Each request will have one of three possible outcomes:

1. success (lucky you!)
2. "Error parsing rules at line X, char Y: syntax error"
3. crashing Prometheus with a slice-out-of-bounds

For the record, the panic has this stacktrace:

rules/lexer.l.go:196 (0x44c886)
rules/load.go:51 (0x44ce7e)
rules/parser.y.go:192 (0x44debf)
rules/parser.y.go:265 (0x450c0b)
rules/load.go:75 (0x44d177)
rules/load.go:116 (0x44d634)
rules/load.go:125 (0x44d746)
web/api/query.go:30 (0x5461d5)
web/api/api.go:0 (0x547565)

[matttproud]

1. Does the same expression work through the web expression browser interface?
2. Can you provide a prototype expression that you used in the report including a prototype of the type of metrics against which you want to execute it?
3. Which revision of the server did you run this against?

[peterbourgon]

1. Yes, and it works fine if I serialize or stagger my GETs, too.
2. http://xen-10-20-2-96.int.s-cloud.net:9090/api/query?expr=AVG%28indexer_poke_duration_nanoseconds%7Bstage%3D%22overall%22%7D%29&json=JSON
3. adeabca

[peterbourgon]

In an IM, Matt writes:

For long-standing queries, you probably want to add a rule to the rules file.

We're still in exploratory mode, but I'll definitely do this long-term. The context here is getting instantaneous values for the purposes of alerting. A "rule" is a precomputed expression? (Where does the "rule" terminology come from?)

A side-question I was going to raise later is: how do I construct an expression to get a scalar value (say average) over a certain time window? "Windowing" is exposed in the Graph UI as Range, but not in the expression grammar or query API as far as I could tell.

[juliusv]

Right, this is a known caveat still and I just didn't think anyone would hit it that early on in the experimental phase:

https://github.com/prometheus/prometheus/blob/master/rules/load.go#L25

I'll prioritize this.

[peterbourgon]

I just didn't think anyone would hit it that early on in the experimental phase:

:D

[juliusv]

Yeah, quite impressive, since the parsing time should be so negligibly small compared to the rest of the expression evaluation that it should be quite hard to trigger this condition under normal circumstances. In fact, I haven't managed to recreate it yet with 4 parallel curls, but they are probably just too slow. I think I'll just wrap the entire parser in a lock, since goyacc/golex don't give me much better options :-/

[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.