Hey John, I think NaN would be the better option semantically. If we were to use that, maybe it makes sense to change `Avg()`'s default value from 0.0 to NaN as well.
I am a bit more concerned with backwards compatibility if we were to use NaN since we would change three types of metrics. There were only three metrics that use `Min` but `Avg` and `Max` are used everywhere. I guess it boils down to how likely it is that such a change can break users' tools and if we're okay with it. My assumption is that it won't - most tools should be mature enough to handle these values. Thanks for the suggestion! I will wait out a bit more for other people to share their thoughts and update the KIP with `NaN` On Tue, Oct 23, 2018 at 6:12 AM John Roesler <j...@confluent.io> wrote: > Hi Stanislav, > Thanks for this KIP. I coincidentally just noticed these strange initial > values while doing some performance measurements. > > Since the metric type is a double, could we consider NaN instead? It seems > like +Inf is somewhat arbitrary for Max, so it might as well be an > arbitrary value that actually means "this is not a number". > > Consider that +- Infinity is technically "in range" for max and min, while > NaN is not. NaN is "in range" for an average or a rate, but in those cases > it would mean that the result is over 0 samples or 0 time, respectively. I > think this only happens when nothing has been recorded, so it would still > be sound for the situation you're attempting to address. > > Just to throw it out there, `null` is technically also (maybe moreso) a > sound choice, but I'd be concerned about causing a bunch of null > dereference errors. > > Thanks again, > -John > > On Mon, Oct 22, 2018 at 2:27 PM Stanislav Kozlovski < > stanis...@confluent.io> > wrote: > > > Hi Suman, > > > > Thanks for taking a look at this. Yeah, it's very minor but it changes > the > > public API (even if this is a very slight change) and as far as I know > this > > warrants some discussion > > > > On Mon, Oct 22, 2018 at 9:28 PM Suman B N <sumannew...@gmail.com> wrote: > > > > > Looks good to me. Maintains uniformity. +1 from me. > > > But its more of a bug rather than improvement proposal. Let's see what > > > contributors got to say. > > > > > > On Mon, Oct 22, 2018 at 7:23 PM Stanislav Kozlovski < > > > stanis...@confluent.io> > > > wrote: > > > > > > > Hey everybody, > > > > > > > > I've opened up a very short KIP to make the Max and Min metrics' > > default > > > > values consistent with each other. > > > > > > > > KIP: > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-386%3A+Make+Min+metrics%27+default+value+consistent+with+Max+metrics > > > > JIRA: https://issues.apache.org/jira/browse/KAFKA-7528 > > > > > > > > This is hopefully a very straightforward change. Please provide > > feedback. > > > > Thanks > > > > > > > > -- > > > > Best, > > > > Stanislav > > > > > > > > > > > > > -- > > > *Suman* > > > *OlaCabs* > > > > > > > > > -- > > Best, > > Stanislav > > > -- Best, Stanislav
