Sleep after over-long iteration

[vickenty]

Too short iteration interval is problematic for rate calculation on slow moving
counters: small time delta pushes calculated rate way above the average rate
for the couner.

This PR revises logic introduced in #323 and limits minimum interval between
iterations to a half of the configured check period to minimize negative effect
on the rate calculations.

[truthbk]

I think Thread.sleep() for a negative value will throw a java.lang.IllegalArgumentException. Please bear in mind that some instances might take longer to collect than the actual loopPeriod. That would result in a negative sleepPeriod that could trigger that exception.

The goal of this part of the code was to ensure we didn't skip iterations (and sleep) when the duration of the collection exceeded the check period, I think this change could change that expected behavior.

Maybe I'm missing something, feel free to ping me (or drop it in here) if you want to provide more context or discuss.

[vickenty]

@truthbk

I think Thread.sleep() for a negative value will throw a java.lang.IllegalArgumentException. Please bear in mind that some instances might take longer to collect than the actual loopPeriod. That would result in a negative sleepPeriod that could trigger that exception.

sleepPeriod will not be negative, because it is assigned a new value a few lines above.

The goal of this part of the code was to ensure we didn't skip iterations (and sleep) when the duration of the collection exceeded the check period, I think this change could change that expected behavior.

True, but when the slowness is caused by the bean update process, two metric collections happen within a short time, sending two sets of metrics within one period. This is useless for gauges, skews histograms a bit and can cause confusing spikes in reported counter rates (small change over tiny time interval yields very high rate). Given that this behavior is actually relatively new, we might take a step back until a better solution is worked out. WDYT?

[truthbk]

Let's merge this fix for now to avoid those potentially nasty spikes. We should work on a refactor that might allow us improve scheduling at a per-instance level.