This repository was archived by the owner on Aug 30, 2019. It is now read-only.
Ingest old traces and align their stats on oldestTs#493
Merged
Conversation
gbbr
approved these changes
Oct 16, 2018
gbbr
left a comment
Contributor
There was a problem hiding this comment.
Left some questions and a made a small grammar correction, but looks good.
| // bucket duration in nanoseconds | ||
| bsize int64 | ||
| // Timestamp of the oldest time bucket for which we still allow data. | ||
| // Any ingested stats older than it gets added to that bucket. |
Contributor
There was a problem hiding this comment.
Will this not cause incorrect stats? e.g. trace comes in 2 hours in the past but stats get added to the oldest bucket which is 10min ago.
| // Any ingested stats older than it gets added to that bucket. | ||
| oldestTs int64 | ||
| // bufferLen is the number of 10s stats bucket we keep in memory before flushing them. | ||
| // It means that we can compute stats only for the last `bufferLen * bsize` and that we |
Contributor
There was a problem hiding this comment.
we can compute stats only for the last
bufferLen * bsize
This seems to overlap with the definition of oldestTs. I'm a bit confused. Is bufferLen * bsize the same as oldestTs ?
|
|
||
| // alignTs returns the provided timestamp truncated to the bucket size. | ||
| // It gives us the start time of the time bucket in which such timestamp falls. | ||
| func alignTs(ts int64, bsize int64) int64 { |
Contributor
There was a problem hiding this comment.
Should we make bsize a constant here? (since it's not going to be configurable anymore). We could then avoid to keep passing it around and simply have it set in this file.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 PR changes the way we aggregate and flush stats in the concentrator.
What it does.
agent.Process. Theconcentratortakes care of handling traces of any time itself.bufferLen. You can see it previously used inif model.Now()-2*a.conf.BucketInterval.Nanoseconds()andif ts > now-2*c.bsize[1]. It still defaults to 2.bufferLendrives two things: it is the number of active time buckets for which we ingest statistics and the age of the time buckets we flush (aka. the delay before flushing a bucket).bufferLen +1active buckets (2 in the past + the current one) tobufferLen(the current one + the old one). That's the same behavior as before.now-c.bufferLen*bsize, like before. It means a latency of 1 to 2 bsize aka. 10s to 20s.bsize, we only flush one bucket, the one atalignedNow-c.bufferLen*bsize.oldestTs, wherenow + bufferLen*c.bsize < oldestTs < now + (bufferLen-1)*c.bsize) we basically count the span as if its time wasoldestTs. It means any old data ends up in the oldest bucket we kept.[1]
bsizeis the length of a time bucket, it's isn't publicly configurable so consider it as always 10 seconds (there is a TODO in the code to remove its configuration interface).It practice, we can summarize it to two core changes:
oldestTsbucket.oldestTsbucket.TODOS left:
bufferLen.