timer count sampling and persist #93
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in addition to current SIGTERM
possibly more robust
ploxiln
force-pushed
the
timer_counter_fix
branch
2 times, most recently
from
029cd0e to
d9e86f9
Compare
Contributor
Author
|
squashed in two little cleanup commits |
Contributor
Author
|
I'm actually going to do more work on this ... |
works out great: adding to zero works, and a "nil" slice, which acts like an empty slice, works with append()
Gauge relative change limit checks are a hold-over from when gauge values were uint64. float64 loses much precision at very high values, and it rounds to values it is able to represent: MaxFloat64 + 123456789.0 = MaxFloat64 (you have to subtract about 1e292 to get it to budge!) Negative gauge results seem to be part of the informal spec: > This implies you can't explicitly set a gauge to a negative number > without first setting it to zero. https://github.com/etsy/statsd/blob/master/docs/metric_types.md#gauges And really, who uses relative gauge changes anyway ...
instead of just after parsing bucket/key before counting - less work if multiple messages aggregate into single bucket need special handling for receive-counter so it continues to not have prefix/postfix apply to it rename countInactivity to inactivCounters, change to map[string]uint (planning to add inactivTimers etc) update tests for prefix/postfix applied during different stage and minor cleanup to TestProcessCounters persist-count-keys bits
by putting the sampled count first in the time values array and with new inactivTimers map also, BenchmarkManyDifferentSensors should clear the stats maps ...
ploxiln
force-pushed
the
timer_counter_fix
branch
from
d9e86f9 to
c572d91
Compare
Contributor
Author
|
refactored a bit, updated benchmark results |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This started with the motivation of making timer ".count" keys persist like counters, and also it seems like they're intended to honor the sampling so they should do that too. But the next annoying bit is how postfix needs to be removed and re-added for timers, so maybe postfix should be added last ... but then receive-counter should probably keep its current behavior of not getting the prefix/postfix ... and then when trying to add code concisely it becomes apparent that much of
packetHandler()can just be removed ...It's best to look at each commit individually for a good explanation. I would also be willing to break out some separate pull-requests (and then this one would depend on most of those going in first).
According to our benchmarks, handling timers got a bit slower, because now it adds to two maps instead of one. Some other stuff got a bit better.EDIT: refactored, and updated benchmark results.