Why are there no (purposeful) callers of Monitor#getValue(int)?
seh opened this issue · 3 comments
The interface com.netflix.servo.monitor.Monitor declares the method getValue(int pollerIndex). Many classes implement that method, using the poller index to look up or update a particular value out of a set (of cardinality Pollers#NUM_POLLERS) held internally.
What I can't figure out from reading the code is why nothing calls on Monitor#getValue(int) with a poller index corresponding to those defined in Pollers#POLLING_INTERVALS. There are many call sites that pass a hard-coded zero argument for the pollerIndex parameter—presuming that at least one poller must exist—but none that I can find that, say, iterate over each of the configured pollers and request a value for an index other than zero.
Was something planned here but never implemented? Was something once present and since removed, but not without leaving this method behind as detritus?
I ask because I'm evaluating the consequence of running with the servo.pollers system property configured incorrectly. It's clear that it can induce inaccurate values for the first entry (at index zero), but past that I can't find any impact—apart from causing Servo to maintain extra values for more than one poller.
At Netflix some applications use two pollers that run at different polling intervals: one every 60s (the main poller) and one every 10s (the critical metrics poller).
Some metrics like a gauge that reports a queue size don't really care if they are called from different pollers. They will probably implement the getValue(int pollerIndex) method to return the current size and not look at the pollerIndex value at all. Others like a max gauge need to know whether you are asking for the max value for the whole minute or the max value for the last ten seconds. Another example is a StepCounter which is a counter that reports a rate for the given poller interval.
The code that calls getValue(pollerIndex) for all pollers configured looks at the system property and schedules the collection of values appropriately. Unfortunately that code is not open source since it mostly deals with how to send and transform the data in ways that make sense for our internal infrastructure.
In general if you set servo.pollers to only one value (the expected polling interval) you should be fine.
Thank you for the explanation.
In most of my applications, I run two polling threads—each at the same 10-second interval—with one polling Servo monitors via MonitorRegistryMetricPoller, the other using JvmMetricPoller. Hence I noticed that I should set servo.pollers to "10000,10000". But then I realized that the second entry won't ever get used, and I could save some space (and work) in my program by omitting the second entry. It sounds like you agree with that idea.
By the way, it's annoying that this configuration has to be known at static initialization time, as our integration of Servo reads some configuration of its own (via Archaius) to decide which pollers to schedule at what frequency. Needing to set the system property before the application starts forces us to commit to agreeing with configuration that's otherwise decided later in the application start-up sequence. But that's fodder for a separate complaint or PR.
You are correct in that servo.pollers=10000 is what you should set your property. It really is all about the polling intervals more than the pollers.