nsqd: Validate E2E percentiles & fix the config example #988
Conversation
protoss-player
changed the title
apps/nsqd/nsqd.go
Outdated
| @@ -220,6 +220,13 @@ func (p *program) Start() error { | |||
| cfg.Validate() | |||
|
|
|||
| options.Resolve(opts, flagSet, cfg) | |||
|
|
|||
| for _, v := range opts.E2EProcessingLatencyPercentiles { | |||
| if v <= 0 || v > 1 { | |||
There was a problem hiding this comment.
this line:
https://github.com/bmizerany/perks/blob/master/quantile/stream.go#L62
makes me think that 1.0 might not be an acceptable value ... though I don't understand the algorithm and can't say for sure
There was a problem hiding this comment.
It seems like 1.0 is legit, since the math you pointed at evaluates to +Inf in this case, and thus should not affect the if f < m logic a couple of lines below.
There was a problem hiding this comment.
I had to run a little experiment of my own to be sure ... dividing by zero here indeed does result in +Inf instead of NaN or a trap or something.
One more thing: the rest of the argument checking is done in NSQD.New() (in nsqd/nsqd.go) so this should probably go there.
Passing values greater than 1.0 made `quantile` library go nuts and consume lots of memory and CPU after a couple of minutes under ~1000 messages/second. I believe `nsqd` should exit with error instead of pretending that everything's OK in that case. Also, example config contradicted the nsqd's CLI, which stated that percentile values should be from `(0.0; 1.0]`.
protoss-player
force-pushed
the
validate-percentiles
branch
from
2cb61c7 to
8d2e9e6
Compare
|
Thanks! |
Passing values greater than 1 makes
quantilelibrary go nuts and consume lots of memory and CPU after a couple of minutes under ~1000 messages/second. I believensqdshould exit with error instead of pretending that everything's OK in that case.Also, example config contradicted the nsqd's CLI, which stated that percentile values should be from
(0.0; 1.0].Addresses #899.